Merge "Only force the comment re-creation workaround for Safari"
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 57191c5..8006c61 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -43,6 +43,7 @@
import com.google.gerrit.exceptions.NotSignedInException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.SchemaUtil;
@@ -521,22 +522,14 @@
@Operator
public Predicate<ChangeData> mergedBefore(String value) throws QueryParseException {
- if (!args.index.getSchema().hasField(ChangeField.MERGED_ON)) {
- throw new QueryParseException(
- String.format(
- "'%s' operator is not supported by change index version", OPERATOR_MERGED_BEFORE));
- }
+ checkFieldAvailable(ChangeField.MERGED_ON, OPERATOR_MERGED_BEFORE);
return new BeforePredicate(
ChangeField.MERGED_ON, ChangeQueryBuilder.OPERATOR_MERGED_BEFORE, value);
}
@Operator
public Predicate<ChangeData> mergedAfter(String value) throws QueryParseException {
- if (!args.index.getSchema().hasField(ChangeField.MERGED_ON)) {
- throw new QueryParseException(
- String.format(
- "'%s' operator is not supported by change index version", OPERATOR_MERGED_AFTER));
- }
+ checkFieldAvailable(ChangeField.MERGED_ON, OPERATOR_MERGED_AFTER);
return new AfterPredicate(
ChangeField.MERGED_ON, ChangeQueryBuilder.OPERATOR_MERGED_AFTER, value);
}
@@ -626,10 +619,7 @@
}
if ("attention".equalsIgnoreCase(value)) {
- if (!args.index.getSchema().hasField(ChangeField.ATTENTION_SET_USERS)) {
- throw new QueryParseException(
- "'has:attention' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.ATTENTION_SET_USERS, "has:attention");
return new IsAttentionPredicate();
}
@@ -672,20 +662,13 @@
}
if ("uploader".equalsIgnoreCase(value)) {
- if (!args.getSchema().hasField(ChangeField.UPLOADER)) {
- throw new QueryParseException(
- "'is:uploader' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.UPLOADER, "is:uploader");
return ChangePredicates.uploader(self());
}
if ("reviewer".equalsIgnoreCase(value)) {
- if (args.getSchema().hasField(ChangeField.WIP)) {
- return Predicate.and(
- Predicate.not(new BooleanPredicate(ChangeField.WIP)),
- ReviewerPredicate.reviewer(self()));
- }
- return ReviewerPredicate.reviewer(self());
+ return Predicate.and(
+ Predicate.not(new BooleanPredicate(ChangeField.WIP)), ReviewerPredicate.reviewer(self()));
}
if ("cc".equalsIgnoreCase(value)) {
@@ -700,25 +683,16 @@
}
if ("merge".equalsIgnoreCase(value)) {
- if (args.getSchema().hasField(ChangeField.MERGE)) {
- return new BooleanPredicate(ChangeField.MERGE);
- }
- throw new QueryParseException("'is:merge' operator is not supported by change index version");
+ checkFieldAvailable(ChangeField.MERGE, "is:merge");
+ return new BooleanPredicate(ChangeField.MERGE);
}
if ("private".equalsIgnoreCase(value)) {
- if (args.getSchema().hasField(ChangeField.PRIVATE)) {
- return new BooleanPredicate(ChangeField.PRIVATE);
- }
- throw new QueryParseException(
- "'is:private' operator is not supported by change index version");
+ return new BooleanPredicate(ChangeField.PRIVATE);
}
if ("attention".equalsIgnoreCase(value)) {
- if (!args.index.getSchema().hasField(ChangeField.ATTENTION_SET_USERS)) {
- throw new QueryParseException(
- "'is:attention' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.ATTENTION_SET_USERS, "is:attention");
return new IsAttentionPredicate();
}
@@ -747,26 +721,17 @@
}
if ("started".equalsIgnoreCase(value)) {
- if (args.getSchema().hasField(ChangeField.STARTED)) {
- return new BooleanPredicate(ChangeField.STARTED);
- }
- throw new QueryParseException(
- "'is:started' operator is not supported by change index version");
+ checkFieldAvailable(ChangeField.STARTED, "is:started");
+ return new BooleanPredicate(ChangeField.STARTED);
}
if ("wip".equalsIgnoreCase(value)) {
- if (args.getSchema().hasField(ChangeField.WIP)) {
- return new BooleanPredicate(ChangeField.WIP);
- }
- throw new QueryParseException("'is:wip' operator is not supported by change index version");
+ return new BooleanPredicate(ChangeField.WIP);
}
if ("cherrypick".equalsIgnoreCase(value)) {
- if (args.getSchema().hasField(ChangeField.CHERRY_PICK)) {
- return new BooleanPredicate(ChangeField.CHERRY_PICK);
- }
- throw new QueryParseException(
- "'is:cherrypick' operator is not supported by change index version");
+ checkFieldAvailable(ChangeField.CHERRY_PICK, "is:cherrypick");
+ return new BooleanPredicate(ChangeField.CHERRY_PICK);
}
// for plugins the value will be operandName_pluginName
@@ -883,10 +848,7 @@
return ChangePredicates.hashtag(hashtag);
}
- if (!args.index.getSchema().hasField(ChangeField.FUZZY_HASHTAG)) {
- throw new QueryParseException(
- "'inhashtag' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.FUZZY_HASHTAG, "inhashtag");
return ChangePredicates.fuzzyHashtag(hashtag);
}
@@ -942,10 +904,7 @@
@Operator
public Predicate<ChangeData> extension(String ext) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.EXTENSION)) {
- return new FileExtensionPredicate(ext);
- }
- throw new QueryParseException("'extension' operator is not supported by change index version");
+ return new FileExtensionPredicate(ext);
}
@Operator
@@ -955,19 +914,12 @@
@Operator
public Predicate<ChangeData> onlyextensions(String extList) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.ONLY_EXTENSIONS)) {
- return new FileExtensionListPredicate(extList);
- }
- throw new QueryParseException(
- "'onlyextensions' operator is not supported by change index version");
+ return new FileExtensionListPredicate(extList);
}
@Operator
public Predicate<ChangeData> footer(String footer) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.FOOTER)) {
- return ChangePredicates.footer(footer);
- }
- throw new QueryParseException("'footer' operator is not supported by change index version");
+ return ChangePredicates.footer(footer);
}
@Operator
@@ -977,13 +929,10 @@
@Operator
public Predicate<ChangeData> directory(String directory) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.DIRECTORY)) {
- if (directory.startsWith("^")) {
- return new RegexDirectoryPredicate(directory);
- }
- return ChangePredicates.directory(directory);
+ if (directory.startsWith("^")) {
+ return new RegexDirectoryPredicate(directory);
}
- throw new QueryParseException("'directory' operator is not supported by change index version");
+ return ChangePredicates.directory(directory);
}
@Operator
@@ -1055,7 +1004,7 @@
// If the vote piece looks like Code-Review=NEED with a valid non-numeric
// submit record status, interpret as a submit record query.
int eq = name.indexOf('=');
- if (args.getSchema().hasField(ChangeField.SUBMIT_RECORD) && eq > 0) {
+ if (eq > 0) {
String statusName = name.substring(eq + 1).toUpperCase();
if (!isInt(statusName) && !MagicLabelValue.tryParse(statusName).isPresent()) {
SubmitRecord.Label.Status status =
@@ -1197,9 +1146,7 @@
@Operator
public Predicate<ChangeData> uploader(String who)
throws QueryParseException, IOException, ConfigInvalidException {
- if (!args.getSchema().hasField(ChangeField.UPLOADER)) {
- throw new QueryParseException("'uploader' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.UPLOADER, "uploader");
return uploader(parseAccount(who, (AccountState s) -> true));
}
@@ -1214,10 +1161,7 @@
@Operator
public Predicate<ChangeData> attention(String who)
throws QueryParseException, IOException, ConfigInvalidException {
- if (!args.index.getSchema().hasField(ChangeField.ATTENTION_SET_USERS)) {
- throw new QueryParseException(
- "'attention' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.ATTENTION_SET_USERS, "attention");
return attention(parseAccount(who, (AccountState s) -> true));
}
@@ -1262,9 +1206,7 @@
@Operator
public Predicate<ChangeData> uploaderin(String group) throws QueryParseException, IOException {
- if (!args.getSchema().hasField(ChangeField.UPLOADER)) {
- throw new QueryParseException("'uploader' operator is not supported by change index version");
- }
+ checkFieldAvailable(ChangeField.UPLOADER, "uploaderin");
GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
if (g == null) {
@@ -1309,10 +1251,7 @@
if (Objects.equals(byState, Predicate.<ChangeData>any())) {
return Predicate.any();
}
- if (args.getSchema().hasField(ChangeField.WIP)) {
- return Predicate.and(Predicate.not(new BooleanPredicate(ChangeField.WIP)), byState);
- }
- return byState;
+ return Predicate.and(Predicate.not(new BooleanPredicate(ChangeField.WIP)), byState);
}
@Operator
@@ -1488,20 +1427,14 @@
@Operator
public Predicate<ChangeData> author(String who) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.EXACT_AUTHOR)) {
- return getAuthorOrCommitterPredicate(
- who.trim(), ChangePredicates::exactAuthor, ChangePredicates::author);
- }
- return getAuthorOrCommitterFullTextPredicate(who.trim(), ChangePredicates::author);
+ return getAuthorOrCommitterPredicate(
+ who.trim(), ChangePredicates::exactAuthor, ChangePredicates::author);
}
@Operator
public Predicate<ChangeData> committer(String who) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.EXACT_COMMITTER)) {
- return getAuthorOrCommitterPredicate(
- who.trim(), ChangePredicates::exactCommitter, ChangePredicates::committer);
- }
- return getAuthorOrCommitterFullTextPredicate(who.trim(), ChangePredicates::committer);
+ return getAuthorOrCommitterPredicate(
+ who.trim(), ChangePredicates::exactCommitter, ChangePredicates::committer);
}
@Operator
@@ -1524,41 +1457,31 @@
if (value == null || Ints.tryParse(value) == null) {
throw new QueryParseException("'revertof' must be an integer");
}
- if (args.getSchema().hasField(ChangeField.REVERT_OF)) {
- return ChangePredicates.revertOf(Change.id(Ints.tryParse(value)));
- }
- throw new QueryParseException("'revertof' operator is not supported by change index version");
+ return ChangePredicates.revertOf(Change.id(Ints.tryParse(value)));
}
@Operator
public Predicate<ChangeData> submissionId(String value) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.SUBMISSIONID)) {
- return ChangePredicates.submissionId(value);
- }
- throw new QueryParseException(
- "'submissionid' operator is not supported by change index version");
+ return ChangePredicates.submissionId(value);
}
@Operator
public Predicate<ChangeData> cherryPickOf(String value) throws QueryParseException {
- if (args.getSchema().hasField(ChangeField.CHERRY_PICK_OF_CHANGE)
- && args.getSchema().hasField(ChangeField.CHERRY_PICK_OF_PATCHSET)) {
- if (Ints.tryParse(value) != null) {
- return ChangePredicates.cherryPickOf(Change.id(Ints.tryParse(value)));
- }
- try {
- PatchSet.Id patchSetId = PatchSet.Id.parse(value);
- return ChangePredicates.cherryPickOf(patchSetId);
- } catch (IllegalArgumentException e) {
- throw new QueryParseException(
- "'"
- + value
- + "' is not a valid input. It must be in the 'ChangeNumber[,PatchsetNumber]' format.",
- e);
- }
+ checkFieldAvailable(ChangeField.CHERRY_PICK_OF_CHANGE, "cherryPickOf");
+ checkFieldAvailable(ChangeField.CHERRY_PICK_OF_PATCHSET, "cherryPickOf");
+ if (Ints.tryParse(value) != null) {
+ return ChangePredicates.cherryPickOf(Change.id(Ints.tryParse(value)));
}
- throw new QueryParseException(
- "'cherrypickof' operator is not supported by change index version");
+ try {
+ PatchSet.Id patchSetId = PatchSet.Id.parse(value);
+ return ChangePredicates.cherryPickOf(patchSetId);
+ } catch (IllegalArgumentException e) {
+ throw new QueryParseException(
+ "'"
+ + value
+ + "' is not a valid input. It must be in the 'ChangeNumber[,PatchsetNumber]' format.",
+ e);
+ }
}
@Override
@@ -1617,6 +1540,14 @@
return Predicate.or(predicates);
}
+ protected void checkFieldAvailable(FieldDef<ChangeData, ?> field, String operator)
+ throws QueryParseException {
+ if (!args.index.getSchema().hasField(field)) {
+ throw new QueryParseException(
+ String.format("'%s' operator is not supported by change index version", operator));
+ }
+ }
+
private Predicate<ChangeData> getAuthorOrCommitterPredicate(
String who,
Function<String, Predicate<ChangeData>> exactPredicateFunc,
@@ -1731,11 +1662,9 @@
String who, ReviewerStateInternal state, boolean forDefaultField)
throws QueryParseException, IOException, ConfigInvalidException {
Predicate<ChangeData> reviewerByEmailPredicate = null;
- if (args.index.getSchema().hasField(ChangeField.REVIEWER_BY_EMAIL)) {
- Address address = Address.tryParse(who);
- if (address != null) {
- reviewerByEmailPredicate = ReviewerByEmailPredicate.forState(address, state);
- }
+ Address address = Address.tryParse(who);
+ if (address != null) {
+ reviewerByEmailPredicate = ReviewerByEmailPredicate.forState(address, state);
}
Predicate<ChangeData> reviewerPredicate = null;
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
index e63714f..cd0fee3 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.query.QueryBuilder;
import com.google.inject.Inject;
@@ -32,4 +33,10 @@
SubmitRequirementChangeQueryBuilder(Arguments args) {
super(def, args);
}
+
+ @Override
+ protected void checkFieldAvailable(FieldDef<ChangeData, ?> field, String operator) {
+ // Submit requirements don't rely on the index, so they can be used regardless of index schema
+ // version.
+ }
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts
index 2b9abfd..4432cc82 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts
@@ -20,6 +20,7 @@
import {customElement, property} from 'lit/decorators';
import {ChangeInfo, SubmitRequirementStatus} from '../../../api/rest-api';
import {changeIsMerged} from '../../../utils/change-util';
+import {getRequirements} from '../../../utils/label-util';
@customElement('gr-change-list-column-requirements')
export class GrChangeListColumRequirements extends LitElement {
@@ -52,9 +53,7 @@
return this.renderState('check', 'Merged');
}
- const submitRequirements = (this.change?.submit_requirements ?? []).filter(
- req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE
- );
+ const submitRequirements = getRequirements(this.change);
if (!submitRequirements.length) return html`n/a`;
const numOfRequirements = submitRequirements.length;
const numOfSatisfiedRequirements = submitRequirements.filter(
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 82deeb5..3e5d11c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -131,7 +131,11 @@
ChangeComments,
GrCommentApi,
} from '../../diff/gr-comment-api/gr-comment-api';
-import {assertIsDefined, hasOwnProperty} from '../../../utils/common-util';
+import {
+ assertIsDefined,
+ hasOwnProperty,
+ query,
+} from '../../../utils/common-util';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {
CommentThread,
@@ -234,7 +238,6 @@
downloadOverlay: GrOverlay;
downloadDialog: GrDownloadDialog;
replyOverlay: GrOverlay;
- replyDialog: GrReplyDialog;
mainContent: HTMLDivElement;
changeStar: GrChangeStar;
actions: GrChangeActions;
@@ -519,7 +522,7 @@
_activeTabs: string[] = [PrimaryTab.FILES, SecondaryTab.CHANGE_LOG];
@property({type: Boolean})
- unresolvedOnly = false;
+ unresolvedOnly = true;
@property({type: Boolean})
_showAllRobotComments = false;
@@ -544,6 +547,10 @@
@property({type: String})
scrollCommentId?: UrlEncodedCommentId;
+ /** Just reflects the `opened` prop of the overlay. */
+ @property({type: Boolean})
+ replyOverlayOpened = false;
+
@property({
type: Array,
computed: '_computeResolveWeblinks(_change, _commitInfo, _serverConfig)',
@@ -558,8 +565,6 @@
private readonly shortcuts = appContext.shortcutsService;
- private replyDialogResizeObserver?: ResizeObserver;
-
override keyboardShortcuts(): ShortcutListener[] {
return [
listen(Shortcut.SEND_REPLY, _ => {}), // docOnly
@@ -678,11 +683,6 @@
}
});
- this.replyDialogResizeObserver = new ResizeObserver(() =>
- this.$.replyOverlay.center()
- );
- this.replyDialogResizeObserver.observe(this.$.replyDialog);
-
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -865,7 +865,7 @@
const hasUnresolvedThreads =
(this._commentThreads ?? []).filter(thread => isUnresolved(thread))
.length > 0;
- if (hasUnresolvedThreads) this.unresolvedOnly = true;
+ if (!hasUnresolvedThreads) this.unresolvedOnly = false;
}
this.reporting.reportInteraction(Interaction.SHOW_TAB, {
@@ -1055,7 +1055,7 @@
_handleReplyTap(e: MouseEvent) {
e.preventDefault();
- this._openReplyDialog(this.$.replyDialog.FocusTarget.ANY);
+ this._openReplyDialog(FocusTarget.ANY);
}
onReplyOverlayCanceled() {
@@ -1099,8 +1099,7 @@
.split('\n')
.map(line => '> ' + line)
.join('\n') + '\n\n';
- this.$.replyDialog.quote = quoteStr;
- this._openReplyDialog(this.$.replyDialog.FocusTarget.BODY);
+ this._openReplyDialog(FocusTarget.BODY, quoteStr);
}
_handleHideBackgroundContent() {
@@ -1137,9 +1136,9 @@
}
_handleShowReplyDialog(e: CustomEvent<{value: {ccsOnly: boolean}}>) {
- let target = this.$.replyDialog.FocusTarget.REVIEWERS;
+ let target = FocusTarget.REVIEWERS;
if (e.detail.value && e.detail.value.ccsOnly) {
- target = this.$.replyDialog.FocusTarget.CCS;
+ target = FocusTarget.CCS;
}
this._openReplyDialog(target);
}
@@ -1217,27 +1216,43 @@
basePatchNum: value.basePatchNum,
};
- this.$.fileList.collapseAllDiffs();
this._patchRange = patchRange;
this.scrollCommentId = value.commentId;
const patchKnown =
!patchRange.patchNum ||
(this._allPatchSets ?? []).some(ps => ps.num === patchRange.patchNum);
+ // _allPatchsets does not know value.patchNum so force a reload.
+ const forceReload = value.forceReload || !patchKnown;
- // If the change has already been loaded and the parameter change is only
- // in the patch range, then don't do a full reload.
- if (this._changeNum !== undefined && patchChanged && patchKnown) {
- if (!patchRange.patchNum) {
- patchRange.patchNum = computeLatestPatchNum(this._allPatchSets);
- rightPatchNumChanged = true;
+ // If changeNum is defined that means the change has already been
+ // rendered once before so a full reload is not required.
+ if (this._changeNum !== undefined && !forceReload) {
+ if (patchChanged) {
+ // We need to collapse all diffs when params change so that a non
+ // existing diff is not requested. See Issue 125270 for more details.
+ this.$.fileList.collapseAllDiffs();
+ if (!patchRange.patchNum) {
+ patchRange.patchNum = computeLatestPatchNum(this._allPatchSets);
+ rightPatchNumChanged = true;
+ }
+ this._reloadPatchNumDependentResources(rightPatchNumChanged).then(
+ () => {
+ this._sendShowChangeEvent();
+ }
+ );
}
- this._reloadPatchNumDependentResources(rightPatchNumChanged).then(() => {
- this._sendShowChangeEvent();
- });
+
+ // If there is no change in patchset or changeNum, such as when user goes
+ // to the diff view and then comes back to change page then there is no
+ // need to reload anything and we render the change view component as is.
return;
}
+ // We need to collapse all diffs when params change so that a non existing
+ // diff is not requested. See Issue 125270 for more details.
+ this.$.fileList.collapseAllDiffs();
+
this._initialLoadComplete = false;
this._changeNum = value.changeNum;
this.loadData(true).then(() => {
@@ -1253,8 +1268,8 @@
_initActiveTabs(params?: AppElementChangeViewParams) {
let primaryTab = PrimaryTab.FILES;
- if (params && params.queryMap && params.queryMap.has('tab')) {
- primaryTab = params.queryMap.get('tab') as PrimaryTab;
+ if (params?.tab) {
+ primaryTab = params?.tab as PrimaryTab;
} else if (params && 'commentId' in params) {
primaryTab = PrimaryTab.COMMENT_THREADS;
}
@@ -1390,7 +1405,7 @@
}
if (this.viewState.showReplyDialog) {
- this._openReplyDialog(this.$.replyDialog.FocusTarget.ANY);
+ this._openReplyDialog(FocusTarget.ANY);
this.set('viewState.showReplyDialog', false);
}
});
@@ -1496,7 +1511,7 @@
fireEvent(this, 'show-auth-required');
return;
}
- this._openReplyDialog(this.$.replyDialog.FocusTarget.ANY);
+ this._openReplyDialog(FocusTarget.ANY);
});
}
@@ -1681,12 +1696,17 @@
});
}
- _openReplyDialog(section?: FocusTarget) {
+ _openReplyDialog(focusTarget?: FocusTarget, quote?: string) {
if (!this._change) return;
- this.$.replyOverlay.open().finally(() => {
+ const overlay = this.$.replyOverlay;
+ overlay.open().finally(async () => {
// the following code should be executed no matter open succeed or not
+ const dialog = query<GrReplyDialog>(this, '#replyDialog');
+ assertIsDefined(dialog, 'reply dialog');
this._resetReplyOverlayFocusStops();
- this.$.replyDialog.open(section);
+ dialog.open(focusTarget, quote);
+ const observer = new ResizeObserver(() => overlay.center());
+ observer.observe(dialog);
});
fireDialogChange(this, {opened: true});
this._changeViewAriaHidden = true;
@@ -2017,8 +2037,8 @@
*
* @param isLocationChange Reloads the related changes
* when true and ends reporting events that started on location change.
- * @param clearPatchset Reloads the related changes
- * ignoring any patchset choice made.
+ * @param clearPatchset Reloads the change ignoring any patchset
+ * choice made.
* @return A promise that resolves when the core data has loaded.
* Some non-core data loading may still be in-flight when the core data
* promise resolves.
@@ -2026,7 +2046,14 @@
loadData(isLocationChange?: boolean, clearPatchset?: boolean): Promise<void> {
if (this.isChangeObsolete()) return Promise.resolve();
if (clearPatchset && this._change) {
- GerritNav.navigateToChange(this._change);
+ GerritNav.navigateToChange(
+ this._change,
+ undefined,
+ undefined,
+ undefined,
+ undefined,
+ true
+ );
return Promise.resolve();
}
this._loading = true;
@@ -2470,18 +2497,34 @@
) {
patchNum = this._patchRange.patchNum;
}
- GerritNav.navigateToChange(this._change, patchNum, undefined, true);
+ GerritNav.navigateToChange(
+ this._change,
+ patchNum,
+ undefined,
+ true,
+ undefined,
+ true
+ );
}
_handleStopEditTap() {
assertIsDefined(this._change, '_change');
if (!this._patchRange)
throw new Error('missing required _patchRange property');
- GerritNav.navigateToChange(this._change, this._patchRange.patchNum);
+ GerritNav.navigateToChange(
+ this._change,
+ this._patchRange.patchNum,
+ undefined,
+ undefined,
+ undefined,
+ true
+ );
}
_resetReplyOverlayFocusStops() {
- this.$.replyOverlay.setFocusStops(this.$.replyDialog.getFocusStops());
+ const dialog = query<GrReplyDialog>(this, '#replyDialog');
+ if (!dialog) return;
+ this.$.replyOverlay.setFocusStops(dialog.getFocusStops());
}
_handleToggleStar(e: CustomEvent<{change: ChangeInfo; starred: boolean}>) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index d42fc17..f125670 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -695,24 +695,27 @@
no-cancel-on-esc-key=""
scroll-action="lock"
with-backdrop=""
+ opened="{{replyOverlayOpened}}"
on-iron-overlay-canceled="onReplyOverlayCanceled"
>
- <gr-reply-dialog
- id="replyDialog"
- change="{{_change}}"
- patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
- permitted-labels="[[_change.permitted_labels]]"
- draft-comment-threads="[[_draftCommentThreads]]"
- project-config="[[_projectConfig]]"
- server-config="[[_serverConfig]]"
- can-be-started="[[_canStartReview]]"
- on-send="_handleReplySent"
- on-cancel="_handleReplyCancel"
- on-autogrow="_handleReplyAutogrow"
- on-send-disabled-changed="_resetReplyOverlayFocusStops"
- hidden$="[[!_loggedIn]]"
- >
- </gr-reply-dialog>
+ <template is="dom-if" if="[[replyOverlayOpened]]">
+ <gr-reply-dialog
+ id="replyDialog"
+ change="{{_change}}"
+ patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
+ permitted-labels="[[_change.permitted_labels]]"
+ draft-comment-threads="[[_draftCommentThreads]]"
+ project-config="[[_projectConfig]]"
+ server-config="[[_serverConfig]]"
+ can-be-started="[[_canStartReview]]"
+ on-send="_handleReplySent"
+ on-cancel="_handleReplyCancel"
+ on-autogrow="_handleReplyAutogrow"
+ on-send-disabled-changed="_resetReplyOverlayFocusStops"
+ hidden$="[[!_loggedIn]]"
+ >
+ </gr-reply-dialog>
+ </template>
</gr-overlay>
<gr-comment-api id="commentAPI"></gr-comment-api>
`;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index acdaefe..591aa41 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -37,7 +37,14 @@
import {EventType, PluginApi} from '../../../api/plugin';
import 'lodash/lodash';
-import {mockPromise, stubRestApi, stubUsers} from '../../../test/test-utils';
+import {
+ mockPromise,
+ queryAndAssert,
+ stubRestApi,
+ stubUsers,
+ waitQueryAndAssert,
+ waitUntil,
+} from '../../../test/test-utils';
import {
createAppElementChangeViewParams,
createApproval,
@@ -97,6 +104,8 @@
import {appContext} from '../../../services/app-context';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
import {_testOnly_setState} from '../../../services/user/user-model';
+import {FocusTarget, GrReplyDialog} from '../gr-reply-dialog/gr-reply-dialog';
+import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
const pluginApi = _testOnly_initGerritPluginApi();
const fixture = fixtureFromElement('gr-change-view');
@@ -548,13 +557,12 @@
test('param change should switch primary tab correctly', async () => {
assert.equal(element._activeTabs[0], PrimaryTab.FILES);
- const queryMap = new Map<string, string>();
- queryMap.set('tab', PrimaryTab.FINDINGS);
// view is required
+ element._changeNum = undefined;
element.params = {
...createAppElementChangeViewParams(),
...element.params,
- queryMap,
+ tab: PrimaryTab.FINDINGS,
};
await flush();
assert.equal(element._activeTabs[0], PrimaryTab.FINDINGS);
@@ -562,13 +570,11 @@
test('invalid param change should not switch primary tab', async () => {
assert.equal(element._activeTabs[0], PrimaryTab.FILES);
- const queryMap = new Map<string, string>();
- queryMap.set('tab', 'random');
// view is required
element.params = {
...createAppElementChangeViewParams(),
...element.params,
- queryMap,
+ tab: 'random',
};
await flush();
assert.equal(element._activeTabs[0], PrimaryTab.FILES);
@@ -681,9 +687,7 @@
element.$.replyOverlay.close();
assert.isFalse(element.$.replyOverlay.opened);
assert(
- openSpy.lastCall.calledWithExactly(
- element.$.replyDialog.FocusTarget.ANY
- ),
+ openSpy.lastCall.calledWithExactly(FocusTarget.ANY),
'_openReplyDialog should have been passed ANY'
);
assert.equal(openSpy.callCount, 1);
@@ -705,7 +709,8 @@
},
};
const handlerSpy = sinon.spy(element, '_handleHideBackgroundContent');
- element.$.replyDialog.dispatchEvent(
+ const overlay = queryAndAssert<GrOverlay>(element, '#replyOverlay');
+ overlay.dispatchEvent(
new CustomEvent('fullscreen-overlay-opened', {
composed: true,
bubbles: true,
@@ -732,7 +737,8 @@
},
};
const handlerSpy = sinon.spy(element, '_handleShowBackgroundContent');
- element.$.replyDialog.dispatchEvent(
+ const overlay = queryAndAssert<GrOverlay>(element, '#replyOverlay');
+ overlay.dispatchEvent(
new CustomEvent('fullscreen-overlay-closed', {
composed: true,
bubbles: true,
@@ -1306,12 +1312,12 @@
.callsFake(() => Promise.resolve([undefined, undefined, undefined]));
flush();
const collapseStub = sinon.stub(element.$.fileList, 'collapseAllDiffs');
-
const value: AppElementChangeViewParams = {
...createAppElementChangeViewParams(),
view: GerritView.CHANGE,
patchNum: 1 as RevisionPatchSetNum,
};
+ element._changeNum = undefined;
element.params = value;
await flush();
assert.isTrue(reloadStub.calledOnce);
@@ -1369,7 +1375,7 @@
assert.isTrue(reloadPortedCommentsStub.calledOnce);
});
- test('reload entire page when patchRange doesnt change', async () => {
+ test('do not reload entire page when patchRange doesnt change', async () => {
const reloadStub = sinon
.stub(element, 'loadData')
.callsFake(() => Promise.resolve());
@@ -1377,13 +1383,15 @@
const value: AppElementChangeViewParams =
createAppElementChangeViewParams();
element.params = value;
+ // change already loaded
+ assert.isOk(element._changeNum);
await flush();
- assert.isTrue(reloadStub.calledOnce);
+ assert.isFalse(reloadStub.calledOnce);
element._initialLoadComplete = true;
element.params = {...value};
await flush();
- assert.isTrue(reloadStub.calledTwice);
- assert.isTrue(collapseStub.calledTwice);
+ assert.isFalse(reloadStub.calledTwice);
+ assert.isFalse(collapseStub.calledTwice);
});
test('do not handle new change numbers', async () => {
@@ -1608,9 +1616,7 @@
const openStub = sinon.stub(element, '_openReplyDialog');
tap(element.$.replyBtn);
assert(
- openStub.lastCall.calledWithExactly(
- element.$.replyDialog.FocusTarget.ANY
- ),
+ openStub.lastCall.calledWithExactly(FocusTarget.ANY),
'_openReplyDialog should have been passed ANY'
);
assert.equal(openStub.callCount, 1);
@@ -1629,18 +1635,12 @@
bubbles: true,
})
);
- assert(
- openStub.lastCall.calledWithExactly(
- element.$.replyDialog.FocusTarget.BODY
- ),
- '_openReplyDialog should have been passed BODY'
- );
- assert.equal(openStub.callCount, 1);
+ assert.isTrue(openStub.calledOnce);
+ assert.equal(openStub.lastCall.args[0], FocusTarget.BODY);
}
);
test('reply dialog focus can be controlled', () => {
- const FocusTarget = element.$.replyDialog.FocusTarget;
const openStub = sinon.stub(element, '_openReplyDialog');
const e = new CustomEvent('show-reply-dialog', {
@@ -1716,7 +1716,6 @@
suite('reply dialog tests', () => {
setup(() => {
- sinon.stub(element.$.replyDialog, '_draftChanged');
element._change = {
...createChangeViewChange(),
revisions: createRevisions(1),
@@ -1747,52 +1746,18 @@
assert.isTrue(openReplyDialogStub.calledOnce);
});
- test('reply from comment adds quote text', () => {
+ test('reply from comment adds quote text', async () => {
const e = new CustomEvent('', {
detail: {message: {message: 'quote text'}},
});
element._handleMessageReply(e);
- assert.equal(element.$.replyDialog.quote, '> quote text\n\n');
- });
-
- test('reply from comment replaces quote text', () => {
- element.$.replyDialog.draft = '> old quote text\n\n some draft text';
- element.$.replyDialog.quote = '> old quote text\n\n';
- const e = new CustomEvent('', {
- detail: {message: {message: 'quote text'}},
- });
- element._handleMessageReply(e);
- assert.equal(element.$.replyDialog.quote, '> quote text\n\n');
- });
-
- test('reply from same comment preserves quote text', () => {
- element.$.replyDialog.draft = '> quote text\n\n some draft text';
- element.$.replyDialog.quote = '> quote text\n\n';
- const e = new CustomEvent('', {
- detail: {message: {message: 'quote text'}},
- });
- element._handleMessageReply(e);
- assert.equal(
- element.$.replyDialog.draft,
- '> quote text\n\n some draft text'
+ const dialog = await waitQueryAndAssert<GrReplyDialog>(
+ element,
+ '#replyDialog'
);
- assert.equal(element.$.replyDialog.quote, '> quote text\n\n');
- });
-
- test('reply from top of page contains previous draft', () => {
- const div = document.createElement('div');
- element.$.replyDialog.draft = '> quote text\n\n some draft text';
- element.$.replyDialog.quote = '> quote text\n\n';
- const e = {
- target: div,
- preventDefault: sinon.spy(),
- } as unknown as MouseEvent;
- element._handleReplyTap(e);
- assert.equal(
- element.$.replyDialog.draft,
- '> quote text\n\n some draft text'
- );
- assert.equal(element.$.replyDialog.quote, '> quote text\n\n');
+ const openSpy = sinon.spy(dialog, 'open');
+ await waitUntil(() => openSpy.called && !!openSpy.lastCall.args[1]);
+ assert.equal(openSpy.lastCall.args[1], '> quote text\n\n');
});
});
@@ -2097,7 +2062,7 @@
test('no edit exists in revisions, non-latest patchset', async () => {
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 4);
+ assert.equal(args.length, 6);
assert.equal(args[1], 1 as PatchSetNum); // patchNum
assert.equal(args[3], true); // opt_isEdit
promise.resolve();
@@ -2114,7 +2079,7 @@
test('no edit exists in revisions, latest patchset', async () => {
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 4);
+ assert.equal(args.length, 6);
// No patch should be specified when patchNum == latest.
assert.isNotOk(args[1]); // patchNum
assert.equal(args[3], true); // opt_isEdit
@@ -2138,7 +2103,7 @@
navigateToChangeStub.restore();
const promise = mockPromise();
sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 2);
+ assert.equal(args.length, 6);
assert.equal(args[1], 1 as PatchSetNum); // patchNum
promise.resolve();
});
@@ -2256,6 +2221,8 @@
appContext.reportingService,
'changeFullyLoaded'
);
+ // reset so reload is triggered
+ element._changeNum = undefined;
element.params = {
...createAppElementChangeViewParams(),
changeNum: TEST_NUMERIC_CHANGE_ID,
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index b8932e5..5109b72 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -240,9 +240,6 @@
@property({type: String, observer: '_draftChanged'})
draft = '';
- @property({type: String})
- quote = '';
-
@property({type: Object})
filterReviewerSuggestion: (input: Suggestion) => boolean;
@@ -427,7 +424,13 @@
super.disconnectedCallback();
}
- open(focusTarget?: FocusTarget) {
+ /**
+ * Note that this method is not actually *opening* the dialog. Opening and
+ * showing the dialog is dealt with by the overlay. This method is used by the
+ * change view for initializing the dialog after opening the overlay. Maybe it
+ * should be called `onOpened()` or `initialize()`?
+ */
+ open(focusTarget?: FocusTarget, quote?: string) {
assertIsDefined(this.change, 'change');
this.knownLatestState = LatestPatchState.CHECKING;
this.changeService.fetchChangeUpdates(this.change).then(result => {
@@ -437,10 +440,9 @@
});
this._focusOn(focusTarget);
- if (this.quote && this.quote.length) {
- // If a reply quote has been provided, use it and clear the property.
- this.draft = this.quote;
- this.quote = '';
+ if (quote?.length) {
+ // If a reply quote has been provided, use it.
+ this.draft = quote;
} else {
// Otherwise, check for an unsaved draft in localstorage.
this.draft = this._loadStoredDraft();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index 5a11f47..cf31a4f 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -18,9 +18,11 @@
import '../../../test/common-test-setup-karma';
import './gr-reply-dialog';
import {
+ addListenerForTest,
mockPromise,
queryAll,
queryAndAssert,
+ stubRestApi,
stubStorage,
} from '../../../test/test-utils';
import {
@@ -29,8 +31,6 @@
SpecialFilePath,
} from '../../../constants/constants';
import {appContext} from '../../../services/app-context';
-import {addListenerForTest} from '../../../test/test-utils';
-import {stubRestApi} from '../../../test/test-utils';
import {JSON_PREFIX} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
import {StandardLabels} from '../../../utils/label-util';
import {
@@ -44,7 +44,7 @@
pressAndReleaseKeyOn,
tap,
} from '@polymer/iron-test-helpers/mock-interactions';
-import {GrReplyDialog} from './gr-reply-dialog';
+import {FocusTarget, GrReplyDialog} from './gr-reply-dialog';
import {
AccountId,
AccountInfo,
@@ -1317,11 +1317,9 @@
const storedDraft = 'hello world';
const quote = '> foo bar';
getDraftCommentStub.returns({message: storedDraft});
- element.quote = quote;
- element.open();
+ element.open(FocusTarget.ANY, quote);
assert.isFalse(getDraftCommentStub.called);
assert.equal(element.draft, quote);
- assert.isNotOk(element.quote);
});
test('updates stored draft on edits', async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index 157336d..2f3e1e2 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -33,6 +33,7 @@
import {
extractAssociatedLabels,
getAllUniqueApprovals,
+ getRequirements,
hasNeutralStatus,
hasVotes,
iconForStatus,
@@ -139,18 +140,9 @@
}
override render() {
- let submit_requirements = orderSubmitRequirements(
- this.change?.submit_requirements ?? []
- ).filter(req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE);
-
- const hasNonLegacyRequirements = submit_requirements.some(
- req => req.is_legacy === false
+ const submit_requirements = orderSubmitRequirements(
+ getRequirements(this.change)
);
- if (hasNonLegacyRequirements) {
- submit_requirements = submit_requirements.filter(
- req => req.is_legacy === false
- );
- }
return html` <h3
class="metadata-title heading-3"
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index b8f2630..6b4006c 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -256,11 +256,9 @@
edit?: boolean;
host?: string;
messageHash?: string;
- queryMap?: Map<string, string> | URLSearchParams;
commentId?: UrlEncodedCommentId;
-
- // TODO(TS): querystring isn't set anywhere, try to remove
- querystring?: string;
+ forceReload?: boolean;
+ tab?: string;
}
export interface GenerateUrlRepoViewParameters {
@@ -612,7 +610,8 @@
patchNum?: PatchSetNum,
basePatchNum?: BasePatchSetNum,
isEdit?: boolean,
- messageHash?: string
+ messageHash?: string,
+ forceReload?: boolean
) {
if (basePatchNum === ParentPatchSetNum) {
basePatchNum = undefined;
@@ -628,6 +627,7 @@
edit: isEdit,
host: change.internalHost || undefined,
messageHash,
+ forceReload,
});
},
@@ -649,17 +649,28 @@
* @param redirect redirect to a change - if true, the current
* location (i.e. page which makes redirect) is not added to a history.
* I.e. back/forward buttons skip current location
- *
+ * @param forceReload Some views are smart about how to handle the reload
+ * of the view. In certain cases we want to force the view to reload
+ * and re-render everything.
*/
+ // TODO(dhruvsri): move the arguments into one options object
navigateToChange(
change: Pick<ChangeInfo, '_number' | 'project' | 'internalHost'>,
patchNum?: PatchSetNum,
basePatchNum?: BasePatchSetNum,
isEdit?: boolean,
- redirect?: boolean
+ redirect?: boolean,
+ forceReload?: boolean
) {
this._navigate(
- this.getUrlForChange(change, patchNum, basePatchNum, isEdit),
+ this.getUrlForChange(
+ change,
+ patchNum,
+ basePatchNum,
+ isEdit,
+ undefined,
+ forceReload
+ ),
redirect
);
},
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index f0cff85..65ac9df 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -530,17 +530,22 @@
range = '/' + range;
}
let suffix = `${range}`;
- if (params.querystring) {
- suffix += '?' + params.querystring;
- } else if (params.edit) {
- suffix += ',edit';
+ let queryString = '';
+ if (params.forceReload) {
+ queryString = 'forceReload=true';
}
- if (params.messageHash) {
- suffix += params.messageHash;
+ if (params.edit) {
+ suffix += ',edit';
}
if (params.commentId) {
suffix = suffix + `/comments/${params.commentId}`;
}
+ if (queryString) {
+ suffix += '?' + queryString;
+ }
+ if (params.messageHash) {
+ suffix += params.messageHash;
+ }
if (params.project) {
const encodedProject = encodeURL(params.project, true);
return `/c/${encodedProject}/+/${params.changeNum}${suffix}`;
@@ -1563,9 +1568,20 @@
basePatchNum: convertToPatchSetNum(ctx.params[4]) as BasePatchSetNum,
patchNum: convertToPatchSetNum(ctx.params[6]),
view: GerritView.CHANGE,
- queryMap: ctx.queryMap,
};
+ if (ctx.queryMap.has('forceReload')) {
+ params.forceReload = true;
+ history.replaceState(
+ null,
+ '',
+ location.href.replace(/[?&]forceReload=true/, '')
+ );
+ }
+
+ const tab = ctx.queryMap.get('tab');
+ if (tab) params.tab = tab;
+
this.reporting.setRepoName(params.project);
this.reporting.setChangeId(changeNum);
this._redirectOrNavigate(params);
@@ -1661,13 +1677,24 @@
// Parameter order is based on the regex group number matched.
const project = ctx.params[0] as RepoName;
const changeNum = Number(ctx.params[1]) as NumericChangeId;
- this._redirectOrNavigate({
+ const params: GenerateUrlChangeViewParameters = {
project,
changeNum,
patchNum: convertToPatchSetNum(ctx.params[3]),
view: GerritView.CHANGE,
edit: true,
- });
+ tab: ctx.queryMap.get('tab') ?? '',
+ };
+ if (ctx.queryMap.has('forceReload')) {
+ params.forceReload = true;
+ history.replaceState(
+ null,
+ '',
+ location.href.replace(/[?&]forceReload=true/, '')
+ );
+ }
+ this._redirectOrNavigate(params);
+
this.reporting.setRepoName(project);
this.reporting.setChangeId(changeNum);
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index b91bf0c..7f1a40b 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -312,28 +312,14 @@
changeNum: '1234',
project: 'test',
};
- const paramsWithQuery = {
- view: GerritView.CHANGE,
- changeNum: '1234',
- project: 'test',
- querystring: 'revert&foo=bar',
- };
assert.equal(element._generateUrl(params), '/c/test/+/1234');
- assert.equal(element._generateUrl(paramsWithQuery),
- '/c/test/+/1234?revert&foo=bar');
params.patchNum = 10;
assert.equal(element._generateUrl(params), '/c/test/+/1234/10');
- paramsWithQuery.patchNum = 10;
- assert.equal(element._generateUrl(paramsWithQuery),
- '/c/test/+/1234/10?revert&foo=bar');
params.basePatchNum = 5;
assert.equal(element._generateUrl(params), '/c/test/+/1234/5..10');
- paramsWithQuery.basePatchNum = 5;
- assert.equal(element._generateUrl(paramsWithQuery),
- '/c/test/+/1234/5..10?revert&foo=bar');
params.messageHash = '#123';
assert.equal(element._generateUrl(params), '/c/test/+/1234/5..10#123');
@@ -1382,7 +1368,6 @@
changeNum: 1234,
basePatchNum: 4,
patchNum: 7,
- queryMap: new Map(),
});
assert.isFalse(redirectStub.called);
assert.isTrue(normalizeRangeStub.called);
@@ -1549,6 +1534,7 @@
null,
3, // 3 Patch num
],
+ queryMap: new Map(),
};
const appParams = {
project: 'foo/bar',
@@ -1556,6 +1542,7 @@
view: GerritView.CHANGE,
patchNum: 3,
edit: true,
+ tab: '',
};
element._handleChangeEditRoute(ctx);
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 24ebd67..a295588 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -259,7 +259,14 @@
_viewEditInChangeView() {
if (this._change)
- GerritNav.navigateToChange(this._change, undefined, undefined, true);
+ GerritNav.navigateToChange(
+ this._change,
+ undefined,
+ undefined,
+ true,
+ undefined,
+ true
+ );
}
_getFileData(
diff --git a/polygerrit-ui/app/elements/gr-app-types.ts b/polygerrit-ui/app/elements/gr-app-types.ts
index 6c8bdb9..39070bd 100644
--- a/polygerrit-ui/app/elements/gr-app-types.ts
+++ b/polygerrit-ui/app/elements/gr-app-types.ts
@@ -124,8 +124,9 @@
edit?: boolean;
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
- queryMap?: Map<string, string> | URLSearchParams;
commentId?: UrlEncodedCommentId;
+ forceReload?: boolean;
+ tab?: string;
}
export interface AppElementJustRegisteredParams {
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 391d56f..ed8d792 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -26,6 +26,7 @@
import {CommentsService} from '../services/comments/comments-service';
import {UserService} from '../services/user/user-service';
import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
+import {queryAndAssert, query} from '../utils/common-util';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
export interface MockPromise extends Promise<unknown> {
@@ -167,22 +168,33 @@
document.head.querySelector('#dark-theme')?.remove();
}
+export async function waitQueryAndAssert<E extends Element = Element>(
+ el: Element | null | undefined,
+ selector: string
+): Promise<E> {
+ await waitUntil(
+ () => !!query<E>(el, selector),
+ `The element '${selector}' did not appear in the DOM within 1000 ms.`
+ );
+ return queryAndAssert<E>(el, selector);
+}
+
export function waitUntil(
predicate: () => boolean,
- maxMillis = 100
+ message = 'The waitUntil() predicate is still false after 1000 ms.'
): Promise<void> {
const start = Date.now();
- let sleep = 1;
+ let sleep = 0;
return new Promise((resolve, reject) => {
const waiter = () => {
if (predicate()) {
return resolve();
}
- if (Date.now() - start >= maxMillis) {
- return reject(new Error('Took to long to waitUntil'));
+ if (Date.now() - start >= 1000) {
+ return reject(new Error(message));
}
setTimeout(waiter, sleep);
- sleep *= 2;
+ sleep = sleep === 0 ? 1 : sleep * 4;
};
waiter();
});
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 9e9d0a3..0fd4ca4 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -15,6 +15,7 @@
* limitations under the License.
*/
import {
+ ChangeInfo,
isQuickLabelInfo,
SubmitRequirementResultInfo,
SubmitRequirementStatus,
@@ -28,6 +29,7 @@
LabelNameToInfoMap,
VotingRangeInfo,
} from '../types/common';
+import {ParsedChangeInfo} from '../types/types';
import {assertNever, unique} from './common-util';
// Name of the standard Code-Review label.
@@ -243,6 +245,28 @@
}
}
+/**
+ * Show only applicable.
+ * If there are only legacy requirements, show all legacy requirements.
+ * If there is at least one non-legacy requirement, filter legacy requirements.
+ */
+export function getRequirements(change?: ParsedChangeInfo | ChangeInfo) {
+ let submit_requirements = (change?.submit_requirements ?? []).filter(
+ req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE
+ );
+
+ const hasNonLegacyRequirements = submit_requirements.some(
+ req => req.is_legacy === false
+ );
+ if (hasNonLegacyRequirements) {
+ submit_requirements = submit_requirements.filter(
+ req => req.is_legacy === false
+ );
+ }
+
+ return submit_requirements;
+}
+
// TODO(milutin): This may be temporary for demo purposes
export const PRIORITY_REQUIREMENTS_ORDER: string[] = [
StandardLabels.CODE_REVIEW,
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 2d59294..6af883b 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -24,6 +24,7 @@
getRepresentativeValue,
getVotingRange,
getVotingRangeOrDefault,
+ getRequirements,
hasNeutralStatus,
labelCompare,
LabelStatus,
@@ -38,9 +39,14 @@
} from '../types/common';
import {
createAccountWithEmail,
+ createChange,
createSubmitRequirementExpressionInfo,
createSubmitRequirementResultInfo,
} from '../test/test-data-generators';
+import {
+ SubmitRequirementResultInfo,
+ SubmitRequirementStatus,
+} from '../api/rest-api';
const VALUES_0 = {
'0': 'neutral',
@@ -270,4 +276,47 @@
assert.deepEqual(labels, ['Verified', 'Build-cop-override']);
});
});
+
+ suite('getRequirements()', () => {
+ function createChangeInfoWith(
+ submit_requirements: SubmitRequirementResultInfo[]
+ ) {
+ return {
+ ...createChange(),
+ submit_requirements,
+ };
+ }
+ test('only legacy', () => {
+ const requirement = {
+ ...createSubmitRequirementResultInfo(),
+ is_legacy: true,
+ };
+ const change = createChangeInfoWith([requirement]);
+ assert.deepEqual(getRequirements(change), [requirement]);
+ });
+ test('legacy and non-legacy - filter legacy', () => {
+ const requirement = {
+ ...createSubmitRequirementResultInfo(),
+ is_legacy: true,
+ };
+ const requirement2 = {
+ ...createSubmitRequirementResultInfo(),
+ is_legacy: false,
+ };
+ const change = createChangeInfoWith([requirement, requirement2]);
+ assert.deepEqual(getRequirements(change), [requirement2]);
+ });
+ test('filter not applicable', () => {
+ const requirement = {
+ ...createSubmitRequirementResultInfo(),
+ is_legacy: true,
+ };
+ const requirement2 = {
+ ...createSubmitRequirementResultInfo(),
+ status: SubmitRequirementStatus.NOT_APPLICABLE,
+ };
+ const change = createChangeInfoWith([requirement, requirement2]);
+ assert.deepEqual(getRequirements(change), [requirement]);
+ });
+ });
});
