Add CC with bulk action
https://imgur.com/a/6qR0E9J
Google-Bug-Id: b/216478677
Release-Notes: skip
Change-Id: I376709c5db4d3e8c61d073c5ad9878d95ea35532
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
index 227115f..7029ad6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
@@ -5,7 +5,7 @@
*/
import {html, LitElement} from 'lit';
import {customElement, query, state} from 'lit/decorators';
-import {ProgressStatus} from '../../../constants/constants';
+import {ProgressStatus, ReviewerState} from '../../../constants/constants';
import {bulkActionsModelToken} from '../../../models/bulk-actions/bulk-actions-model';
import {resolve} from '../../../models/dependency';
import {AccountInfo, ChangeInfo} from '../../../types/common';
@@ -21,12 +21,29 @@
} from '../../../scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider';
import '../../shared/gr-account-list/gr-account-list';
+const SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE: Record<
+ ReviewerState,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES
+> = {
+ REVIEWER: SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
+ CC: SUGGESTIONS_PROVIDERS_USERS_TYPES.CC,
+ REMOVED: SUGGESTIONS_PROVIDERS_USERS_TYPES.ANY,
+};
+
@customElement('gr-change-list-reviewer-flow')
export class GrChangeListReviewerFlow extends LitElement {
@state() private selectedChanges: ChangeInfo[] = [];
- // given to gr-account-list to mutate
- @state() private updatedReviewers: AccountInfo[] = [];
+ // contents are given to gr-account-lists to mutate
+ @state() private updatedAccountsByReviewerState: Map<
+ ReviewerState,
+ AccountInfo[]
+ > = new Map();
+
+ @state() private suggestionsProviderByReviewerState: Map<
+ ReviewerState,
+ ReviewerSuggestionsProvider
+ > = new Map();
@state() private progressByChange = new Map<ChangeInfo, ProgressStatus>();
@@ -36,8 +53,6 @@
private restApiService = getAppContext().restApiService;
- private suggestionsProvider?: ReviewerSuggestionsProvider;
-
override connectedCallback(): void {
super.connectedCallback();
subscribe(
@@ -71,14 +86,16 @@
<div slot="header">Add Reviewer / CC</div>
<div slot="main">
<div>
- <span>Reviewers:</span>
- <gr-account-list
- .accounts=${this.updatedReviewers}
- .removableValues=${[]}
- .suggestionsProvider=${this.suggestionsProvider}
- .placeholder=${'Add reviewer...'}
- >
- </gr-account-list>
+ <span>Reviewers</span>
+ ${this.renderAccountList(
+ ReviewerState.REVIEWER,
+ 'reviewer-list',
+ 'Add reviewer'
+ )}
+ </div>
+ <div>
+ <span>CC</span>
+ ${this.renderAccountList(ReviewerState.CC, 'cc-list', 'Add CC')}
</div>
</div>
</gr-dialog>
@@ -86,27 +103,53 @@
`;
}
+ private renderAccountList(
+ reviewerState: ReviewerState,
+ id: string,
+ placeholder: string
+ ) {
+ const updatedAccounts =
+ this.updatedAccountsByReviewerState.get(reviewerState);
+ const suggestionsProvider =
+ this.suggestionsProviderByReviewerState.get(reviewerState);
+ if (!updatedAccounts || !suggestionsProvider) {
+ return;
+ }
+ return html`
+ <gr-account-list
+ id=${id}
+ .accounts=${updatedAccounts}
+ .removableValues=${[]}
+ .suggestionsProvider=${suggestionsProvider}
+ .placeholder=${placeholder}
+ >
+ </gr-account-list>
+ `;
+ }
+
private resetFlow() {
this.progressByChange = new Map(
this.selectedChanges.map(change => [change, ProgressStatus.NOT_STARTED])
);
- this.updatedReviewers = this.getCurrentReviewers();
- if (this.selectedChanges.length === 0) {
- return;
+ for (const state of [ReviewerState.REVIEWER, ReviewerState.CC]) {
+ this.updatedAccountsByReviewerState.set(
+ state,
+ this.getCurrentAccounts(state)
+ );
+ if (this.selectedChanges.length > 0) {
+ this.suggestionsProviderByReviewerState.set(
+ state,
+ this.createSuggestionsProvider(state)
+ );
+ }
}
- this.suggestionsProvider = GrReviewerSuggestionsProvider.create(
- this.restApiService,
- // TODO: fan out and get suggestions allowed by all changes
- this.selectedChanges[0]._number,
- SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER
- );
- this.suggestionsProvider.init();
+ this.requestUpdate();
}
private onConfirm(overallStatus: ProgressStatus) {
switch (overallStatus) {
case ProgressStatus.NOT_STARTED:
- this.saveReviewers();
+ this.saveChanges();
break;
case ProgressStatus.SUCCESSFUL:
this.overlay.close();
@@ -114,12 +157,12 @@
}
}
- private saveReviewers() {
+ private saveChanges() {
this.progressByChange = new Map(
this.selectedChanges.map(change => [change, ProgressStatus.RUNNING])
);
const inFlightActions = this.getBulkActionsModel().addReviewers(
- this.getAddedReviewers()
+ this.updatedAccountsByReviewerState
);
for (let index = 0; index < this.selectedChanges.length; index++) {
const change = this.selectedChanges[index];
@@ -149,9 +192,9 @@
: 'Close';
}
- private getCurrentReviewers() {
- const reviewersPerChange = this.selectedChanges.map(change =>
- Object.values(change.reviewers).flat()
+ private getCurrentAccounts(reviewerState: ReviewerState) {
+ const reviewersPerChange = this.selectedChanges.map(
+ change => change.reviewers[reviewerState] ?? []
);
if (reviewersPerChange.length === 0) {
return [];
@@ -162,11 +205,17 @@
);
}
- private getAddedReviewers(): AccountInfo[] {
- const oldReviewers = this.getCurrentReviewers();
- return this.updatedReviewers.filter(
- reviewer => !oldReviewers.includes(reviewer)
+ private createSuggestionsProvider(
+ state: ReviewerState
+ ): ReviewerSuggestionsProvider {
+ const suggestionsProvider = GrReviewerSuggestionsProvider.create(
+ this.restApiService,
+ // TODO: fan out and get suggestions allowed by all changes
+ this.selectedChanges[0]._number,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE[state]
);
+ suggestionsProvider.init();
+ return suggestionsProvider;
}
private getOverallStatus() {
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 131b823..4e9e790 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
@@ -35,19 +35,25 @@
createAccountWithIdNameAndEmail(0),
createAccountWithIdNameAndEmail(1),
createAccountWithIdNameAndEmail(2),
+ createAccountWithIdNameAndEmail(3),
+ createAccountWithIdNameAndEmail(4),
+ createAccountWithIdNameAndEmail(5),
];
const changes: ChangeInfo[] = [
{
...createChange(),
_number: 1 as NumericChangeId,
subject: 'Subject 1',
- reviewers: {REVIEWER: [accounts[0], accounts[1]]},
+ reviewers: {
+ REVIEWER: [accounts[0], accounts[1]],
+ CC: [accounts[3], accounts[4]],
+ },
},
{
...createChange(),
_number: 2 as NumericChangeId,
subject: 'Subject 2',
- reviewers: {REVIEWER: [accounts[0]]},
+ reviewers: {REVIEWER: [accounts[0]], CC: [accounts[3]]},
},
];
@@ -103,8 +109,12 @@
<div slot="header">Add Reviewer / CC</div>
<div slot="main">
<div>
- <span>Reviewers:</span>
- <gr-account-list></gr-account-list>
+ <span>Reviewers</span>
+ <gr-account-list id="reviewer-list"></gr-account-list>
+ </div>
+ <div>
+ <span>CC</span>
+ <gr-account-list id="cc-list"></gr-account-list>
</div>
</div>
</gr-dialog>
@@ -141,23 +151,23 @@
assert.isTrue(overlay.opened);
});
- suite('dialog', () => {
- let markActivePromises: MockPromise<Response>[];
+ suite('dialog flow', () => {
+ let saveChangesPromises: MockPromise<Response>[];
let saveChangeReviewStub: sinon.SinonStub;
let dialog: GrDialog;
async function resolvePromises() {
- markActivePromises[0].resolve(new Response());
- markActivePromises[1].resolve(new Response());
+ saveChangesPromises[0].resolve(new Response());
+ saveChangesPromises[1].resolve(new Response());
await element.updateComplete;
}
setup(async () => {
- markActivePromises = [];
+ saveChangesPromises = [];
saveChangeReviewStub = stubRestApi('saveChangeReview');
for (let i = 0; i < changes.length; i++) {
const promise = mockPromise<Response>();
- markActivePromises.push(promise);
+ saveChangesPromises.push(promise);
saveChangeReviewStub
.withArgs(changes[i]._number, sinon.match.any, sinon.match.any)
.returns(promise);
@@ -169,21 +179,32 @@
await dialog.updateComplete;
});
- test('only lists reviewers shared by all changes', async () => {
- const accountList = queryAndAssert<GrAccountList>(
+ test('only lists reviewers/CCs shared by all changes', async () => {
+ const reviewerList = queryAndAssert<GrAccountList>(
dialog,
- 'gr-account-list'
+ 'gr-account-list#reviewer-list'
+ );
+ const ccList = queryAndAssert<GrAccountList>(
+ dialog,
+ 'gr-account-list#cc-list'
);
// does not include account 1
- assert.sameMembers(accountList.accounts, [accounts[0]]);
+ assert.sameMembers(reviewerList.accounts, [accounts[0]]);
+ // does not include account 4
+ assert.sameMembers(ccList.accounts, [accounts[3]]);
});
- test('adds reviewer', async () => {
- const accountList = queryAndAssert<GrAccountList>(
+ test('adds reviewer & CC', async () => {
+ const reviewerList = queryAndAssert<GrAccountList>(
dialog,
- 'gr-account-list'
+ 'gr-account-list#reviewer-list'
);
- accountList.accounts.push(accounts[2]);
+ const ccList = queryAndAssert<GrAccountList>(
+ dialog,
+ 'gr-account-list#cc-list'
+ );
+ reviewerList.accounts.push(accounts[2]);
+ ccList.accounts.push(accounts[5]);
await flush();
dialog.confirmButton!.click();
await element.updateComplete;
@@ -195,6 +216,7 @@
{
reviewers: [
{reviewer: accounts[2]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: accounts[5]._account_id, state: ReviewerState.CC},
],
},
]);
@@ -204,6 +226,7 @@
{
reviewers: [
{reviewer: accounts[2]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: accounts[5]._account_id, state: ReviewerState.CC},
],
},
]);
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
index b4fba63..a3bd530 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
@@ -16,7 +16,7 @@
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {define} from '../dependency';
import {select} from '../../utils/observable-util';
-import {ReviewInput} from '../../types/common';
+import {ReviewInput, ReviewerInput} from '../../types/common';
export const bulkActionsModelToken =
define<BulkActionsModel>('bulk-actions-model');
@@ -129,25 +129,25 @@
});
}
- addReviewers(addedReviewers: AccountInfo[]): Promise<Response>[] {
+ addReviewers(
+ changedReviewers: Map<ReviewerState, AccountInfo[]>
+ ): Promise<Response>[] {
const current = this.subject$.getValue();
const changes = current.selectedChangeNums.map(
changeNum => current.allChanges.get(changeNum)!
);
return changes.map(change => {
- const reviewersNewToChange = addedReviewers.filter(
- account => !change.reviewers[ReviewerState.REVIEWER]?.includes(account)
+ const reviewersNewToChange = [
+ ReviewerState.REVIEWER,
+ ReviewerState.CC,
+ ].flatMap(state =>
+ this.getNewReviewersToChange(change, state, changedReviewers)
);
if (reviewersNewToChange.length === 0) {
return Promise.resolve(new Response());
}
const reviewInput: ReviewInput = {
- reviewers: reviewersNewToChange.map(account => {
- return {
- state: ReviewerState.REVIEWER,
- reviewer: account._account_id!,
- };
- }),
+ reviewers: reviewersNewToChange,
};
return this.restApiService.saveChangeReview(
change._number,
@@ -212,5 +212,20 @@
};
}
+ private getNewReviewersToChange(
+ change: ChangeInfo,
+ state: ReviewerState,
+ changedReviewers: Map<ReviewerState, AccountInfo[]>
+ ): ReviewerInput[] {
+ return (
+ changedReviewers
+ .get(state)
+ ?.filter(account => !change.reviewers[state]?.includes(account))
+ .map(account => {
+ return {state, reviewer: account._account_id!};
+ }) ?? []
+ );
+ }
+
finalize() {}
}
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
index 763df9b..8f2330d 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
@@ -158,22 +158,21 @@
const accounts: AccountInfo[] = [
createAccountWithIdNameAndEmail(0),
createAccountWithIdNameAndEmail(1),
- createAccountWithIdNameAndEmail(2),
];
const changes: ChangeInfo[] = [
{
...createChange(),
_number: 1 as NumericChangeId,
subject: 'Subject 1',
- reviewers: {REVIEWER: [accounts[0], accounts[1]]},
- removable_reviewers: [accounts[0]],
+ reviewers: {
+ REVIEWER: [accounts[0]],
+ CC: [accounts[1]],
+ },
},
{
...createChange(),
_number: 2 as NumericChangeId,
subject: 'Subject 2',
- reviewers: {REVIEWER: [accounts[0]]},
- removable_reviewers: [accounts[0]],
},
];
let saveChangeReviewStub: sinon.SinonStub;
@@ -191,18 +190,23 @@
bulkActionsModel.addSelectedChangeNum(changes[1]._number);
});
- test('adds reviewers only to changes that need it', async () => {
- bulkActionsModel.addReviewers([accounts[1]]);
+ test('adds reviewers/cc only to changes that need it', async () => {
+ bulkActionsModel.addReviewers(
+ new Map([
+ [ReviewerState.REVIEWER, [accounts[0]]],
+ [ReviewerState.CC, [accounts[1]]],
+ ])
+ );
- // changes[0] is not updated since it already has accounts[1]
- // as a reviewer
+ // changes[0] is not updated since it already has the reviewer & CC
assert.isTrue(saveChangeReviewStub.calledOnce);
assert.sameDeepOrderedMembers(saveChangeReviewStub.firstCall.args, [
changes[1]._number,
'current',
{
reviewers: [
- {reviewer: accounts[1]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: accounts[0]._account_id, state: ReviewerState.REVIEWER},
+ {reviewer: accounts[1]._account_id, state: ReviewerState.CC},
],
},
]);
