Merge "Submit Requirements - undefined value in approvalinfo is neutral status"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 84e68ed..acf65a5 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4343,7 +4343,7 @@
before it is focefully cancelled.
+
The receive timeout cannot be overriden by setting a higher
-link:user-upload#deadline[deadline] on the git push request.
+link:user-upload.html#deadline[deadline] on the git push request.
+
Default is 4 minutes. If no unit is specified, milliseconds
is assumed.
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 f8bdbef..c19be58e 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
@@ -301,15 +301,6 @@
@property({type: Boolean})
disableEdit = false;
- @property({type: Boolean})
- disableDiffPrefs = false;
-
- @property({
- type: Boolean,
- computed: '_computeDiffPrefsDisabled(disableDiffPrefs, _loggedIn)',
- })
- _diffPrefsDisabled?: boolean;
-
@property({type: Array})
_commentThreads?: CommentThread[];
@@ -1714,11 +1705,7 @@
if (this.shortcuts.shouldSuppress(e) || this.shortcuts.modifierPressed(e)) {
return;
}
-
- if (this._diffPrefsDisabled) {
- return;
- }
-
+ if (!this._loggedIn) return;
e.preventDefault();
this.$.fileList.openDiffPrefs();
}
@@ -2602,10 +2589,6 @@
return currentRevision && revisions && revisions[currentRevision];
}
- _computeDiffPrefsDisabled(disableDiffPrefs: boolean, loggedIn: boolean) {
- return disableDiffPrefs || !loggedIn;
- }
-
/**
* Wrapper for using in the element template and computed properties
*/
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 d57aca8..0b77bc7 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
@@ -537,7 +537,7 @@
patch-num="{{_patchRange.patchNum}}"
base-patch-num="{{_patchRange.basePatchNum}}"
files-expanded="[[_filesExpanded]]"
- diff-prefs-disabled="[[_diffPrefsDisabled]]"
+ diff-prefs-disabled="[[!_loggedIn]]"
on-open-diff-prefs="_handleOpenDiffPrefs"
on-open-download-dialog="_handleOpenDownloadDialog"
on-expand-diffs="_expandAllDiffs"
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 a82fceb..e508c63 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
@@ -806,16 +806,11 @@
'open'
);
element._loggedIn = false;
- element.disableDiffPrefs = true;
pressAndReleaseKeyOn(element, 188, null, ',');
assert.isFalse(stub.called);
element._loggedIn = true;
pressAndReleaseKeyOn(element, 188, null, ',');
- assert.isFalse(stub.called);
-
- element.disableDiffPrefs = false;
- pressAndReleaseKeyOn(element, 188, null, ',');
assert.isTrue(stub.called);
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 8aef3c0..50bb665 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -126,9 +126,6 @@
@property({type: Object})
diffPrefs?: DiffPreferencesInfo;
- @property({type: Boolean})
- diffPrefsDisabled?: boolean;
-
@property({type: String, notify: true})
diffViewMode?: DiffViewMode;
@@ -175,11 +172,8 @@
return classes.join(' ');
}
- _computePrefsButtonHidden(
- prefs: DiffPreferencesInfo,
- diffPrefsDisabled: boolean
- ) {
- return diffPrefsDisabled || !prefs;
+ _computePrefsButtonHidden(prefs: DiffPreferencesInfo, loggedIn: boolean) {
+ return !loggedIn || !prefs;
}
_fileListActionsVisible(
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
index 5972393..73d0819 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
@@ -170,12 +170,12 @@
<gr-diff-mode-selector
id="modeSelect"
mode="{{diffViewMode}}"
- save-on-change="[[!diffPrefsDisabled]]"
+ save-on-change="[[loggedIn]]"
></gr-diff-mode-selector>
<span
id="diffPrefsContainer"
class="hideOnEdit"
- hidden$="[[_computePrefsButtonHidden(diffPrefs, diffPrefsDisabled)]]"
+ hidden$="[[_computePrefsButtonHidden(diffPrefs, loggedIn)]]"
hidden=""
>
<gr-tooltip-content has-tooltip title="Diff preferences">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
index c90cfcc..479a9a1 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
@@ -38,21 +38,11 @@
await flush();
});
- test('Diff preferences hidden when no prefs or diffPrefsDisabled', () => {
- element.diffPrefsDisabled = true;
- flush();
+ test('Diff preferences hidden when no prefs', () => {
assert.isTrue(element.$.diffPrefsContainer.hidden);
- element.diffPrefsDisabled = false;
- flush();
- assert.isTrue(element.$.diffPrefsContainer.hidden);
-
- element.diffPrefsDisabled = true;
element.diffPrefs = {font_size: '12'};
- flush();
- assert.isTrue(element.$.diffPrefsContainer.hidden);
-
- element.diffPrefsDisabled = false;
+ element.loggedIn = true;
flush();
assert.isFalse(element.$.diffPrefsContainer.hidden);
});
@@ -168,7 +158,7 @@
suite('editMode behavior', () => {
setup(() => {
- element.diffPrefsDisabled = false;
+ element.loggedIn = true;
element.diffPrefs = {};
});
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 e3edb5e..de11b16 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
@@ -21,8 +21,6 @@
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-reviewer-list_html';
-import {isSelf, isServiceUser} from '../../../utils/account-util';
-import {hasAttention} from '../../../utils/attention-set-util';
import {customElement, property, computed, observe} from '@polymer/decorators';
import {
ChangeInfo,
@@ -44,6 +42,7 @@
import {appContext} from '../../../services/app-context';
import {fireAlert} from '../../../utils/event-util';
import {getApprovalInfo, getCodeReviewLabel} from '../../../utils/label-util';
+import {sortReviewers} from '../../../utils/attention-set-util';
@customElement('gr-reviewer-list')
export class GrReviewerList extends PolymerElement {
@@ -237,22 +236,7 @@
}
this._reviewers = result
.filter(reviewer => reviewer._account_id !== owner._account_id)
- // Sort order:
- // 1. The user themselves
- // 2. Human users in the attention set.
- // 3. Other human users.
- // 4. Service users.
- .sort((r1, r2) => {
- if (this.account) {
- if (isSelf(r1, this.account)) return -1;
- if (isSelf(r2, this.account)) return 1;
- }
- const a1 = hasAttention(r1, this.change!) ? 1 : 0;
- const a2 = hasAttention(r2, this.change!) ? 1 : 0;
- const s1 = isServiceUser(r1) ? -2 : 0;
- const s2 = isServiceUser(r2) ? -2 : 0;
- return a2 - a1 + s2 - s1;
- });
+ .sort((r1, r2) => sortReviewers(r1, r2, this.change, this.account));
if (this._reviewers.length > 8) {
this._displayedReviewers = this._reviewers.slice(0, 6);
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 40d80bd..e302b43 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
@@ -81,7 +81,7 @@
color: var(--success-foreground);
}
iron-icon.close {
- color: var(--warning-foreground);
+ color: var(--error-foreground);
}
.requirements,
section.trigger-votes {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 295e41f..084f9f6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -174,15 +174,6 @@
@property({type: Object, notify: true, observer: '_changeViewStateChanged'})
changeViewState: Partial<ChangeViewState> = {};
- @property({type: Boolean})
- disableDiffPrefs = false;
-
- @property({
- type: Boolean,
- computed: '_computeDiffPrefsDisabled(disableDiffPrefs, _loggedIn)',
- })
- _diffPrefsDisabled?: boolean;
-
@property({type: Object})
_patchRange?: PatchRange;
@@ -805,7 +796,7 @@
_handleCommaKey(e: IronKeyboardEvent) {
if (this.shortcuts.shouldSuppress(e)) return;
if (this.shortcuts.modifierPressed(e)) return;
- if (this._diffPrefsDisabled) return;
+ if (!this._loggedIn) return;
e.preventDefault();
this.$.diffPreferencesDialog.open();
@@ -1409,11 +1400,8 @@
return dropdownContent;
}
- _computePrefsButtonHidden(
- prefs?: DiffPreferencesInfo,
- prefsDisabled?: boolean
- ) {
- return prefsDisabled || !prefs;
+ _computePrefsButtonHidden(prefs?: DiffPreferencesInfo, loggedIn?: boolean) {
+ return !loggedIn || !prefs;
}
_handleFileChange(e: CustomEvent) {
@@ -1842,10 +1830,6 @@
this.$.diffHost.toggleAllContext();
}
- _computeDiffPrefsDisabled(disableDiffPrefs?: boolean, loggedIn?: boolean) {
- return disableDiffPrefs || !loggedIn;
- }
-
_handleNextUnreviewedFile(e: IronKeyboardEvent) {
if (this.shortcuts.shouldSuppress(e)) return;
this._setReviewed(true);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index b25be5a8..308c353 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -338,14 +338,14 @@
<span>Diff view:</span>
<gr-diff-mode-selector
id="modeSelect"
- save-on-change="[[!_diffPrefsDisabled]]"
+ save-on-change="[[_loggedIn]]"
mode="{{changeViewState.diffMode}}"
show-tooltip-below=""
></gr-diff-mode-selector>
</div>
<span
id="diffPrefsContainer"
- hidden$="[[_computePrefsButtonHidden(_prefs, _diffPrefsDisabled)]]"
+ hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]"
hidden=""
>
<span class="preferences desktop">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 0c7abc8..cc35c3c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -489,10 +489,6 @@
MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
assert(showPrefsStub.calledOnce);
- element.disableDiffPrefs = true;
- MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
- assert(showPrefsStub.calledOnce);
-
let scrollStub = sinon.stub(element.cursor, 'moveToNextChunk');
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
assert(scrollStub.calledOnce);
@@ -988,8 +984,7 @@
});
suite('diff prefs hidden', () => {
- test('when no prefs or logged out', () => {
- element.disableDiffPrefs = false;
+ test('whenlogged out', () => {
element._loggedIn = false;
flush();
assert.isTrue(element.$.diffPrefsContainer.hidden);
@@ -1004,21 +999,9 @@
assert.isTrue(element.$.diffPrefsContainer.hidden);
element._loggedIn = true;
- flush();
- assert.isFalse(element.$.diffPrefsContainer.hidden);
- });
-
- test('when disableDiffPrefs is set', () => {
- element._loggedIn = true;
element._prefs = {font_size: '12'};
- element.disableDiffPrefs = false;
flush();
-
assert.isFalse(element.$.diffPrefsContainer.hidden);
- element.disableDiffPrefs = true;
- flush();
-
- assert.isTrue(element.$.diffPrefsContainer.hidden);
});
});
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 947ef3e..bdae6f7 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,
+ valueString,
} from '../../../utils/label-util';
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
@@ -53,6 +54,7 @@
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 {
interface HTMLElementTagNameMap {
@@ -213,11 +215,13 @@
private renderNewSubmitRequirements() {
const labelInfo = this.labelInfo;
if (!labelInfo) return;
- const reviewers = (this.change?.reviewers['REVIEWER'] ?? []).filter(
- reviewer =>
- (this.showAllReviewers && canVote(labelInfo, reviewer)) ||
- (!this.showAllReviewers && hasVoted(labelInfo, reviewer))
- );
+ const reviewers = (this.change?.reviewers['REVIEWER'] ?? [])
+ .filter(
+ reviewer =>
+ (this.showAllReviewers && canVote(labelInfo, reviewer)) ||
+ (!this.showAllReviewers && hasVoted(labelInfo, reviewer))
+ )
+ .sort((r1, r2) => sortReviewers(r1, r2, this.change, this.account));
return html`<div>
${reviewers.map(reviewer => this.renderReviewerVote(reviewer))}
</div>`;
@@ -255,7 +259,7 @@
></gr-vote-chip
></gr-account-chip>
${noVoteYet
- ? html`<span class="no-votes">No votes</span>`
+ ? this.renderVoteAbility(reviewer)
: html`${this.renderRemoveVote(reviewer)}`}
</div>`;
}
@@ -283,6 +287,19 @@
</tr>`;
}
+ private renderVoteAbility(reviewer: AccountInfo) {
+ if (this.labelInfo && isDetailedLabelInfo(this.labelInfo)) {
+ const approvalInfo = getApprovalInfo(this.labelInfo, reviewer);
+ if (approvalInfo?.permitted_voting_range) {
+ const {min, max} = approvalInfo?.permitted_voting_range;
+ return html`<span class="no-votes"
+ >Can vote ${valueString(min)}/${valueString(max)}</span
+ >`;
+ }
+ }
+ return html`<span class="no-votes">No votes</span>`;
+ }
+
private renderRemoveVote(reviewer: AccountInfo) {
return html`<gr-tooltip-content has-tooltip title="Remove vote">
<gr-button
diff --git a/polygerrit-ui/app/utils/attention-set-util.ts b/polygerrit-ui/app/utils/attention-set-util.ts
index dcd2863..b0b7ef8 100644
--- a/polygerrit-ui/app/utils/attention-set-util.ts
+++ b/polygerrit-ui/app/utils/attention-set-util.ts
@@ -19,6 +19,7 @@
import {ParsedChangeInfo} from '../types/types';
import {
getAccountTemplate,
+ isSelf,
isServiceUser,
replaceTemplates,
} from './account-util';
@@ -92,3 +93,27 @@
const entry = change!.attention_set![account!._account_id!];
return entry?.last_update ? entry.last_update : '';
}
+
+/**
+ * Sort order:
+ * 1. The user themselves
+ * 2. Human users in the attention set.
+ * 3. Other human users.
+ * 4. Service users.
+ */
+export function sortReviewers(
+ r1: AccountInfo,
+ r2: AccountInfo,
+ change?: ChangeInfo | ParsedChangeInfo,
+ selfAccount?: AccountInfo
+) {
+ if (selfAccount) {
+ if (isSelf(r1, selfAccount)) return -1;
+ if (isSelf(r2, selfAccount)) return 1;
+ }
+ const a1 = hasAttention(r1, change) ? 1 : 0;
+ const a2 = hasAttention(r2, change) ? 1 : 0;
+ const s1 = isServiceUser(r1) ? -2 : 0;
+ const s2 = isServiceUser(r2) ? -2 : 0;
+ return a2 - a1 + s2 - s1;
+}
