Merge changes I73d72ae7,I5f1847f6,I7f1ec31b
* changes:
Add an endpoint to test submit requirements on a change
Add "ERROR" to the SubmitRequirementResultInfo#Status field
Refactor SubmitRequirementsJson into a separate class
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index ee579ff..905d6be 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -55,7 +55,7 @@
/**
* Represents a "generic" text range in the code (e.g. text selection)
*/
-interface TextRange {
+export declare interface TextRange {
/** first line of the range (1-based inclusive). */
start_line: number;
/** first column of the range (in the first line) (1-based inclusive). */
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
index 61785fc..e3edb5e 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
@@ -31,7 +31,6 @@
ApprovalInfo,
Reviewers,
AccountId,
- DetailedLabelInfo,
EmailAddress,
AccountDetailInfo,
isDetailedLabelInfo,
@@ -158,24 +157,19 @@
if (!change.labels) {
return NaN;
}
- const detailedLabel = change.labels[label] as DetailedLabelInfo;
- if (!detailedLabel.all) {
+ const detailedLabel = change.labels[label];
+ if (!isDetailedLabelInfo(detailedLabel) || !detailedLabel.all) {
return NaN;
}
- const detailed = detailedLabel.all
- .filter(
- (approval: ApprovalInfo) =>
- reviewer._account_id === approval._account_id
- )
- .pop();
- if (!detailed) {
+ const approvalInfo = getApprovalInfo(detailedLabel, reviewer);
+ if (!approvalInfo) {
return NaN;
}
- if (hasOwnProperty(detailed, 'permitted_voting_range')) {
- if (!detailed.permitted_voting_range) return NaN;
- return detailed.permitted_voting_range.max;
- } else if (hasOwnProperty(detailed, 'value')) {
- // If preset, user can vote on the label.
+ if (hasOwnProperty(approvalInfo, 'permitted_voting_range')) {
+ if (!approvalInfo.permitted_voting_range) return NaN;
+ return approvalInfo.permitted_voting_range.max;
+ } else if (hasOwnProperty(approvalInfo, 'value')) {
+ // If present, user can vote on the label.
return 0;
}
return NaN;
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
index d3f23ad..bf15bb5 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
@@ -26,6 +26,7 @@
import {
createAccountDetailWithId,
createChange,
+ createDetailedLabelInfo,
} from '../../../test/test-data-generators';
import {tap} from '@polymer/iron-test-helpers/mock-interactions';
import {GrButton} from '../../shared/gr-button/gr-button';
@@ -423,6 +424,7 @@
...createChange(),
labels: {
Foo: {
+ ...createDetailedLabelInfo(),
all: [
{
_account_id: 7 as AccountId,
@@ -431,6 +433,7 @@
],
},
Bar: {
+ ...createDetailedLabelInfo(),
all: [
{
...createAccountDetailWithId(1),
@@ -443,6 +446,7 @@
],
},
FooBar: {
+ ...createDetailedLabelInfo(),
all: [{_account_id: 7 as AccountId, value: 0}],
},
},
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 3c9f54c..d90d173 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -120,6 +120,9 @@
margin-top: var(--spacing-m);
padding: var(--spacing-m) var(--spacing-xl) 0;
}
+ .status-placeholder {
+ visibility: hidden;
+ }
`,
];
}
@@ -155,31 +158,27 @@
private renderLabelSection() {
const labels = this.computeLabels();
+ const showLabelName = labels.length >= 2;
return html` <div class="section">
- ${labels.map(l => this.renderLabel(l))}
+ <div class="sectionIcon"></div>
+ <div class="row">
+ <!-- Hidden placeholder to be aligned as Status line above -->
+ <div class="title status-placeholder">Status</div>
+ <div>${labels.map(l => this.renderLabel(l, showLabelName))}</div>
+ </div>
</div>`;
}
- private renderLabel(label: Label) {
+ private renderLabel(label: Label, showLabelName: boolean) {
return html`
- <section class="label">
- <div class="label-title">
- <gr-limited-text
- class="name"
- limit="25"
- text="${label.labelName}"
- ></gr-limited-text>
- </div>
- <div class="label-value">
- <gr-label-info
- .change=${this.change}
- .account=${this.account}
- .mutable=${this.mutable}
- .label="${label.labelName}"
- .labelInfo="${label.labelInfo}"
- ></gr-label-info>
- </div>
- </section>
+ ${showLabelName ? html`<div>${label.labelName} votes</div>` : ''}
+ <gr-label-info
+ .change=${this.change}
+ .account=${this.account}
+ .mutable=${this.mutable}
+ .label="${label.labelName}"
+ .labelInfo="${label.labelInfo}"
+ ></gr-label-info>
`;
}
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 2df2ccb..f60b9b6 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
@@ -17,8 +17,10 @@
import '../../../styles/gr-font-styles';
import '../../../styles/gr-voting-styles';
import '../../../styles/shared-styles';
+import '../gr-vote-chip/gr-vote-chip';
import '../gr-account-label/gr-account-label';
import '../gr-account-link/gr-account-link';
+import '../gr-account-chip/gr-account-chip';
import '../gr-button/gr-button';
import '../gr-icons/gr-icons';
import '../gr-label/gr-label';
@@ -36,7 +38,12 @@
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
import {GrButton} from '../gr-button/gr-button';
-import {getVotingRangeOrDefault} from '../../../utils/label-util';
+import {
+ canVote,
+ getApprovalInfo,
+ getVotingRangeOrDefault,
+ hasNeutralStatus,
+} from '../../../utils/label-util';
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
import {fontStyles} from '../../../styles/gr-font-styles';
@@ -44,6 +51,7 @@
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';
declare global {
interface HTMLElementTagNameMap {
@@ -160,28 +168,83 @@
.labelValueContainer:not(:first-of-type) td {
padding-top: var(--spacing-s);
}
+ .reviewer-row {
+ padding-top: var(--spacing-s);
+ }
+ .reviewer-row:first-of-type {
+ padding-top: 0;
+ }
+ .reviewer-row gr-account-chip,
+ .reviewer-row gr-tooltip-content {
+ display: inline-block;
+ vertical-align: top;
+ }
+ .reviewer-row .no-votes {
+ color: var(--deemphasized-text-color);
+ margin-left: var(--spacing-xs);
+ }
`,
];
}
+ private readonly flagsService = appContext.flagsService;
+
override render() {
+ if (this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI)) {
+ return this.renderNewSubmitRequirements();
+ } else {
+ return this.renderOldSubmitRequirements();
+ }
+ }
+
+ private renderNewSubmitRequirements() {
+ const labelInfo = this.labelInfo;
+ if (!labelInfo) return;
+ const reviewers = (this.change?.reviewers['REVIEWER'] ?? []).filter(
+ reviewer => canVote(labelInfo, reviewer)
+ );
+ return html`<div>
+ ${reviewers.map(reviewer => this.renderReviewerVote(reviewer))}
+ </div>`;
+ }
+
+ private renderOldSubmitRequirements() {
+ const labelInfo = this.labelInfo;
return html` <p
class="placeholder ${this.computeShowPlaceholder(
- this.labelInfo,
+ labelInfo,
this.change?.labels
)}"
>
No votes
</p>
<table>
- ${this.mapLabelInfo(
- this.labelInfo,
- this.account,
- this.change?.labels
- ).map(mappedLabel => this.renderLabel(mappedLabel))}
+ ${this.mapLabelInfo(labelInfo, this.account, this.change?.labels).map(
+ mappedLabel => this.renderLabel(mappedLabel)
+ )}
</table>`;
}
+ renderReviewerVote(reviewer: AccountInfo) {
+ const labelInfo = this.labelInfo;
+ if (!labelInfo || !isDetailedLabelInfo(labelInfo)) return;
+ const approvalInfo = getApprovalInfo(labelInfo, reviewer);
+ const noVoteYet =
+ !approvalInfo || hasNeutralStatus(labelInfo, approvalInfo);
+ return html`<div class="reviewer-row">
+ <gr-account-chip .account="${reviewer}" .change="${this.change}">
+ <gr-vote-chip
+ slot="vote-chip"
+ .vote="${approvalInfo}"
+ .label="${labelInfo}"
+ ></gr-vote-chip
+ ></gr-account-chip>
+ ${noVoteYet
+ ? html`<span class="no-votes">No votes</span>`
+ : html`${this.renderRemoveVote(reviewer)}`}
+ </div>`;
+ }
+
renderLabel(mappedLabel: FormattedLabel) {
const {labelInfo, change} = this;
return html` <tr class="labelValueContainer">
@@ -201,26 +264,28 @@
.change="${change}"
></gr-account-link>
</td>
- <td>
- <gr-tooltip-content has-tooltip title="Remove vote">
- <gr-button
- link
- aria-label="Remove vote"
- @click="${this.onDeleteVote}"
- data-account-id="${ifDefined(mappedLabel.account._account_id)}"
- class="deleteBtn ${this.computeDeleteClass(
- mappedLabel.account,
- this.mutable,
- change
- )}"
- >
- <iron-icon icon="gr-icons:delete"></iron-icon>
- </gr-button>
- </gr-tooltip-content>
- </td>
+ <td>${this.renderRemoveVote(mappedLabel.account)}</td>
</tr>`;
}
+ private renderRemoveVote(reviewer: AccountInfo) {
+ return html`<gr-tooltip-content has-tooltip title="Remove vote">
+ <gr-button
+ link
+ aria-label="Remove vote"
+ @click="${this.onDeleteVote}"
+ data-account-id="${ifDefined(reviewer._account_id)}"
+ class="deleteBtn ${this.computeDeleteClass(
+ reviewer,
+ this.mutable,
+ this.change
+ )}"
+ >
+ <iron-icon icon="gr-icons:delete"></iron-icon>
+ </gr-button>
+ </gr-tooltip-content>`;
+ }
+
/**
* This method also listens on change.labels.*,
* to trigger computation when a label is removed from the change.
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index db84043..351cc13 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -102,6 +102,7 @@
import {ChangeMessage} from '../elements/change/gr-message/gr-message';
import {GenerateUrlEditViewParameters} from '../elements/core/gr-navigation/gr-navigation';
import {
+ DetailedLabelInfo,
SubmitRequirementExpressionInfo,
SubmitRequirementResultInfo,
SubmitRequirementStatus,
@@ -706,3 +707,13 @@
message: 'This is the test message.',
};
}
+
+export function createDetailedLabelInfo(): DetailedLabelInfo {
+ return {
+ values: {
+ ' 0': 'No score',
+ '+1': 'Style Verified',
+ '-1': 'Wrong Style or Formatting',
+ },
+ };
+}
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 78c4132..960b02f 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -134,6 +134,16 @@
return label.all?.filter(x => x._account_id === account._account_id)[0];
}
+export function canVote(label: DetailedLabelInfo, account: AccountInfo) {
+ const approvalInfo = getApprovalInfo(label, account);
+ if (!approvalInfo) return false;
+ if (approvalInfo.permitted_voting_range) {
+ return approvalInfo.permitted_voting_range.max > 0;
+ }
+ // If value present, user can vote on the label.
+ return approvalInfo.value !== undefined;
+}
+
export function getAllUniqueApprovals(labelInfo?: LabelInfo) {
if (!labelInfo || !isDetailedLabelInfo(labelInfo)) return [];
const uniqueApprovals = (labelInfo.all ?? [])
@@ -148,8 +158,7 @@
export function hasVotes(labelInfo: LabelInfo): boolean {
if (isDetailedLabelInfo(labelInfo)) {
return (labelInfo.all ?? []).some(
- approval =>
- getLabelStatus(labelInfo, approval.value) !== LabelStatus.NEUTRAL
+ approval => !hasNeutralStatus(labelInfo, approval)
);
}
if (isQuickLabelInfo(labelInfo)) {
