Merge "Remove old submit requirements from change-list"
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 933b300..b4ebd3c 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
@@ -40,12 +40,9 @@
ChangeInfo,
ServerInfo,
AccountInfo,
- QuickLabelInfo,
Timestamp,
} from '../../../types/common';
import {hasOwnProperty, assertIsDefined} from '../../../utils/common-util';
-import {pluralize} from '../../../utils/string-util';
-import {showNewSubmitRequirements} from '../../../utils/label-util';
import {changeListStyles} from '../../../styles/gr-change-list-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, css, html} from 'lit';
@@ -248,22 +245,6 @@
.subject:hover .content {
text-decoration: underline;
}
- .u-monospace {
- font-family: var(--monospace-font-family);
- font-size: var(--font-size-mono);
- line-height: var(--line-height-mono);
- }
- .u-green,
- .u-green iron-icon {
- color: var(--positive-green-text-color);
- }
- .u-red,
- .u-red iron-icon {
- color: var(--negative-red-text-color);
- }
- .u-gray-background {
- background-color: var(--table-header-background-color);
- }
.comma,
.placeholder {
color: var(--deemphasized-text-color);
@@ -618,32 +599,13 @@
}
private renderChangeLabels(labelName: string) {
- if (showNewSubmitRequirements(this.flagsService, this.change)) {
- return html` <td class="cell label requirement">
- <gr-change-list-column-requirement
- .change=${this.change}
- .labelName=${labelName}
- >
- </gr-change-list-column-requirement>
- </td>`;
- }
- return html`
- <td
- title=${this.computeLabelTitle(labelName)}
- class=${this.computeLabelClass(labelName)}
+ return html` <td class="cell label requirement">
+ <gr-change-list-column-requirement
+ .change=${this.change}
+ .labelName=${labelName}
>
- ${this.renderChangeHasLabelIcon(labelName)}
- </td>
- `;
- }
-
- private renderChangeHasLabelIcon(labelName: string) {
- if (this.computeLabelIcon(labelName) === '')
- return html`<span>${this.computeLabelValue(labelName)}</span>`;
-
- return html`
- <iron-icon icon=${this.computeLabelIcon(labelName)}></iron-icon>
- `;
+ </gr-change-list-column-requirement>
+ </td>`;
}
private renderChangePluginEndpoint(pluginEndpointName: string) {
@@ -676,118 +638,6 @@
return GerritNav.getUrlForChange(this.change);
}
- // private but used in test
- computeLabelTitle(labelName: string) {
- const label: QuickLabelInfo | undefined = this.change?.labels?.[labelName];
- const category = this.computeLabelCategory(labelName);
- if (!label || category === LabelCategory.NOT_APPLICABLE) {
- return 'Label not applicable';
- }
- const titleParts: string[] = [];
- if (category === LabelCategory.UNRESOLVED_COMMENTS) {
- const num = this.change?.unresolved_comment_count ?? 0;
- titleParts.push(pluralize(num, 'unresolved comment'));
- }
- const significantLabel =
- label.rejected || label.approved || label.disliked || label.recommended;
- if (significantLabel?.name) {
- titleParts.push(`${labelName} by ${significantLabel.name}`);
- }
- if (titleParts.length > 0) {
- return titleParts.join(',\n');
- }
- return labelName;
- }
-
- // private but used in test
- computeLabelClass(labelName: string) {
- const classes = ['cell', 'label'];
- const category = this.computeLabelCategory(labelName);
- switch (category) {
- case LabelCategory.NOT_APPLICABLE:
- classes.push('u-gray-background');
- break;
- case LabelCategory.APPROVED:
- classes.push('u-green');
- break;
- case LabelCategory.POSITIVE:
- classes.push('u-monospace');
- classes.push('u-green');
- break;
- case LabelCategory.NEGATIVE:
- classes.push('u-monospace');
- classes.push('u-red');
- break;
- case LabelCategory.REJECTED:
- classes.push('u-red');
- break;
- }
- return classes.sort().join(' ');
- }
-
- // private but used in test
- computeLabelIcon(labelName: string): string {
- const category = this.computeLabelCategory(labelName);
- switch (category) {
- case LabelCategory.APPROVED:
- return 'gr-icons:check';
- case LabelCategory.UNRESOLVED_COMMENTS:
- return 'gr-icons:comment';
- case LabelCategory.REJECTED:
- return 'gr-icons:close';
- default:
- return '';
- }
- }
-
- // private but used in test
- computeLabelCategory(labelName: string) {
- const label: QuickLabelInfo | undefined = this.change?.labels?.[labelName];
- if (!label) {
- return LabelCategory.NOT_APPLICABLE;
- }
- if (label.rejected) {
- return LabelCategory.REJECTED;
- }
- if (label.value && label.value < 0) {
- return LabelCategory.NEGATIVE;
- }
- if (this.change?.unresolved_comment_count && labelName === 'Code-Review') {
- return LabelCategory.UNRESOLVED_COMMENTS;
- }
- if (label.approved) {
- return LabelCategory.APPROVED;
- }
- if (label.value && label.value > 0) {
- return LabelCategory.POSITIVE;
- }
- return LabelCategory.NEUTRAL;
- }
-
- // private but used in test
- computeLabelValue(labelName: string) {
- const label: QuickLabelInfo | undefined = this.change?.labels?.[labelName];
- const category = this.computeLabelCategory(labelName);
- switch (category) {
- case LabelCategory.NOT_APPLICABLE:
- return '';
- case LabelCategory.APPROVED:
- return '\u2713'; // ✓
- case LabelCategory.POSITIVE:
- return `+${label?.value}`;
- case LabelCategory.NEUTRAL:
- return '';
- case LabelCategory.UNRESOLVED_COMMENTS:
- return 'u';
- case LabelCategory.NEGATIVE:
- return `${label?.value}`;
- case LabelCategory.REJECTED:
- return '\u2715'; // ✕
- default:
- return '';
- }
- }
-
private computeRepoUrl() {
if (!this.change) return '';
return GerritNav.getUrlForProjectChanges(
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index e0ce6a8..7216249 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -48,7 +48,7 @@
import {StandardLabels} from '../../../utils/label-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import './gr-change-list-item';
-import {GrChangeListItem, LabelCategory} from './gr-change-list-item';
+import {GrChangeListItem} from './gr-change-list-item';
import {
DIProviderElement,
wrapInProvider,
@@ -92,209 +92,6 @@
await element.updateComplete;
});
- test('computeLabelCategory', () => {
- element.change = {
- ...change,
- labels: {},
- };
- assert.equal(
- element.computeLabelCategory('Verified'),
- LabelCategory.NOT_APPLICABLE
- );
- element.change.labels = {Verified: {approved: account, value: 1}};
- assert.equal(
- element.computeLabelCategory('Verified'),
- LabelCategory.APPROVED
- );
- element.change.labels = {Verified: {rejected: account, value: -1}};
- assert.equal(
- element.computeLabelCategory('Verified'),
- LabelCategory.REJECTED
- );
- element.change.labels = {'Code-Review': {approved: account, value: 1}};
- element.change.unresolved_comment_count = 1;
- assert.equal(
- element.computeLabelCategory('Code-Review'),
- LabelCategory.UNRESOLVED_COMMENTS
- );
- element.change.labels = {'Code-Review': {value: 1}};
- element.change.unresolved_comment_count = 0;
- assert.equal(
- element.computeLabelCategory('Code-Review'),
- LabelCategory.POSITIVE
- );
- element.change.labels = {'Code-Review': {value: -1}};
- assert.equal(
- element.computeLabelCategory('Code-Review'),
- LabelCategory.NEGATIVE
- );
- element.change.labels = {'Code-Review': {value: -1}};
- assert.equal(
- element.computeLabelCategory('Verified'),
- LabelCategory.NOT_APPLICABLE
- );
- });
-
- test('computeLabelClass', () => {
- element.change = {
- ...change,
- labels: {},
- };
- assert.equal(
- element.computeLabelClass('Verified'),
- 'cell label u-gray-background'
- );
- element.change.labels = {Verified: {approved: account, value: 1}};
- assert.equal(element.computeLabelClass('Verified'), 'cell label u-green');
- element.change.labels = {Verified: {rejected: account, value: -1}};
- assert.equal(element.computeLabelClass('Verified'), 'cell label u-red');
- element.change.labels = {'Code-Review': {value: 1}};
- assert.equal(
- element.computeLabelClass('Code-Review'),
- 'cell label u-green u-monospace'
- );
- element.change.labels = {'Code-Review': {value: -1}};
- assert.equal(
- element.computeLabelClass('Code-Review'),
- 'cell label u-monospace u-red'
- );
- element.change.labels = {'Code-Review': {value: -1}};
- assert.equal(
- element.computeLabelClass('Verified'),
- 'cell label u-gray-background'
- );
- });
-
- test('computeLabelTitle', () => {
- element.change = {
- ...change,
- labels: {},
- };
- assert.equal(element.computeLabelTitle('Verified'), 'Label not applicable');
-
- element.change.labels = {Verified: {approved: {name: 'Diffy'}}};
- assert.equal(element.computeLabelTitle('Verified'), 'Verified by Diffy');
-
- element.change.labels = {Verified: {approved: {name: 'Diffy'}}};
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Label not applicable'
- );
-
- element.change.labels = {Verified: {rejected: {name: 'Diffy'}}};
- assert.equal(element.computeLabelTitle('Verified'), 'Verified by Diffy');
-
- element.change.labels = {
- 'Code-Review': {disliked: {name: 'Diffy'}, value: -1},
- };
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Code-Review by Diffy'
- );
-
- element.change.labels = {
- 'Code-Review': {recommended: {name: 'Diffy'}, value: 1},
- };
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Code-Review by Diffy'
- );
-
- element.change.labels = {
- 'Code-Review': {recommended: {name: 'Diffy'}, rejected: {name: 'Admin'}},
- };
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Code-Review by Admin'
- );
-
- element.change.labels = {
- 'Code-Review': {approved: {name: 'Diffy'}, rejected: {name: 'Admin'}},
- };
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Code-Review by Admin'
- );
-
- element.change.labels = {
- 'Code-Review': {
- recommended: {name: 'Diffy'},
- disliked: {name: 'Admin'},
- value: -1,
- },
- };
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Code-Review by Admin'
- );
-
- element.change.labels = {
- 'Code-Review': {
- approved: {name: 'Diffy'},
- disliked: {name: 'Admin'},
- value: -1,
- },
- };
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- 'Code-Review by Diffy'
- );
-
- element.change.labels = {'Code-Review': {approved: account, value: 1}};
- element.change.unresolved_comment_count = 1;
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- '1 unresolved comment'
- );
-
- element.change.labels = {
- 'Code-Review': {approved: {name: 'Diffy'}, value: 1},
- };
- element.change.unresolved_comment_count = 1;
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- '1 unresolved comment,\nCode-Review by Diffy'
- );
-
- element.change.labels = {'Code-Review': {approved: account, value: 1}};
- element.change.unresolved_comment_count = 2;
- assert.equal(
- element.computeLabelTitle('Code-Review'),
- '2 unresolved comments'
- );
- });
-
- test('computeLabelIcon', () => {
- element.change = {
- ...change,
- labels: {},
- };
- assert.equal(element.computeLabelIcon('missingLabel'), '');
- element.change.labels = {Verified: {approved: account, value: 1}};
- assert.equal(element.computeLabelIcon('Verified'), 'gr-icons:check');
- element.change.labels = {'Code-Review': {approved: account, value: 1}};
- element.change.unresolved_comment_count = 1;
- assert.equal(element.computeLabelIcon('Code-Review'), 'gr-icons:comment');
- });
-
- test('computeLabelValue', () => {
- element.change = {
- ...change,
- labels: {},
- };
- assert.equal(element.computeLabelValue('Verified'), '');
- element.change.labels = {Verified: {approved: account, value: 1}};
- assert.equal(element.computeLabelValue('Verified'), '✓');
- element.change.labels = {Verified: {value: 1}};
- assert.equal(element.computeLabelValue('Verified'), '+1');
- element.change.labels = {Verified: {value: -1}};
- assert.equal(element.computeLabelValue('Verified'), '-1');
- element.change.labels = {Verified: {approved: account}};
- assert.equal(element.computeLabelValue('Verified'), '✓');
- element.change.labels = {Verified: {rejected: account}};
- assert.equal(element.computeLabelValue('Verified'), '✕');
- });
-
test('no hidden columns', async () => {
element.visibleChangeTableColumns = [
ColumnNames.SUBJECT,
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 2a90bfc..2f5965b 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
@@ -45,7 +45,6 @@
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {queryAll} from '../../../utils/common-util';
import {ValueChangedEvent} from '../../../types/events';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
import {Execution} from '../../../constants/reporting';
@@ -309,11 +308,7 @@
const prefColumns = this.preferences.change_table
.map(column => (column === 'Project' ? ColumnNames.REPO : column))
.map(column =>
- this.flagsService.isEnabled(
- KnownExperimentId.SUBMIT_REQUIREMENTS_UI
- ) && column === ColumnNames.STATUS
- ? ColumnNames.STATUS2
- : column
+ column === ColumnNames.STATUS ? ColumnNames.STATUS2 : column
);
this.reporting.reportExecution(Execution.USER_PREFERENCES_COLUMNS, {
statusColumn: prefColumns.includes(ColumnNames.STATUS2),
@@ -336,50 +331,23 @@
if (!config || !config.change) return true;
if (column === 'Comments')
return this.flagsService.isEnabled('comments-column');
- if (column === 'Status') {
- return !this.flagsService.isEnabled(
- KnownExperimentId.SUBMIT_REQUIREMENTS_UI
- );
- }
- if (column === ColumnNames.STATUS2)
- return this.flagsService.isEnabled(
- KnownExperimentId.SUBMIT_REQUIREMENTS_UI
- );
+ if (column === 'Status') return false;
+ if (column === ColumnNames.STATUS2) return true;
return true;
}
// private but used in test
computeLabelNames(sections: ChangeListSection[]) {
if (!sections) return [];
- let labels: string[] = [];
- const nonExistingLabel = function (item: string) {
- return !labels.includes(item);
- };
- for (const section of sections) {
- if (!section.results) {
- continue;
- }
- for (const change of section.results) {
- if (!change.labels) {
- continue;
- }
- const currentLabels = Object.keys(change.labels);
- labels = labels.concat(currentLabels.filter(nonExistingLabel));
- }
+ if (this.config?.submit_requirement_dashboard_columns?.length) {
+ return this.config?.submit_requirement_dashboard_columns;
}
-
- if (this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI)) {
- if (this.config?.submit_requirement_dashboard_columns?.length) {
- return this.config?.submit_requirement_dashboard_columns;
- } else {
- const changes = sections.map(section => section.results).flat();
- labels = (changes ?? [])
- .map(change => getRequirements(change))
- .flat()
- .map(requirement => requirement.name)
- .filter(unique);
- }
- }
+ const changes = sections.map(section => section.results).flat();
+ const labels = (changes ?? [])
+ .map(change => getRequirements(change))
+ .flat()
+ .map(requirement => requirement.name)
+ .filter(unique);
return labels.sort();
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index 63a6d8f..1e81123 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -32,6 +32,7 @@
import {
createChange,
createServerInfo,
+ createSubmitRequirementResultInfo,
} from '../../../test/test-data-generators';
import {GrChangeListItem} from '../gr-change-list-item/gr-change-list-item';
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
@@ -165,23 +166,36 @@
{
...createChange(),
_number: 0 as NumericChangeId,
- labels: {Verified: {approved: {}}},
+ submit_requirements: [
+ {
+ ...createSubmitRequirementResultInfo(),
+ name: 'Verified',
+ },
+ ],
},
{
...createChange(),
_number: 1 as NumericChangeId,
- labels: {
- Verified: {approved: {}},
- 'Code-Review': {approved: {}},
- },
+ submit_requirements: [
+ {
+ ...createSubmitRequirementResultInfo(),
+ name: 'Verified',
+ },
+ {
+ ...createSubmitRequirementResultInfo(),
+ name: 'Code-Review',
+ },
+ ],
},
{
...createChange(),
_number: 2 as NumericChangeId,
- labels: {
- Verified: {approved: {}},
- 'Library-Compliance': {approved: {}},
- },
+ submit_requirements: [
+ {
+ ...createSubmitRequirementResultInfo(),
+ name: 'Library-Compliance',
+ },
+ ],
},
],
},
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index f5703f4..2b6f700 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -21,7 +21,6 @@
SubmitRequirementStatus,
LabelNameToValuesMap,
} from '../api/rest-api';
-import {FlagsService, KnownExperimentId} from '../services/flags/flags';
import {
AccountInfo,
ApprovalInfo,
@@ -421,16 +420,3 @@
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;
-}
