Merge "Add a plugin endpoint to the cherrypick dialog"
diff --git a/Documentation/user-notify.txt b/Documentation/user-notify.txt
index 20ad07c..24c35f0 100644
--- a/Documentation/user-notify.txt
+++ b/Documentation/user-notify.txt
@@ -18,8 +18,8 @@
[[user]]
== User Level Settings
-Individual users can configure email subscriptions by editing
-watched projects through Settings > Watched Projects with the web UI.
+Individual users can configure email subscriptions by editing their
+notifications in the Web UI under `Settings` > `Notifications`.
Specific projects may be watched, or the special project
`All-Projects` can be watched to watch all projects that
diff --git a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
index 629cf83..2fba6ab 100644
--- a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.index.IndexedField;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.SchemaDefinitions;
@@ -43,7 +44,7 @@
GroupField.NAME_PART_FIELD,
GroupField.OWNER_UUID_FIELD,
GroupField.SUBGROUP_FIELD),
- ImmutableList.of(
+ ImmutableList.<IndexedField<InternalGroup, ?>.SearchSpec>of(
GroupField.CREATED_ON_SPEC,
GroupField.DESCRIPTION_SPEC,
GroupField.ID_FIELD_SPEC,
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
index 5a4a90c..ef48323 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow_test.ts
@@ -891,8 +891,6 @@
);
assert.deepEqual(reviewerList.accounts[1], {
- _group: true,
- _pendingAdd: true,
confirmed: true,
id: '5' as GroupId,
name: 'large-group',
@@ -933,8 +931,6 @@
getComputedStyle(confirmDialog).getPropertyValue('display') === 'none'
);
assert.deepEqual(reviewerList.accounts[1], {
- _group: true,
- _pendingAdd: true,
id: '5' as GroupId,
name: 'small-group',
});
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 753097a..ccdcc15 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
@@ -27,15 +27,14 @@
} from '../../../constants/constants';
import {
accountOrGroupKey,
- isReviewerOrCC,
- mapReviewer,
+ isAccountNewlyAdded,
removeServiceUsers,
+ toReviewInput,
} from '../../../utils/account-util';
import {IronA11yAnnouncer} from '@polymer/iron-a11y-announcer/iron-a11y-announcer';
import {TargetElement} from '../../../api/plugin';
import {FixIronA11yAnnouncer} from '../../../types/types';
import {
- AccountAddition,
AccountInfoInput,
AccountInput,
AccountInputDetail,
@@ -71,6 +70,7 @@
areSetsEqual,
assertIsDefined,
containsAll,
+ difference,
queryAndAssert,
} from '../../../utils/common-util';
import {CommentThread, isUnresolved} from '../../../utils/comment-util';
@@ -108,6 +108,7 @@
import {customElement, property, state, query} from 'lit/decorators';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
+import {hasHumanReviewer, isOwner} from '../../../utils/change-util';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -745,6 +746,7 @@
id="reviewers"
.accounts=${this.getAccountListCopy(this.reviewers)}
.change=${this.change}
+ .reviewerState=${ReviewerState.REVIEWER}
@account-added=${this.accountAdded}
@accounts-changed=${this.handleReviewersChanged}
.removableValues=${this.change?.removable_reviewers}
@@ -771,6 +773,8 @@
<gr-account-list
id="ccs"
.accounts=${this.getAccountListCopy(this.ccs)}
+ .change=${this.change}
+ .reviewerState=${ReviewerState.CC}
@account-added=${this.accountAdded}
@accounts-changed=${this.handleCcsChanged}
.removableValues=${this.change?.removable_reviewers}
@@ -1281,42 +1285,32 @@
return isResolvedPatchsetLevelComment ? 'resolved' : 'unresolved';
}
- computeReviewers(change: ChangeInfo) {
+ computeReviewers() {
const reviewers: ReviewerInput[] = [];
- const addToReviewInput = (
- additions: AccountAddition[],
- state?: ReviewerState
- ) => {
- additions.forEach(addition => {
- const reviewer = mapReviewer(addition);
- if (state) reviewer.state = state;
- reviewers.push(reviewer);
- });
- };
- addToReviewInput(this.reviewersList!.additions(), ReviewerState.REVIEWER);
- addToReviewInput(this.ccsList!.additions(), ReviewerState.CC);
- addToReviewInput(
- this.reviewersList!.removals().filter(
- r =>
- isReviewerOrCC(change, r) &&
- // ignore removal from reviewer request if being added to CC
- !this.ccsList!.additions().some(
- account => mapReviewer(account).reviewer === mapReviewer(r).reviewer
- )
- ),
- ReviewerState.REMOVED
+ const reviewerAdditions = this.reviewersList?.additions() ?? [];
+ reviewers.push(
+ ...reviewerAdditions.map(v => toReviewInput(v, ReviewerState.REVIEWER))
);
- addToReviewInput(
- this.ccsList!.removals().filter(
- r =>
- isReviewerOrCC(change, r) &&
- // ignore removal from CC request if being added as reviewer
- !this.reviewersList!.additions().some(
- account => mapReviewer(account).reviewer === mapReviewer(r).reviewer
- )
- ),
- ReviewerState.REMOVED
- );
+
+ const ccAdditions = this.ccsList?.additions() ?? [];
+ reviewers.push(...ccAdditions.map(v => toReviewInput(v, ReviewerState.CC)));
+
+ // ignore removal from reviewer request if being added as CC
+ let removals = difference(
+ this.reviewersList?.removals() ?? [],
+ ccAdditions,
+ (a, b) => accountOrGroupKey(a) === accountOrGroupKey(b)
+ ).map(v => toReviewInput(v, ReviewerState.REMOVED));
+ reviewers.push(...removals);
+
+ // ignore removal from CC request if being added as reviewer
+ removals = difference(
+ this.ccsList?.removals() ?? [],
+ reviewerAdditions,
+ (a, b) => accountOrGroupKey(a) === accountOrGroupKey(b)
+ ).map(v => toReviewInput(v, ReviewerState.REMOVED));
+ reviewers.push(...removals);
+
return reviewers;
}
@@ -1367,7 +1361,7 @@
}
assertIsDefined(this.change, 'change');
- reviewInput.reviewers = this.computeReviewers(this.change);
+ reviewInput.reviewers = this.computeReviewers();
this.disabled = true;
const errFn = (r?: Response | null) => this.handle400Error(r);
@@ -1422,19 +1416,9 @@
}
chooseFocusTarget() {
- // If we are the owner and the reviewers field is empty, focus on that.
- if (
- this.account &&
- this.change &&
- this.change.owner &&
- this.account._account_id === this.change.owner._account_id &&
- (!this.reviewers || this.reviewers?.length === 0)
- ) {
- return FocusTarget.REVIEWERS;
- }
-
- // Default to BODY.
- return FocusTarget.BODY;
+ if (!isOwner(this.change, this.account)) return FocusTarget.BODY;
+ if (hasHumanReviewer(this.change)) return FocusTarget.BODY;
+ return FocusTarget.REVIEWERS;
}
isOwner(account?: AccountInfo, change?: ChangeInfo) {
@@ -1624,7 +1608,11 @@
);
this.reviewers
.filter(r => isAccount(r))
- .filter(r => r._pendingAdd || (this.canBeStarted && isOwner))
+ .filter(
+ r =>
+ isAccountNewlyAdded(r, ReviewerState.REVIEWER, this.change) ||
+ (this.canBeStarted && isOwner)
+ )
.filter(notIsReviewerAndHasDraftOrLabel)
.forEach(r => newAttention.add((r as AccountInfo)._account_id!));
// Add owner and uploader, if someone else replies.
@@ -1812,7 +1800,6 @@
})
);
queryAndAssert<GrTextarea>(this, 'gr-textarea').closeDropdown();
- this.reviewersList?.clearPendingRemovals();
this.rebuildReviewerArrays();
}
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 55a98f2..67190de 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
@@ -28,6 +28,7 @@
createCommentThread,
createDraft,
createRevision,
+ createServiceUserWithId,
} from '../../../test/test-data-generators';
import {
pressAndReleaseKeyOn,
@@ -899,8 +900,8 @@
await element.updateComplete;
element.reviewers = [
- {_account_id: 1 as AccountId, _pendingAdd: true},
- {_account_id: 2 as AccountId, _pendingAdd: true},
+ {_account_id: 1 as AccountId},
+ {_account_id: 2 as AccountId},
];
element.ccs = [];
element.draftCommentThreads = [];
@@ -929,6 +930,12 @@
owner: {_account_id: 1 as AccountId},
status: ChangeStatus.NEW,
attention_set: {},
+ reviewers: {
+ [ReviewerState.REVIEWER]: [
+ {...createAccountWithId(2)},
+ {...createAccountWithId(3)},
+ ],
+ },
};
// let rebuildReviewers triggered by change update finish running
await element.updateComplete;
@@ -1342,14 +1349,8 @@
);
// No reviewer/CC should have been added.
- assert.equal(
- queryAndAssert<GrAccountList>(element, '#ccs').additions().length,
- 0
- );
- assert.equal(
- queryAndAssert<GrAccountList>(element, '#reviewers').additions().length,
- 0
- );
+ assert.equal(element.ccsList?.additions().length, 0);
+ assert.equal(element.reviewersList?.additions().length, 0);
// Reopen confirmation dialog.
observer = overlayObserver('opened');
@@ -1379,17 +1380,11 @@
isVisible(queryAndAssert(element, 'reviewerConfirmationOverlay'))
);
const additions = cc
- ? queryAndAssert<GrAccountList>(element, '#ccs').additions()
- : queryAndAssert<GrAccountList>(element, '#reviewers').additions();
+ ? element.ccsList?.additions()
+ : element.reviewersList?.additions();
assert.deepEqual(additions, [
{
- group: {
- id: 'id' as GroupId,
- name: 'name' as GroupName,
- confirmed: true,
- _group: true,
- _pendingAdd: true,
- },
+ name: 'name' as GroupName,
},
]);
@@ -1630,28 +1625,22 @@
test('chooseFocusTarget', () => {
element.account = undefined;
- assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+ assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
- element.account = {_account_id: 1 as AccountId};
- assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+ element.account = element.change!.owner;
+ assert.equal(element.chooseFocusTarget(), element.FocusTarget.REVIEWERS);
- element.change!.owner = {_account_id: 2 as AccountId};
- assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+ element.change!.reviewers.REVIEWER = [createAccountWithId(314)];
+ assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
- element.change!.owner._account_id = 1 as AccountId;
- assert.strictEqual(
- element.chooseFocusTarget(),
- element.FocusTarget.REVIEWERS
- );
+ element.change!.reviewers.REVIEWER = [createServiceUserWithId(314)];
+ assert.equal(element.chooseFocusTarget(), element.FocusTarget.REVIEWERS);
- element.reviewers = [];
- assert.strictEqual(
- element.chooseFocusTarget(),
- element.FocusTarget.REVIEWERS
- );
-
- element.reviewers.push({});
- assert.strictEqual(element.chooseFocusTarget(), element.FocusTarget.BODY);
+ element.change!.reviewers.REVIEWER = [
+ createAccountWithId(314),
+ createServiceUserWithId(314),
+ ];
+ assert.equal(element.chooseFocusTarget(), element.FocusTarget.BODY);
});
test('only send labels that have changed', async () => {
@@ -2005,6 +1994,8 @@
[ReviewerState.REVIEWER]: [{_account_id: reviewer1._account_id}],
};
+ await element.updateComplete;
+
const mutations: ReviewerInput[] = [];
stubSaveReview((review: ReviewInput) => {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-action.ts b/polygerrit-ui/app/elements/checks/gr-checks-action.ts
index 3564ec9..d7373de 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-action.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-action.ts
@@ -6,7 +6,7 @@
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
import {Action} from '../../api/checks';
-import {checkRequiredProperty} from '../../utils/common-util';
+import {assertIsDefined} from '../../utils/common-util';
import {resolve} from '../../models/dependency';
import {checksModelToken} from '../../models/checks/checks-model';
@customElement('gr-checks-action')
@@ -25,7 +25,7 @@
override connectedCallback() {
super.connectedCallback();
- checkRequiredProperty(this.action, 'action');
+ assertIsDefined(this.action, 'action');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 202c502..edefdd1 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -33,7 +33,7 @@
secondaryLinks,
tooltipForLink,
} from '../../models/checks/checks-util';
-import {assertIsDefined, check} from '../../utils/common-util';
+import {assertIsDefined, assert} from '../../utils/common-util';
import {modifierPressed, toggleClass, whenVisible} from '../../utils/dom-util';
import {durationString} from '../../utils/date-util';
import {charsOnly} from '../../utils/string-util';
@@ -1201,7 +1201,7 @@
private onPatchsetSelected(e: CustomEvent<{value: string}>) {
const patchset = Number(e.detail.value);
- check(!isNaN(patchset), 'selected patchset must be a number');
+ assert(!isNaN(patchset), 'selected patchset must be a number');
this.getChecksModel().setPatchset(patchset as PatchSetNumber);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
index 1fa21e5..c8a2995 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
@@ -15,12 +15,16 @@
SuggestedReviewerGroupInfo,
SuggestedReviewerAccountInfo,
SuggestedReviewerInfo,
+ isGroup,
} from '../../../types/common';
import {ReviewerSuggestionsProvider} from '../../../scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider';
import {GrAccountEntry} from '../gr-account-entry/gr-account-entry';
import {GrAccountChip} from '../gr-account-chip/gr-account-chip';
import {fire, fireAlert} from '../../../utils/event-util';
-import {accountOrGroupKey} from '../../../utils/account-util';
+import {
+ accountOrGroupKey,
+ isAccountNewlyAdded,
+} from '../../../utils/account-util';
import {LitElement, css, html, PropertyValues} from 'lit';
import {customElement, property, query, state} from 'lit/decorators';
import {sharedStyles} from '../../../styles/shared-styles';
@@ -31,9 +35,10 @@
GrAutocomplete,
} from '../gr-autocomplete/gr-autocomplete';
import {ValueChangedEvent} from '../../../types/events';
-import {queryAndAssert} from '../../../utils/common-util';
+import {difference, queryAndAssert} from '../../../utils/common-util';
import {PaperInputElement} from '@polymer/paper-input/paper-input';
import {IronInputElement} from '@polymer/iron-input';
+import {ReviewerState} from '../../../api/rest-api';
const VALID_EMAIL_ALERT = 'Please input a valid email.';
@@ -74,37 +79,18 @@
// Internal input type with account info
export interface AccountInfoInput extends AccountInfo {
- _group?: boolean;
_account?: boolean;
- _pendingAdd?: boolean;
confirmed?: boolean;
}
// Internal input type with group info
export interface GroupInfoInput extends GroupInfo {
- _group?: boolean;
_account?: boolean;
- _pendingAdd?: boolean;
confirmed?: boolean;
}
-function isAccountInfoInput(x: AccountInput): x is AccountInfoInput {
- const input = x as AccountInfoInput;
- return !!input._account || !!input._account_id || !!input.email;
-}
-
-function isGroupInfoInput(x: AccountInput): x is GroupInfoInput {
- const input = x as GroupInfoInput;
- return !!input._group || !!input.id;
-}
-
export type AccountInput = AccountInfoInput | GroupInfoInput;
-export interface AccountAddition {
- account?: AccountInfoInput;
- group?: GroupInfoInput;
-}
-
@customElement('gr-account-list')
export class GrAccountList extends LitElement {
/**
@@ -129,6 +115,9 @@
@property({type: Boolean})
disabled = false;
+ @property({type: String})
+ reviewerState?: ReviewerState;
+
/**
* Returns suggestions and convert them to list item
*/
@@ -164,8 +153,6 @@
private readonly reporting = getAppContext().reportingService;
- private pendingRemoval: Set<AccountInput> = new Set();
-
constructor() {
super();
this.querySuggestions = input => this.getSuggestions(input);
@@ -190,7 +177,7 @@
.group {
--account-label-suffix: ' (group)';
}
- .pendingAdd {
+ .newlyAdded {
font-style: italic;
}
.list {
@@ -209,8 +196,12 @@
.account=${account}
.change=${this.change}
class=${classMap({
- group: !!account._group,
- pendingAdd: !!account._pendingAdd,
+ group: isGroup(account),
+ newlyAdded: isAccountNewlyAdded(
+ account,
+ this.reviewerState,
+ this.change
+ ),
})}
?removable=${this.computeRemovable(account)}
@keydown=${this.handleChipKeydown}
@@ -284,8 +275,7 @@
let group;
let itemTypeAdded = 'unknown';
if (isAccountObject(item)) {
- account = {...item.account, _pendingAdd: true};
- this.removeFromPendingRemoval(account);
+ account = {...item.account};
this.accounts.push(account);
itemTypeAdded = 'account';
} else if (isSuggestedReviewerGroupInfo(item)) {
@@ -293,9 +283,8 @@
this.pendingConfirmation = item;
return;
}
- group = {...item.group, _pendingAdd: true, _group: true};
+ group = {...item.group};
this.accounts.push(group);
- this.removeFromPendingRemoval(group);
itemTypeAdded = 'group';
} else if (this.allowAnyInput) {
if (!item.includes('@')) {
@@ -305,9 +294,8 @@
fireAlert(this, VALID_EMAIL_ALERT);
return false;
} else {
- account = {email: item as EmailAddress, _pendingAdd: true};
+ account = {email: item as EmailAddress};
this.accounts.push(account);
- this.removeFromPendingRemoval(account);
itemTypeAdded = 'email';
}
}
@@ -323,8 +311,6 @@
this.accounts.push({
...group,
confirmed: true,
- _pendingAdd: true,
- _group: true,
});
this.pendingConfirmation = null;
fire(this, 'accounts-changed', {value: this.accounts});
@@ -332,18 +318,6 @@
}
// private but used in test
- computeChipClass(account: AccountInput) {
- const classes = [];
- if (account._group) {
- classes.push('group');
- }
- if (account._pendingAdd) {
- classes.push('pendingAdd');
- }
- return classes.join(' ');
- }
-
- // private but used in test
computeRemovable(account: AccountInput) {
if (this.readonly) {
return false;
@@ -357,7 +331,7 @@
return true;
}
}
- return !!account._pendingAdd;
+ return isAccountNewlyAdded(account, this.reviewerState, this.change);
}
return true;
}
@@ -375,7 +349,6 @@
for (let i = 0; i < this.accounts.length; i++) {
if (accountOrGroupKey(toRemove) === accountOrGroupKey(this.accounts[i])) {
this.accounts.splice(i, 1);
- this.pendingRemoval.add(toRemove);
this.reporting.reportInteraction(`Remove from ${this.id}`);
this.requestUpdate();
fire(this, 'accounts-changed', {value: this.accounts.slice()});
@@ -473,37 +446,19 @@
return wasSubmitted;
}
- additions(): AccountAddition[] {
- return this.accounts
- .filter(account => account._pendingAdd)
- .map(account => {
- if (isGroupInfoInput(account)) {
- return {group: account};
- } else if (isAccountInfoInput(account)) {
- return {account};
- } else {
- throw new Error('AccountInput must be either Account or Group.');
- }
- });
+ additions(): (AccountInfoInput | GroupInfoInput)[] {
+ if (!this.change) return [];
+ return this.accounts.filter(account =>
+ isAccountNewlyAdded(account, this.reviewerState, this.change)
+ );
}
- removals(): AccountAddition[] {
- return Array.from(this.pendingRemoval).map(account => {
- if (isGroupInfoInput(account)) {
- return {group: account};
- } else if (isAccountInfoInput(account)) {
- return {account};
- } else {
- throw new Error('AccountInput must be either Account or Group.');
- }
- });
- }
-
- private removeFromPendingRemoval(account: AccountInput) {
- this.pendingRemoval.delete(account);
- }
-
- clearPendingRemovals() {
- this.pendingRemoval.clear();
+ removals(): AccountInfo[] {
+ if (!this.reviewerState) return [];
+ return difference(
+ this.change?.reviewers[this.reviewerState] ?? [],
+ this.accounts,
+ (a, b) => accountOrGroupKey(a) === accountOrGroupKey(b)
+ );
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
index 21b9e59..873b6cf 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
@@ -28,6 +28,8 @@
GrAutocomplete,
} from '../gr-autocomplete/gr-autocomplete';
import {GrAccountEntry} from '../gr-account-entry/gr-account-entry';
+import {createChange} from '../../../test/test-data-generators';
+import {ReviewerState} from '../../../api/rest-api';
const basicFixture = fixtureFromElement('gr-account-list');
@@ -65,7 +67,6 @@
const groupId = `group${++_nextAccountId}`;
return {
id: groupId as GroupId,
- _group: true,
name: 'abcd' as GroupName,
};
};
@@ -94,6 +95,9 @@
element = basicFixture.instantiate();
element.accounts = [existingAccount1, existingAccount2];
+ element.reviewerState = ReviewerState.REVIEWER;
+ element.change = {...createChange()};
+ element.change.reviewers[ReviewerState.REVIEWER] = [...element.accounts];
suggestionsProvider = new MockSuggestionsProvider();
element.suggestionsProvider = suggestionsProvider;
await element.updateComplete;
@@ -130,18 +134,18 @@
// Existing accounts are listed.
let chips = getChips();
assert.equal(chips.length, 2);
- assert.isFalse(chips[0].classList.contains('pendingAdd'));
- assert.isFalse(chips[1].classList.contains('pendingAdd'));
+ assert.isFalse(chips[0].classList.contains('newlyAdded'));
+ assert.isFalse(chips[1].classList.contains('newlyAdded'));
- // New accounts are added to end with pendingAdd class.
+ // New accounts are added to end with newlyAdded class.
const newAccount = makeAccount();
handleAdd({account: newAccount, count: 1});
await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 3);
- assert.isFalse(chips[0].classList.contains('pendingAdd'));
- assert.isFalse(chips[1].classList.contains('pendingAdd'));
- assert.isTrue(chips[2].classList.contains('pendingAdd'));
+ assert.isFalse(chips[0].classList.contains('newlyAdded'));
+ assert.isFalse(chips[1].classList.contains('newlyAdded'));
+ assert.isTrue(chips[2].classList.contains('newlyAdded'));
// Removed accounts are taken out of the list.
element.dispatchEvent(
@@ -154,8 +158,8 @@
await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 2);
- assert.isFalse(chips[0].classList.contains('pendingAdd'));
- assert.isTrue(chips[1].classList.contains('pendingAdd'));
+ assert.isFalse(chips[0].classList.contains('newlyAdded'));
+ assert.isTrue(chips[1].classList.contains('newlyAdded'));
// Invalid remove is ignored.
element.dispatchEvent(
@@ -175,16 +179,16 @@
await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 1);
- assert.isFalse(chips[0].classList.contains('pendingAdd'));
+ assert.isFalse(chips[0].classList.contains('newlyAdded'));
- // New groups are added to end with pendingAdd and group classes.
+ // New groups are added to end with newlyAdded and group classes.
const newGroup = makeGroup();
handleAdd({group: newGroup, confirm: false, count: 1});
await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 2);
assert.isTrue(chips[1].classList.contains('group'));
- assert.isTrue(chips[1].classList.contains('pendingAdd'));
+ assert.isTrue(chips[1].classList.contains('newlyAdded'));
// Removed groups are taken out of the list.
element.dispatchEvent(
@@ -197,7 +201,7 @@
await element.updateComplete;
chips = getChips();
assert.equal(chips.length, 1);
- assert.isFalse(chips[0].classList.contains('pendingAdd'));
+ assert.isFalse(chips[0].classList.contains('newlyAdded'));
});
test('getSuggestions uses filter correctly', () => {
@@ -260,20 +264,8 @@
});
});
- test('computeChipClass', () => {
- const account = makeAccount() as AccountInfoInput;
- assert.equal(element.computeChipClass(account), '');
- account._pendingAdd = true;
- assert.equal(element.computeChipClass(account), 'pendingAdd');
- account._group = true;
- assert.equal(element.computeChipClass(account), 'group pendingAdd');
- account._pendingAdd = false;
- assert.equal(element.computeChipClass(account), 'group');
- });
-
test('computeRemovable', async () => {
const newAccount = makeAccount() as AccountInfoInput;
- newAccount._pendingAdd = true;
element.readonly = false;
element.removableValues = [];
element.updateComplete;
@@ -320,7 +312,7 @@
assert.isTrue(element.submitEntryText());
assert.isTrue(clearStub.called);
assert.equal(
- element.additions()[0].account?.email,
+ (element.additions()[0] as AccountInfo)?.email,
'test@test' as EmailAddress
);
});
@@ -335,18 +327,11 @@
assert.deepEqual(element.additions(), [
{
- account: {
- _account_id: newAccount._account_id,
- _pendingAdd: true,
- },
+ _account_id: newAccount._account_id,
},
{
- group: {
- id: newGroup.id,
- _group: true,
- _pendingAdd: true,
- name: 'abcd' as GroupName,
- },
+ id: newGroup.id,
+ name: 'abcd' as GroupName,
},
]);
});
@@ -356,7 +341,7 @@
assert.deepEqual(element.additions(), []);
const group = makeGroup();
- const reviewer = {
+ const reviewer: RawAccountInput = {
group,
count: 10,
confirm: true,
@@ -370,13 +355,9 @@
assert.isNull(element.pendingConfirmation);
assert.deepEqual(element.additions(), [
{
- group: {
- id: group.id,
- _group: true,
- _pendingAdd: true,
- confirmed: true,
- name: 'abcd' as GroupName,
- },
+ id: group.id,
+ name: 'abcd' as GroupName,
+ confirmed: true,
},
]);
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
index eac76da..5a34ae3 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
@@ -6,7 +6,7 @@
import {BLANK_LINE, GrDiffLine, GrDiffLineType} from './gr-diff-line';
import {LineRange, Side} from '../../../api/diff';
import {LineNumber} from './gr-diff-line';
-import {assertIsDefined, check} from '../../../utils/common-util';
+import {assertIsDefined, assert} from '../../../utils/common-util';
import {untilRendered} from '../../../utils/dom-util';
export enum GrDiffGroupType {
@@ -267,7 +267,7 @@
};
} else {
assertIsDefined(options.lines);
- check(options.lines.length > 0, 'diff group must have lines');
+ assert(options.lines.length > 0, 'diff group must have lines');
for (const line of options.lines) {
this.addLine(line);
}
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index e7e85c2..81ca830 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -72,6 +72,7 @@
} from '../types/common';
import {
AccountsVisibility,
+ AccountTag,
AppTheme,
AuthType,
ChangeStatus,
@@ -188,6 +189,13 @@
};
}
+export function createServiceUserWithId(id = 5): AccountInfo {
+ return {
+ ...createAccountWithId(id),
+ tags: [AccountTag.SERVICE_USER],
+ };
+}
+
export function createAccountDetailWithId(id = 5): AccountDetailInfo {
return {
_account_id: id as AccountId,
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index 9b8d82a..012dc1c 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -19,7 +19,6 @@
} from '../types/common';
import {AccountTag, ReviewerState} from '../constants/constants';
import {assertNever, hasOwnProperty} from './common-util';
-import {AccountAddition} from '../elements/shared/gr-account-list/gr-account-list';
import {getDisplayName} from './display-name-util';
import {getApprovalInfo} from './label-util';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
@@ -36,30 +35,6 @@
throw new Error('Account has neither _account_id nor email.');
}
-export function mapReviewer(addition: AccountAddition): ReviewerInput {
- if (addition.account) {
- return {reviewer: accountKey(addition.account)};
- }
- if (addition.group) {
- const reviewer = decodeURIComponent(addition.group.id) as GroupId;
- const confirmed = addition.group.confirmed;
- return {reviewer, confirmed};
- }
- throw new Error('Reviewer must be either an account or a group.');
-}
-
-export function isReviewerOrCC(
- change: ChangeInfo,
- reviewerAddition: AccountAddition
-): boolean {
- const reviewers = [
- ...(change.reviewers[ReviewerState.CC] ?? []),
- ...(change.reviewers[ReviewerState.REVIEWER] ?? []),
- ];
- const reviewer = mapReviewer(reviewerAddition);
- return reviewers.some(r => accountOrGroupKey(r) === reviewer.reviewer);
-}
-
export function isServiceUser(account?: AccountInfo): boolean {
return !!account?.tags?.includes(AccountTag.SERVICE_USER);
}
@@ -82,6 +57,18 @@
assertNever(entry, 'entry must be account or group');
}
+export function isAccountNewlyAdded(
+ account: AccountInfo | GroupInfo,
+ state?: ReviewerState,
+ change?: ChangeInfo
+) {
+ if (!change || !state) return false;
+ const accounts = [...(change.reviewers[state] ?? [])];
+ return !accounts.some(
+ a => accountOrGroupKey(a) === accountOrGroupKey(account)
+ );
+}
+
export function uniqueDefinedAvatar(
account: AccountInfo,
index: number,
@@ -221,3 +208,19 @@
}
return emails;
}
+
+export function toReviewInput(
+ account: AccountInfo | GroupInfo,
+ state: ReviewerState
+): ReviewerInput {
+ if (isAccount(account)) {
+ return {
+ reviewer: accountKey(account),
+ state,
+ };
+ } else if (isGroup(account)) {
+ const reviewer = decodeURIComponent(account.id) as GroupId;
+ return {reviewer, state};
+ }
+ throw new Error('Must be either an account or a group.');
+}
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index d6d2f7d..cf0ff53 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -14,6 +14,7 @@
} from '../types/common';
import {ParsedChangeInfo} from '../types/types';
import {ChangeStates} from '../elements/shared/gr-change-status/gr-change-status';
+import {isServiceUser} from './account-util';
// This can be wrong! See WARNING above
interface ChangeStatusesOptions {
@@ -249,6 +250,11 @@
);
}
+export function hasHumanReviewer(change?: ChangeInfo): boolean {
+ const reviewers = change?.reviewers.REVIEWER ?? [];
+ return reviewers.some(r => !isServiceUser(r));
+}
+
export function isRemovableReviewer(
change?: ChangeInfo,
reviewer?: AccountInfo
diff --git a/polygerrit-ui/app/utils/common-util.ts b/polygerrit-ui/app/utils/common-util.ts
index fe86d91..0bae49a 100644
--- a/polygerrit-ui/app/utils/common-util.ts
+++ b/polygerrit-ui/app/utils/common-util.ts
@@ -38,7 +38,7 @@
/**
* Throws an error with the provided error message if the condition is false.
*/
-export function check(
+export function assert(
condition: boolean,
errorMessage: string
): asserts condition {
@@ -48,28 +48,6 @@
/**
* Throws an error if the property is not defined.
*/
-export function checkProperty(
- condition: boolean,
- propertyName: string
-): asserts condition {
- check(condition, `missing required property '${propertyName}'`);
-}
-
-/**
- * Throws an error if the property is not defined.
- */
-export function checkRequiredProperty<T>(
- property: T,
- propertyName: string
-): asserts property is NonNullable<T> {
- if (property === undefined || property === null) {
- throw new Error(`Required property '${propertyName}' not set.`);
- }
-}
-
-/**
- * Throws an error if the property is not defined.
- */
export function assertIsDefined<T>(
val: T,
variableName = 'variable'
@@ -168,3 +146,14 @@
result.filter(t => array.find(u => compareBy(t, u)))
);
}
+
+/**
+ * Returns the elements that are present in A but not present in B.
+ */
+export function difference<T>(
+ a: T[],
+ b: T[],
+ compareBy: (t: T, u: T) => boolean = (t, u) => t === u
+): T[] {
+ return a.filter(aVal => !b.some(bVal => compareBy(aVal, bVal)));
+}
diff --git a/polygerrit-ui/app/utils/common-util_test.ts b/polygerrit-ui/app/utils/common-util_test.ts
index e07401b..c9ba060 100644
--- a/polygerrit-ui/app/utils/common-util_test.ts
+++ b/polygerrit-ui/app/utils/common-util_test.ts
@@ -9,6 +9,7 @@
areSetsEqual,
containsAll,
intersection,
+ difference,
} from './common-util';
suite('common-util tests', () => {
@@ -88,4 +89,11 @@
[foo1]
);
});
+
+ test('difference', () => {
+ assert.deepEqual(difference([1, 2, 3], []), [1, 2, 3]);
+ assert.deepEqual(difference([1, 2, 3], [2, 3, 4]), [1]);
+ assert.deepEqual(difference([1, 2, 3], [1, 2, 3]), []);
+ assert.deepEqual(difference([1, 2, 3], [4, 5, 6]), [1, 2, 3]);
+ });
});
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index f8ba4c3..515f2e7 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -9,7 +9,7 @@
RevisionPatchSetNum,
} from '../types/common';
import {EditRevisionInfo, ParsedChangeInfo} from '../types/types';
-import {check} from './common-util';
+import {assert} from './common-util';
/**
* @license
@@ -261,7 +261,7 @@
if (latest === EDIT) {
latest = allPatchSets[1].num;
}
- check(isNumber(latest), 'Latest patchset cannot be EDIT or PARENT.');
+ assert(isNumber(latest), 'Latest patchset cannot be EDIT or PARENT.');
return latest;
}
diff --git a/polygerrit-ui/app/utils/service-worker-util.ts b/polygerrit-ui/app/utils/service-worker-util.ts
index eb547ea..3cf0dce 100644
--- a/polygerrit-ui/app/utils/service-worker-util.ts
+++ b/polygerrit-ui/app/utils/service-worker-util.ts
@@ -15,8 +15,9 @@
export function filterAttentionChangesAfter(
changes: ParsedChangeInfo[],
account: AccountDetailInfo,
- latestUpdateTimestampMs: number
+ latestUpdateTimestampMs?: number
) {
+ if (!latestUpdateTimestampMs) return changes;
return changes.filter(change => {
const attention_set = change.attention_set![account._account_id!];
if (!attention_set.last_update) return false;
diff --git a/polygerrit-ui/app/workers/service-worker-class.ts b/polygerrit-ui/app/workers/service-worker-class.ts
new file mode 100644
index 0000000..966b6b0
--- /dev/null
+++ b/polygerrit-ui/app/workers/service-worker-class.ts
@@ -0,0 +1,56 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {ParsedChangeInfo} from '../types/types';
+import {getReason} from '../utils/attention-set-util';
+import {readResponsePayload} from '../elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {filterAttentionChangesAfter} from '../utils/service-worker-util';
+import {AccountDetailInfo} from '../api/rest-api';
+import {TRIGGER_NOTIFICATION_UPDATES_MS} from '../services/service-worker-installer';
+
+export class ServiceWorker {
+ constructor(private ctx: ServiceWorkerGlobalScope) {}
+
+ latestUpdateTimestampMs?: number;
+
+ showNotification(change: ParsedChangeInfo, account: AccountDetailInfo) {
+ const body = getReason(undefined, account, change);
+ // TODO(milutin): Implement event.action that
+ // focus on firstWindowClient and open change there.
+ // TODO(milutin): Add gerrit host icon
+ this.ctx.registration.showNotification(change.subject, {body});
+ }
+
+ async getChangesToNotify(account: AccountDetailInfo) {
+ // We throttle polling, since there can be many clients triggerring
+ // always only one service worker.
+ if (this.latestUpdateTimestampMs) {
+ const durationFromLatestUpdateMS =
+ Date.now() - this.latestUpdateTimestampMs;
+ if (durationFromLatestUpdateMS < TRIGGER_NOTIFICATION_UPDATES_MS) {
+ return [];
+ }
+ }
+ const changes = await this.getLatestAttentionSetChanges();
+ const latestAttentionChanges = filterAttentionChangesAfter(
+ changes,
+ account,
+ this.latestUpdateTimestampMs
+ );
+ this.latestUpdateTimestampMs = Date.now();
+ return latestAttentionChanges;
+ }
+
+ async getLatestAttentionSetChanges(): Promise<ParsedChangeInfo[]> {
+ // TODO(milutin): Implement more generic query builder
+ const response = await fetch(
+ '/changes/?O=1000081&S=0&n=25&q=attention%3Aself'
+ );
+ const payload = await readResponsePayload(response);
+ const changes = payload.parsed as unknown as ParsedChangeInfo[] | undefined;
+ return changes ?? [];
+ }
+}
diff --git a/polygerrit-ui/app/workers/service-worker-class_test.ts b/polygerrit-ui/app/workers/service-worker-class_test.ts
new file mode 100644
index 0000000..ecfef5e
--- /dev/null
+++ b/polygerrit-ui/app/workers/service-worker-class_test.ts
@@ -0,0 +1,50 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {Timestamp} from '../api/rest-api';
+import '../test/common-test-setup-karma';
+import {
+ createAccountDetailWithId,
+ createParsedChange,
+} from '../test/test-data-generators';
+import {ParsedChangeInfo} from '../types/types';
+import {parseDate} from '../utils/date-util';
+import {ServiceWorker} from './service-worker-class';
+
+suite('service worker class tests', () => {
+ let serviceWorker: ServiceWorker;
+
+ setup(() => {
+ const moctCtx = {
+ registration: {
+ showNotification: () => {},
+ },
+ } as {} as ServiceWorkerGlobalScope;
+ serviceWorker = new ServiceWorker(moctCtx);
+ });
+
+ test('notify about attention in t2 when time is t3', async () => {
+ const t1 = parseDate('2016-01-12 20:20:00' as Timestamp).getTime();
+ const t2 = '2016-01-12 20:30:00' as Timestamp;
+ const t3 = parseDate('2016-01-12 20:40:00' as Timestamp).getTime();
+ serviceWorker.latestUpdateTimestampMs = t1;
+ const account = createAccountDetailWithId();
+ const change: ParsedChangeInfo = {
+ ...createParsedChange(),
+ attention_set: {
+ [`${account._account_id}`]: {
+ account,
+ last_update: t2,
+ },
+ },
+ };
+ sinon.useFakeTimers(t3);
+ sinon
+ .stub(serviceWorker, 'getLatestAttentionSetChanges')
+ .returns(Promise.resolve([change]));
+ const changes = await serviceWorker.getChangesToNotify(account);
+ assert.equal(changes[0], change);
+ });
+});
diff --git a/polygerrit-ui/app/workers/service-worker.ts b/polygerrit-ui/app/workers/service-worker.ts
index 47bcbd1..ca1f5dc 100644
--- a/polygerrit-ui/app/workers/service-worker.ts
+++ b/polygerrit-ui/app/workers/service-worker.ts
@@ -5,14 +5,8 @@
*/
import {AccountDetailInfo} from '../api/rest-api';
-import {readResponsePayload} from '../elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
-import {
- ServiceWorkerMessageType,
- TRIGGER_NOTIFICATION_UPDATES_MS,
-} from '../services/service-worker-installer';
-import {ParsedChangeInfo} from '../types/types';
-import {getReason} from '../utils/attention-set-util';
-import {filterAttentionChangesAfter} from '../utils/service-worker-util';
+import {ServiceWorkerMessageType} from '../services/service-worker-installer';
+import {ServiceWorker} from './service-worker-class';
/**
* `self` is for a worker what `window` is for the web app. It is called
@@ -38,46 +32,5 @@
}
});
-class ServiceWorker {
- latestUpdateTimestampMs?: number;
-
- showNotification(change: ParsedChangeInfo, account: AccountDetailInfo) {
- const body = getReason(undefined, account, change);
- // TODO(milutin): Implement event.action that
- // focus on firstWindowClient and open change there.
- // TODO(milutin): Add gerrit host icon
- ctx.registration.showNotification(change.subject, {body});
- }
-
- async getChangesToNotify(account: AccountDetailInfo) {
- const changes = await serviceWorker.getLatestAttentionSetChanges();
- return filterAttentionChangesAfter(
- changes,
- account,
- this.latestUpdateTimestampMs!
- );
- }
-
- async getLatestAttentionSetChanges(): Promise<ParsedChangeInfo[]> {
- // We throttle polling, since there can be many clients triggerring
- // always only one service worker.
- if (this.latestUpdateTimestampMs) {
- const durationFromLatestUpdateMS =
- Date.now() - this.latestUpdateTimestampMs;
- if (durationFromLatestUpdateMS < TRIGGER_NOTIFICATION_UPDATES_MS) {
- return [];
- }
- }
- this.latestUpdateTimestampMs = Date.now();
- // TODO(milutin): Implement more generic query builder
- const response = await fetch(
- '/changes/?O=1000081&S=0&n=25&q=attention%3Aself'
- );
- const payload = await readResponsePayload(response);
- const changes = payload.parsed as unknown as ParsedChangeInfo[] | undefined;
- return changes ?? [];
- }
-}
-
/** Singleton instance being referenced in `onmessage` function above. */
-const serviceWorker = new ServiceWorker();
+const serviceWorker = new ServiceWorker(ctx);
