Skip to content

Commit

Permalink
Merge "Simplify how reviewers and ccs are mutated in reply-dialog"
Browse files Browse the repository at this point in the history
  • Loading branch information
@dhruvsrivastava
dhruvsrivastava authored and Gerrit Code Review committed Apr 8, 2022
2 parents 7f4ffb8 + 569d616 commit 90e17bf
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 51 deletions.
65 changes: 21 additions & 44 deletions polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import {FixIronA11yAnnouncer} from '../../../types/types';
import {
AccountAddition,
AccountInfoInput,
AccountInputDetail,
GrAccountList,
GroupInfoInput,
RawAccountInput,
Expand All @@ -62,8 +63,6 @@ import {
AttentionSetInput,
ChangeInfo,
CommentInput,
EmailAddress,
GroupId,
GroupInfo,
isAccount,
isDetailedLabelInfo,
Expand All @@ -83,10 +82,7 @@ import {
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrLabelScores} from '../gr-label-scores/gr-label-scores';
import {GrLabelScoreRow} from '../gr-label-score-row/gr-label-score-row';
import {
PolymerDeepPropertyChange,
PolymerSpliceChange,
} from '@polymer/polymer/interfaces';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {
areSetsEqual,
assertIsDefined,
Expand Down Expand Up @@ -503,46 +499,27 @@ export class GrReplyDialog extends DIPolymerElement {
return selectorEl?.selectedValue;
}

@observe('_ccs.splices')
_ccsChanged(splices: PolymerSpliceChange<AccountInfo[] | GroupInfo[]>) {
this._reviewerTypeChanged(splices, ReviewerType.CC);
}

@observe('_reviewers.splices')
_reviewersChanged(splices: PolymerSpliceChange<AccountInfo[] | GroupInfo[]>) {
this._reviewerTypeChanged(splices, ReviewerType.REVIEWER);
}

_reviewerTypeChanged(
splices: PolymerSpliceChange<AccountInfo[] | GroupInfo[]>,
reviewerType: ReviewerType
) {
if (splices && splices.indexSplices) {
this._reviewersMutated = true;
let key: AccountId | EmailAddress | GroupId | undefined;
let index;
let account;
accountAdded(e: CustomEvent<AccountInputDetail>) {
const account = e.detail.account;
const key = accountOrGroupKey(account);
const reviewerType =
(e.target as GrAccountList).getAttribute('id') === 'ccs'
? ReviewerType.CC
: ReviewerType.REVIEWER;
const isReviewer = ReviewerType.REVIEWER === reviewerType;
const array = isReviewer ? this._ccs : this._reviewers;
const index = array.findIndex(
reviewer => accountOrGroupKey(reviewer) === key
);
if (index >= 0) {
// Remove any accounts that already exist as a CC for reviewer
// or vice versa.
const isReviewer = ReviewerType.REVIEWER === reviewerType;
for (const splice of splices.indexSplices) {
for (let i = 0; i < splice.addedCount; i++) {
account = splice.object[splice.index + i];
key = accountOrGroupKey(account);
const array = isReviewer ? this._ccs : this._reviewers;
index = array.findIndex(
account => accountOrGroupKey(account) === key
);
if (index >= 0) {
this.splice(isReviewer ? '_ccs' : '_reviewers', index, 1);
const moveFrom = isReviewer ? 'CC' : 'reviewer';
const moveTo = isReviewer ? 'reviewer' : 'CC';
const id = account.name || key;
const message = `${id} moved from ${moveFrom} to ${moveTo}.`;
fireAlert(this, message);
}
}
}
this.splice(isReviewer ? '_ccs' : '_reviewers', index, 1);
const moveFrom = isReviewer ? 'CC' : 'reviewer';
const moveTo = isReviewer ? 'reviewer' : 'CC';
const id = account.name || key;
const message = `${id} moved from ${moveFrom} to ${moveTo}.`;
fireAlert(this, message);
}
}

Expand Down
2 changes: 2 additions & 0 deletions polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ export const htmlTemplate = html`
<gr-account-list
id="reviewers"
accounts="{{_reviewers}}"
on-account-added="accountAdded"
removable-values="[[change.removable_reviewers]]"
filter="[[filterReviewerSuggestion]]"
pending-confirmation="{{_reviewerPendingConfirmation}}"
Expand All @@ -284,6 +285,7 @@ export const htmlTemplate = html`
<gr-account-list
id="ccs"
accounts="{{_ccs}}"
on-account-added="accountAdded"
filter="[[filterCCSuggestion]]"
pending-confirmation="{{_ccPendingConfirmation}}"
allow-any-input=""
Expand Down
41 changes: 39 additions & 2 deletions polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,11 @@ suite('gr-reply-dialog tests', () => {
element._reviewers = [reviewer1, reviewer2, reviewer3];
element._ccs = [cc1, cc2, cc3, cc4];
element.push('_reviewers', cc1);
element.$.reviewers.dispatchEvent(
new CustomEvent('account-added', {
detail: {account: cc1},
})
);
flush();

assert.deepEqual(element._reviewers, [
Expand All @@ -1538,7 +1543,20 @@ suite('gr-reply-dialog tests', () => {
]);
assert.deepEqual(element._ccs, [cc2, cc3, cc4]);

element.push('_reviewers', cc4, cc3);
element.push('_reviewers', cc4);
element.$.reviewers.dispatchEvent(
new CustomEvent('account-added', {
detail: {account: cc4},
})
);
flush();

element.push('_reviewers', cc3);
element.$.reviewers.dispatchEvent(
new CustomEvent('account-added', {
detail: {account: cc3},
})
);
flush();

assert.deepEqual(element._reviewers, [
Expand Down Expand Up @@ -1617,12 +1635,31 @@ suite('gr-reply-dialog tests', () => {
element._reviewers = [reviewer1, reviewer2, reviewer3];
element._ccs = [cc1, cc2, cc3, cc4];
element.push('_ccs', reviewer1);
element.$.ccs.dispatchEvent(
new CustomEvent('account-added', {
detail: {account: reviewer1},
})
);

flush();

assert.deepEqual(element._reviewers, [reviewer2, reviewer3]);
assert.deepEqual(element._ccs, [cc1, cc2, cc3, cc4, reviewer1]);

element.push('_ccs', reviewer3, reviewer2);
element.push('_ccs', reviewer3);
element.$.ccs.dispatchEvent(
new CustomEvent('account-added', {
detail: {account: reviewer3},
})
);
flush();

element.push('_ccs', reviewer2);
element.$.ccs.dispatchEvent(
new CustomEvent('account-added', {
detail: {account: reviewer2},
})
);
flush();

assert.deepEqual(element._reviewers, []);
Expand Down
20 changes: 15 additions & 5 deletions polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {GrAccountEntry} from '../gr-account-entry/gr-account-entry';
import {GrAccountChip} from '../gr-account-chip/gr-account-chip';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {PaperInputElementExt} from '../../../types/types';
import {fireAlert} from '../../../utils/event-util';
import {fireAlert, fire} from '../../../utils/event-util';
import {accountOrGroupKey} from '../../../utils/account-util';

const VALID_EMAIL_ALERT = 'Please input a valid email.';
Expand All @@ -47,6 +47,9 @@ declare global {
interface HTMLElementTagNameMap {
'gr-account-list': GrAccountList;
}
interface HTMLElementEventMap {
'account-added': CustomEvent<AccountInputDetail>;
}
}

export interface GrAccountList {
Expand All @@ -55,6 +58,10 @@ export interface GrAccountList {
};
}

export interface AccountInputDetail {
account: AccountInput;
}

/** Supported input to be added */
export type RawAccountInput =
| string
Expand Down Expand Up @@ -100,7 +107,7 @@ function isGroupInfoInput(x: AccountInput): x is GroupInfoInput {
return !!input._group || !!input.id;
}

type AccountInput = AccountInfoInput | GroupInfoInput;
export type AccountInput = AccountInfoInput | GroupInfoInput;

export interface AccountAddition {
account?: AccountInfoInput;
Expand Down Expand Up @@ -213,9 +220,11 @@ export class GrAccountList extends PolymerElement {
// Append new account or group to the accounts property. We add our own
// internal properties to the account/group here, so we clone the object
// to avoid cluttering up the shared change object.
let account;
let group;
let itemTypeAdded = 'unknown';
if (isAccountObject(item)) {
const account = {...item.account, _pendingAdd: true};
account = {...item.account, _pendingAdd: true};
this.removeFromPendingRemoval(account);
this.push('accounts', account);
itemTypeAdded = 'account';
Expand All @@ -224,7 +233,7 @@ export class GrAccountList extends PolymerElement {
this.pendingConfirmation = item;
return;
}
const group = {...item.group, _pendingAdd: true, _group: true};
group = {...item.group, _pendingAdd: true, _group: true};
this.push('accounts', group);
this.removeFromPendingRemoval(group);
itemTypeAdded = 'group';
Expand All @@ -236,13 +245,14 @@ export class GrAccountList extends PolymerElement {
fireAlert(this, VALID_EMAIL_ALERT);
return false;
} else {
const account = {email: item as EmailAddress, _pendingAdd: true};
account = {email: item as EmailAddress, _pendingAdd: true};
this.push('accounts', account);
this.removeFromPendingRemoval(account);
itemTypeAdded = 'email';
}
}

fire(this, 'account-added', {account: (account ?? group)! as AccountInput});
this.reporting.reportInteraction(`Add to ${this.id}`, {itemTypeAdded});
this.pendingConfirmation = null;
return true;
Expand Down

0 comments on commit 90e17bf

Please sign in to comment.