Check if user added valid account or group
Making sure we do not add 'undefined' account/group in send reply. Js
error was in logs where user was not able to send reply.
Release-Notes: skip
Google-Bug-Id: b/269612438
Change-Id: Iaa61ece97b450aed9ad766e4a3837cf0af9e7e77
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 65d8859..9034259 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
@@ -38,18 +38,17 @@
import {ReviewerState} from '../../../api/rest-api';
const VALID_EMAIL_ALERT = 'Please input a valid email.';
+const VALID_USER_GROUP_ALERT = 'Please input a valid user or group.';
declare global {
interface HTMLElementEventMap {
'accounts-changed': ValueChangedEvent<(AccountInfo | GroupInfo)[]>;
'pending-confirmation-changed': ValueChangedEvent<SuggestedReviewerGroupInfo | null>;
+ 'account-added': CustomEvent<{account: AccountInfo | GroupInfo}>;
}
interface HTMLElementTagNameMap {
'gr-account-list': GrAccountList;
}
- interface HTMLElementEventMap {
- 'account-added': CustomEvent<AccountInputDetail>;
- }
}
export interface AccountInputDetail {
account: AccountInput;
@@ -264,24 +263,28 @@
this.addAccountItem(item);
}
- addAccountItem(item: RawAccountInput) {
+ /**
+ * Check if account or group is valid and add it.
+ *
+ * @return true if account or group is added.
+ */
+ addAccountItem(item: RawAccountInput): boolean {
// 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 accountOrGroup: AccountInfo | GroupInfo | undefined;
let itemTypeAdded = 'unknown';
if (isAccountObject(item)) {
- account = {...item.account};
- this.accounts.push(account);
+ accountOrGroup = {...item.account};
+ this.accounts.push(accountOrGroup);
itemTypeAdded = 'account';
} else if (isSuggestedReviewerGroupInfo(item)) {
if (item.confirm) {
this.pendingConfirmation = item;
- return;
+ return false;
}
- group = {...item.group};
- this.accounts.push(group);
+ accountOrGroup = {...item.group};
+ this.accounts.push(accountOrGroup);
itemTypeAdded = 'group';
} else if (this.allowAnyInput) {
if (!item.includes('@')) {
@@ -291,13 +294,17 @@
fireAlert(this, VALID_EMAIL_ALERT);
return false;
} else {
- account = {email: item as EmailAddress};
- this.accounts.push(account);
+ accountOrGroup = {email: item as EmailAddress};
+ this.accounts.push(accountOrGroup);
itemTypeAdded = 'email';
}
}
+ if (!accountOrGroup) {
+ fireAlert(this, VALID_USER_GROUP_ALERT);
+ return false;
+ }
fire(this, 'accounts-changed', {value: this.accounts.slice()});
- fire(this, 'account-added', {account: (account ?? group)! as AccountInput});
+ fire(this, 'account-added', {account: accountOrGroup});
this.reporting.reportInteraction(`Add to ${this.id}`, {itemTypeAdded});
this.pendingConfirmation = null;
this.requestUpdate();
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 cc2723e..814b623 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
@@ -289,6 +289,15 @@
assert.isFalse(element.computeRemovable(newAccount));
});
+ test('addAccountItem with invalid item', () => {
+ const toastHandler = sinon.stub();
+ element.allowAnyInput = false;
+ element.addEventListener(EventType.SHOW_ALERT, toastHandler);
+ const result = element.addAccountItem('test');
+ assert.isFalse(result);
+ assert.isTrue(toastHandler.called);
+ });
+
test('submitEntryText', async () => {
element.allowAnyInput = true;
await element.updateComplete;
