Merge "Migration of RouterModel to be a non-singleton."
diff --git a/plugins/replication b/plugins/replication
index b6f4011..d9c59a0 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit b6f4011070a8f0bf31fb1158ed12be71374eb47a
+Subproject commit d9c59a0be1c423706b2c4c74c955ebbeca5a894d
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 80608da..d4b0328 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -44,10 +44,10 @@
} from '../../../types/common';
import {assertNever, hasOwnProperty} from '../../../utils/common-util';
import {pluralize} from '../../../utils/string-util';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {
getRequirements,
iconForStatus,
+ showNewSubmitRequirements,
StandardLabels,
} from '../../../utils/label-util';
import {SubmitRequirementStatus} from '../../../api/rest-api';
@@ -113,17 +113,12 @@
@state() private dynamicCellEndpoints?: string[];
- @state() private isSubmitRequirementsUiEnabled = false;
-
reporting: ReportingService = getAppContext().reportingService;
private readonly flagsService = getAppContext().flagsService;
override connectedCallback() {
super.connectedCallback();
- this.isSubmitRequirementsUiEnabled = this.flagsService.isEnabled(
- KnownExperimentId.SUBMIT_REQUIREMENTS_UI
- );
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -537,7 +532,7 @@
}
private renderCommentsInfoWithLabel(labelName: string) {
- if (!this.isSubmitRequirementsUiEnabled) return;
+ if (!showNewSubmitRequirements(this.flagsService, this.change)) return;
if (labelName !== StandardLabels.CODE_REVIEW) return;
if (!this.change?.unresolved_comment_count) return;
return html`<iron-icon
@@ -593,7 +588,7 @@
// private but used in test
computeLabelClass(labelName: string) {
const classes = ['cell', 'label'];
- if (this.isSubmitRequirementsUiEnabled) {
+ if (showNewSubmitRequirements(this.flagsService, this.change)) {
const requirements = getRequirements(this.change).filter(
sr => sr.name === labelName
);
@@ -644,7 +639,7 @@
// private but used in test
computeLabelIcon(labelName: string): string {
- if (this.isSubmitRequirementsUiEnabled) {
+ if (showNewSubmitRequirements(this.flagsService, this.change)) {
const requirements = getRequirements(this.change).filter(
sr => sr.name === labelName
);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 58f9dcc..7d618f5 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -51,8 +51,10 @@
import {fireEvent, fireReload} from '../../../utils/event-util';
import {ScrollMode} from '../../../constants/constants';
import {listen} from '../../../services/shortcuts/shortcuts-service';
-import {KnownExperimentId} from '../../../services/flags/flags';
-import {PRIORITY_REQUIREMENTS_ORDER} from '../../../utils/label-util';
+import {
+ PRIORITY_REQUIREMENTS_ORDER,
+ showNewSubmitRequirements,
+} from '../../../utils/label-util';
import {addGlobalShortcut, Key} from '../../../utils/dom-util';
const NUMBER_FIXED_COLUMNS = 3;
@@ -205,20 +207,22 @@
return column.toLowerCase();
}
- @observe('account', 'preferences', '_config')
+ @observe('account', 'preferences', '_config', 'sections')
_computePreferences(
account?: AccountInfo,
preferences?: PreferencesInput,
- config?: ServerInfo
+ config?: ServerInfo,
+ sections?: ChangeListSection[]
) {
if (!config) {
return;
}
+ const changes = (sections ?? []).map(section => section.results).flat();
this.changeTableColumns = columnNames;
this.showNumber = false;
this.visibleChangeTableColumns = this.changeTableColumns.filter(col =>
- this._isColumnEnabled(col, config, this.flagsService.enabledExperiments)
+ this._isColumnEnabled(col, config, changes)
);
if (account && preferences) {
this.showNumber = !!(
@@ -229,11 +233,7 @@
column === 'Project' ? 'Repo' : column
);
this.visibleChangeTableColumns = prefColumns.filter(col =>
- this._isColumnEnabled(
- col,
- config,
- this.flagsService.enabledExperiments
- )
+ this._isColumnEnabled(col, config, changes)
);
}
}
@@ -242,12 +242,15 @@
/**
* Is the column disabled by a server config or experiment?
*/
- _isColumnEnabled(column: string, config: ServerInfo, experiments: string[]) {
+ _isColumnEnabled(column: string, config: ServerInfo, changes?: ChangeInfo[]) {
if (!columnNames.includes(column)) return false;
if (!config || !config.change) return true;
- if (column === 'Comments') return experiments.includes('comments-column');
+ if (column === 'Comments')
+ return this.flagsService.isEnabled('comments-column');
if (column === 'Requirements')
- return experiments.includes(KnownExperimentId.SUBMIT_REQUIREMENTS_UI);
+ return (changes ?? []).every(change =>
+ showNewSubmitRequirements(this.flagsService, change)
+ );
return true;
}
@@ -302,9 +305,10 @@
labels = labels.concat(currentLabels.filter(nonExistingLabel));
}
}
+ const changes = sections.map(section => section.results).flat();
if (
- this.flagsService.enabledExperiments.includes(
- KnownExperimentId.SUBMIT_REQUIREMENTS_UI
+ (changes ?? []).every(change =>
+ showNewSubmitRequirements(this.flagsService, change)
)
) {
labels = labels.filter(l => PRIORITY_REQUIREMENTS_ORDER.includes(l));
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index 416bd43..d0126fd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -87,7 +87,7 @@
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
import {Interaction} from '../../../constants/reporting';
-import {KnownExperimentId} from '../../../services/flags/flags';
+import {showNewSubmitRequirements} from '../../../utils/label-util';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -213,9 +213,6 @@
@property({type: Object})
queryTopic?: AutocompleteQuery;
- @property({type: Boolean})
- _isSubmitRequirementsUiEnabled = false;
-
restApiService = getAppContext().restApiService;
private readonly reporting = getAppContext().reportingService;
@@ -225,9 +222,6 @@
override ready() {
super.ready();
this.queryTopic = (input: string) => this._getTopicSuggestions(input);
- this._isSubmitRequirementsUiEnabled = this.flagsService.isEnabled(
- KnownExperimentId.SUBMIT_REQUIREMENTS_UI
- );
}
@observe('change.labels')
@@ -713,13 +707,7 @@
}
_showNewSubmitRequirements(change?: ParsedChangeInfo) {
- if (!this._isSubmitRequirementsUiEnabled) return false;
- return (change?.submit_requirements ?? []).length > 0;
- }
-
- _showNewSubmitRequirementWarning(change?: ParsedChangeInfo) {
- if (!this._isSubmitRequirementsUiEnabled) return false;
- return (change?.submit_requirements ?? []).length === 0;
+ return showNewSubmitRequirements(this.flagsService, change);
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index 15631e4..46303a9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -113,10 +113,6 @@
--iron-icon-height: 18px;
--iron-icon-width: 18px;
}
- .submit-requirement-error {
- color: var(--deemphasized-text-color);
- padding-left: var(--metadata-horizontal-padding);
- }
</style>
<gr-external-style id="externalStyle" name="change-metadata">
<div class="metadata-header">
@@ -479,11 +475,6 @@
mutable="[[_mutable]]"
></gr-change-requirements>
</template>
- <template is="dom-if" if="[[_showNewSubmitRequirementWarning(change)]]">
- <div class="submit-requirement-error">
- New Submit Requirements don't work on this change.
- </div>
- </template>
</div>
<section
id="webLinks"
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_html.ts b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_html.ts
index 6991c03..8161592 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_html.ts
@@ -154,7 +154,6 @@
mutable="[[mutable]]"
label="[[item.labelName]]"
label-info="[[item.labelInfo]]"
- showAlwaysOldUI
></gr-label-info>
</div>
</section>
@@ -205,7 +204,6 @@
mutable="[[mutable]]"
label="[[item.labelName]]"
label-info="[[item.labelInfo]]"
- showAlwaysOldUI
></gr-label-info>
</div>
</section>
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 c83e610..1d66aa5 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
@@ -882,6 +882,13 @@
this._tabState = e.detail.tabState;
}
+ /**
+ * Currently there is a bug in this code where this.unresolvedOnly is only
+ * assigned the correct value when _onPaperTabClick is triggered which is
+ * only triggered when user explicitly clicks on the tab however the comments
+ * tab can also be opened via the url in which case the correct value to
+ * unresolvedOnly is never assigned.
+ */
_onPaperTabClick(e: MouseEvent) {
let target = e.target as HTMLElement | null;
let tabName: string | undefined;
@@ -893,7 +900,8 @@
} while (target);
if (tabName === PrimaryTab.COMMENT_THREADS) {
- // Show unresolved threads by default only if they are present
+ // Show unresolved threads by default
+ // Show resolved threads only if no unresolved threads exist
const hasUnresolvedThreads =
(this._commentThreads ?? []).filter(thread => isUnresolved(thread))
.length > 0;
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 0e74cc6..e8cce2d 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
@@ -17,6 +17,7 @@
import '../../../test/common-test-setup-karma';
import '../../edit/gr-edit-constants';
+import '../gr-thread-list/gr-thread-list';
import './gr-change-view';
import {
ChangeStatus,
@@ -107,6 +108,7 @@
import {FocusTarget, GrReplyDialog} from '../gr-reply-dialog/gr-reply-dialog';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrChangeStar} from '../../shared/gr-change-star/gr-change-star';
+import {GrThreadList} from '../gr-thread-list/gr-thread-list';
const fixture = fixtureFromElement('gr-change-view');
@@ -862,7 +864,43 @@
});
});
- suite('Findings comment tab', () => {
+ suite('Comments tab', () => {
+ setup(async () => {
+ element._changeNum = TEST_NUMERIC_CHANGE_ID;
+ element._change = {
+ ...createChangeViewChange(),
+ revisions: {
+ rev2: createRevision(2),
+ rev1: createRevision(1),
+ rev13: createRevision(13),
+ rev3: createRevision(3),
+ rev4: createRevision(4),
+ },
+ current_revision: 'rev4' as CommitId,
+ };
+ element._commentThreads = THREADS;
+ await flush();
+ const paperTabs = element.shadowRoot!.querySelector('#primaryTabs')!;
+ tap(paperTabs.querySelectorAll('paper-tab')[1]);
+ await flush();
+ });
+
+ test('commentId overrides unresolveOnly default', async () => {
+ const threadList = queryAndAssert<GrThreadList>(
+ element,
+ 'gr-thread-list'
+ );
+ assert.isTrue(element.unresolvedOnly);
+ assert.isNotOk(element.scrollCommentId);
+ assert.isTrue(threadList.unresolvedOnly);
+
+ element.scrollCommentId = 'abcd' as UrlEncodedCommentId;
+ await flush();
+ assert.isFalse(threadList.unresolvedOnly);
+ });
+ });
+
+ suite('Findings robot-comment tab', () => {
setup(async () => {
element._changeNum = TEST_NUMERIC_CHANGE_ID;
element._change = {
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
index 3c4702c..be04e6e 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
@@ -33,10 +33,13 @@
LabelValuesMap,
} from '../gr-label-score-row/gr-label-score-row';
import {getAppContext} from '../../../services/app-context';
-import {getTriggerVotes, labelCompare} from '../../../utils/label-util';
+import {
+ getTriggerVotes,
+ labelCompare,
+ showNewSubmitRequirements,
+} from '../../../utils/label-util';
import {Execution} from '../../../constants/reporting';
import {ChangeStatus} from '../../../constants/constants';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {fontStyles} from '../../../styles/gr-font-styles';
@customElement('gr-label-scores')
@@ -90,7 +93,7 @@
}
override render() {
- if (this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI)) {
+ if (showNewSubmitRequirements(this.flagsService, this.change)) {
return this.renderNewSubmitRequirements();
} else {
return this.renderOldSubmitRequirements();
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 0349b3f..490162b 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -212,6 +212,7 @@
override willUpdate(changed: PropertyValues) {
if (changed.has('commentTabState')) this.onCommentTabStateUpdate();
+ if (changed.has('scrollCommentId')) this.onScrollCommentIdUpdate();
}
private onCommentTabStateUpdate() {
@@ -228,6 +229,14 @@
}
}
+ /**
+ * When user wants to scroll to a comment, render all comments so that the
+ * appropriate comment can be scrolled into view.
+ */
+ private onScrollCommentIdUpdate() {
+ if (this.scrollCommentId) this.handleAllComments();
+ }
+
static override get styles() {
return [
sharedStyles,
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts
index 8b47437..0a9fbbf 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts
@@ -24,6 +24,7 @@
import {ServerInfo} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {columnNames} from '../../change-list/gr-change-list/gr-change-list';
+import {KnownExperimentId} from '../../../services/flags/flags';
@customElement('gr-change-table-editor')
export class GrChangeTableEditor extends PolymerElement {
@@ -48,24 +49,25 @@
@observe('serverConfig')
_configChanged(config: ServerInfo) {
this.defaultColumns = columnNames.filter(col =>
- this._isColumnEnabled(col, config, this.flagsService.enabledExperiments)
+ this._isColumnEnabled(col, config)
);
if (!this.displayedColumns) return;
this.displayedColumns = this.displayedColumns.filter(column =>
- this._isColumnEnabled(
- column,
- config,
- this.flagsService.enabledExperiments
- )
+ this._isColumnEnabled(column, config)
);
}
/**
* Is the column disabled by a server config or experiment?
*/
- _isColumnEnabled(column: string, config: ServerInfo, experiments: string[]) {
+ _isColumnEnabled(column: string, config: ServerInfo) {
if (!config || !config.change) return true;
- if (column === 'Comments') return experiments.includes('comments-column');
+ if (column === 'Comments')
+ return this.flagsService.isEnabled('comments-column');
+ if (column === 'Requirements')
+ return this.flagsService.isEnabled(
+ KnownExperimentId.SUBMIT_REQUIREMENTS_UI
+ );
return true;
}
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts
index 3813213..c2bcec2 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts
@@ -104,7 +104,7 @@
test('_getDisplayedColumns', () => {
const enabledColumns = columns.filter(column =>
- element._isColumnEnabled(column, element.serverConfig!, [])
+ element._isColumnEnabled(column, element.serverConfig!)
);
assert.deepEqual(element._getDisplayedColumns(), enabledColumns);
const input = queryAndAssert<HTMLInputElement>(
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
index 888f477..70de130 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
@@ -44,6 +44,7 @@
getVotingRangeOrDefault,
hasNeutralStatus,
hasVoted,
+ showNewSubmitRequirements,
valueString,
} from '../../../utils/label-util';
import {getAppContext} from '../../../services/app-context';
@@ -53,7 +54,6 @@
import {votingStyles} from '../../../styles/gr-voting-styles';
import {ifDefined} from 'lit/directives/if-defined';
import {fireReload} from '../../../utils/event-util';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {sortReviewers} from '../../../utils/attention-set-util';
declare global {
@@ -104,10 +104,6 @@
@property({type: Boolean})
showAllReviewers = true;
- /** temporary until submit requirements are finished */
- @property({type: Boolean})
- showAlwaysOldUI = false;
-
private readonly restApiService = getAppContext().restApiService;
private readonly reporting = getAppContext().reportingService;
@@ -214,10 +210,7 @@
}
override render() {
- if (
- this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI) &&
- !this.showAlwaysOldUI
- ) {
+ if (showNewSubmitRequirements(this.flagsService, this.change)) {
return this.renderNewSubmitRequirements();
} else {
return this.renderOldSubmitRequirements();
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 57d2b0c..da4601b 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -741,6 +741,7 @@
name: 'Verified',
status: SubmitRequirementStatus.SATISFIED,
submittability_expression_result: createSubmitRequirementExpressionInfo(),
+ is_legacy: false,
};
}
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 8904cac..c154958 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -20,6 +20,7 @@
SubmitRequirementResultInfo,
SubmitRequirementStatus,
} from '../api/rest-api';
+import {FlagsService, KnownExperimentId} from '../services/flags/flags';
import {
AccountInfo,
ApprovalInfo,
@@ -262,20 +263,9 @@
* 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;
+ return (change?.submit_requirements ?? [])
+ .filter(req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE)
+ .filter(req => req.is_legacy === false);
}
// TODO(milutin): This may be temporary for demo purposes
@@ -309,3 +299,16 @@
label => !labelAssociatedWithSubmitReqs.includes(label)
);
}
+
+export function showNewSubmitRequirements(
+ flagsService: FlagsService,
+ change?: ParsedChangeInfo | ChangeInfo
+) {
+ const isSubmitRequirementsUiEnabled = flagsService.isEnabled(
+ KnownExperimentId.SUBMIT_REQUIREMENTS_UI
+ );
+ if (!isSubmitRequirementsUiEnabled) return false;
+ if ((getRequirements(change) ?? []).length === 0) return false;
+
+ return true;
+}
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 6cb04af..1b8ad0c 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -298,7 +298,7 @@
is_legacy: true,
};
const change = createChangeInfoWith([requirement]);
- assert.deepEqual(getRequirements(change), [requirement]);
+ assert.deepEqual(getRequirements(change), []);
});
test('legacy and non-legacy - filter legacy', () => {
const requirement = {
@@ -313,10 +313,7 @@
assert.deepEqual(getRequirements(change), [requirement2]);
});
test('filter not applicable', () => {
- const requirement = {
- ...createSubmitRequirementResultInfo(),
- is_legacy: true,
- };
+ const requirement = createSubmitRequirementResultInfo();
const requirement2 = {
...createSubmitRequirementResultInfo(),
status: SubmitRequirementStatus.NOT_APPLICABLE,
@@ -348,6 +345,7 @@
...createSubmitRequirementExpressionInfo(),
expression: `label:${triggerVote}=MAX`,
},
+ is_legacy: false,
},
],
labels: {
