Remove pendingRemoval state from GrAccountList
The confirmed property [1] for groups is only relevant when adding a
large group but not when removing the group.
Aim is to make GrAccountList maintain less transient state and be
less smart and let GrReplyDialog be the overall owner of which
account is added/removed.
Testing relies on making sure the existing tests pass which are
checking for the accounts that are removed [2].
[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#reviewer-input
[2] https://cs.opensource.google/gerrit/gerrit/gerrit/+/master:polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts;l=1973?q=%22assert.equal(mutations.length,%205);%22&sq=
Release-Notes: skip
Google-bug-id: b/236921879
Change-Id: I24ecb677031b657b38a4686e7fadc11a9797df6b
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 bfd92d6..cc736cb 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
@@ -26,8 +26,8 @@
SpecialFilePath,
} from '../../../constants/constants';
import {
+ accountKey,
accountOrGroupKey,
- isReviewerOrCC,
mapReviewer,
removeServiceUsers,
} from '../../../utils/account-util';
@@ -63,6 +63,7 @@
ServerInfo,
SuggestedReviewerGroupInfo,
Suggestion,
+ isGroup,
} from '../../../types/common';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrLabelScores} from '../gr-label-scores/gr-label-scores';
@@ -99,7 +100,7 @@
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
-import {ConfigInfo, LabelNameToValuesMap} from '../../../api/rest-api';
+import {ConfigInfo, GroupId, LabelNameToValuesMap} from '../../../api/rest-api';
import {css, html, PropertyValues, LitElement} from 'lit';
import {sharedStyles} from '../../../styles/shared-styles';
import {when} from 'lit/directives/when';
@@ -1281,7 +1282,43 @@
return isResolvedPatchsetLevelComment ? 'resolved' : 'unresolved';
}
- computeReviewers(change: ChangeInfo) {
+ /**
+ * Get the list of users removed.
+ * A user is removed if they were initially present in change.reviewer[state]
+ * and not present in currentAccounts
+ */
+
+ private getRemovals(
+ state: ReviewerState,
+ currentAccounts: AccountInput[]
+ ): AccountInfo[] {
+ const existingAccounts = this.change?.reviewers[state] ?? [];
+ return existingAccounts.filter(
+ existingAccount =>
+ !currentAccounts.some(
+ currentAccount =>
+ accountOrGroupKey(currentAccount) ===
+ accountOrGroupKey(existingAccount)
+ )
+ );
+ }
+
+ private mapAccountToReviewInput(
+ account: AccountInfo | GroupInfo
+ ): ReviewerInput {
+ if (isAccount(account)) {
+ return {
+ reviewer: accountKey(account),
+ state: ReviewerState.REMOVED,
+ };
+ } else if (isGroup(account)) {
+ const reviewer = decodeURIComponent(account.id) as GroupId;
+ return {reviewer, state: ReviewerState.REMOVED};
+ }
+ throw new Error('Must be either an account or a group.');
+ }
+
+ computeReviewers() {
const reviewers: ReviewerInput[] = [];
const addToReviewInput = (
additions: AccountAddition[],
@@ -1295,28 +1332,34 @@
};
addToReviewInput(this.reviewersList!.additions(), ReviewerState.REVIEWER);
addToReviewInput(this.ccsList!.additions(), ReviewerState.CC);
- addToReviewInput(
- this.reviewersList!.removals().filter(
+
+ let removals;
+ removals = this.getRemovals(
+ ReviewerState.REVIEWER,
+ this.reviewersList?.accounts ?? []
+ )
+ .filter(
r =>
- isReviewerOrCC(change, r) &&
- // ignore removal from reviewer request if being added to CC
+ // ignore removal from reviewer request if being added as CC
!this.ccsList!.additions().some(
- account => mapReviewer(account).reviewer === mapReviewer(r).reviewer
+ account => mapReviewer(account).reviewer === accountOrGroupKey(r)
)
- ),
- ReviewerState.REMOVED
- );
- addToReviewInput(
- this.ccsList!.removals().filter(
+ )
+ .map(this.mapAccountToReviewInput);
+ reviewers.push(...removals);
+
+ removals = this.getRemovals(ReviewerState.CC, this.ccsList?.accounts ?? [])
+ .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
+ account => mapReviewer(account).reviewer === accountOrGroupKey(r)
)
- ),
- ReviewerState.REMOVED
- );
+ )
+ .map(this.mapAccountToReviewInput);
+
+ reviewers.push(...removals);
+
return reviewers;
}
@@ -1367,7 +1410,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);
@@ -1816,7 +1859,6 @@
})
);
queryAndAssert<GrTextarea>(this, 'gr-textarea').closeDropdown();
- this.reviewersList?.clearPendingRemovals();
this.rebuildReviewerArrays();
}
