From 2235ecbfd3ffb530ad02281e2acc30010b5124d2 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Thu, 7 Apr 2022 11:58:19 +0200 Subject: [PATCH] Remove type AccountInput and GroupObjectInput Input provided by GrAccountList is of type SuggestionItem which has count in the type SuggestedReviewerInfo. https://cs.opensource.google/gerrit/gerrit/gerrit/+/master:polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts;l=209?q=%22return%20suggestions.map(suggestion%20%3D%3E%22 Verified via dev helper that the addAccountItem() method does have "count" field as part of it's input. Release-Notes: skip Change-Id: I6dd0b88fd8ca45e846db7ee6ef4db9e6512e0429 --- .../change/gr-reply-dialog/gr-reply-dialog.ts | 9 ++-- .../gr-reply-dialog/gr-reply-dialog_test.ts | 4 ++ .../shared/gr-account-list/gr-account-list.ts | 42 ++++++++----------- .../gr-account-list/gr-account-list_test.ts | 18 ++++---- 4 files changed, 38 insertions(+), 35 deletions(-) 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 87b699e0466a..d25f3c18c39e 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 @@ -54,7 +54,6 @@ import { AccountInfoInput, GrAccountList, GroupInfoInput, - GroupObjectInput, RawAccountInput, } from '../../shared/gr-account-list/gr-account-list'; import { @@ -78,6 +77,7 @@ import { ReviewInput, ReviewResult, ServerInfo, + SuggestedReviewerGroupInfo, Suggestion, } from '../../../types/common'; import {GrButton} from '../../shared/gr-button/gr-button'; @@ -275,7 +275,7 @@ export class GrReplyDialog extends DIPolymerElement { _attentionCcsCount = 0; @property({type: Object, observer: '_reviewerPendingConfirmationUpdated'}) - _ccPendingConfirmation: GroupObjectInput | null = null; + _ccPendingConfirmation: SuggestedReviewerGroupInfo | null = null; @property({ type: String, @@ -290,7 +290,7 @@ export class GrReplyDialog extends DIPolymerElement { _uploader?: AccountInfo; @property({type: Object}) - _pendingConfirmationDetails: GroupObjectInput | null = null; + _pendingConfirmationDetails: SuggestedReviewerGroupInfo | null = null; @property({type: Boolean}) _includeComments = true; @@ -299,7 +299,7 @@ export class GrReplyDialog extends DIPolymerElement { _reviewers: (AccountInfo | GroupInfo)[] = []; @property({type: Object, observer: '_reviewerPendingConfirmationUpdated'}) - _reviewerPendingConfirmation: GroupObjectInput | null = null; + _reviewerPendingConfirmation: SuggestedReviewerGroupInfo | null = null; @property({type: Boolean, observer: '_handleHeightChanged'}) _previewFormatting = false; @@ -409,6 +409,7 @@ export class GrReplyDialog extends DIPolymerElement { // elements/shared/gr-account-list/gr-account-list.js#addAccountItem this.$.reviewers.addAccountItem({ account: (e as CustomEvent).detail.reviewer, + count: 1, }); }); 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 dc6820eb4d1a..79fbaa0d08bb 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 @@ -1138,11 +1138,13 @@ suite('gr-reply-dialog tests', () => { element._ccPendingConfirmation = { group, confirm: false, + count: 1, }; } else { element._reviewerPendingConfirmation = { group, confirm: false, + count: 1, }; } flush(); @@ -1203,11 +1205,13 @@ suite('gr-reply-dialog tests', () => { element._ccPendingConfirmation = { group, confirm: false, + count: 1, }; } else { element._reviewerPendingConfirmation = { group, confirm: false, + count: 1, }; } 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 5f0cf7affd92..c63a5091c51b 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 @@ -27,6 +27,8 @@ import { AccountInfo, GroupInfo, EmailAddress, + SuggestedReviewerGroupInfo, + SuggestedReviewerAccountInfo, } from '../../../types/common'; import { ReviewerSuggestionsProvider, @@ -53,31 +55,23 @@ export interface GrAccountList { }; } -/** - * For item added with account info - */ -export interface AccountObjectInput { - account: AccountInfo; -} - -/** - * For item added with group info - */ -export interface GroupObjectInput { - group: GroupInfo; - confirm: boolean; -} - /** Supported input to be added */ -export type RawAccountInput = string | AccountObjectInput | GroupObjectInput; - -// type guards for AccountObjectInput and GroupObjectInput -function isAccountObject(x: RawAccountInput): x is AccountObjectInput { - return !!(x as AccountObjectInput).account; +export type RawAccountInput = + | string + | SuggestedReviewerAccountInfo + | SuggestedReviewerGroupInfo; + +// type guards for SuggestedReviewerAccountInfo and SuggestedReviewerGroupInfo +function isAccountObject( + x: RawAccountInput +): x is SuggestedReviewerAccountInfo { + return !!(x as SuggestedReviewerAccountInfo).account; } -function isGroupObjectInput(x: RawAccountInput): x is GroupObjectInput { - return !!(x as GroupObjectInput).group; +function isSuggestedReviewerGroupInfo( + x: RawAccountInput +): x is SuggestedReviewerGroupInfo { + return !!(x as SuggestedReviewerGroupInfo).group; } // Internal input type with account info @@ -150,7 +144,7 @@ export class GrAccountList extends PolymerElement { * Needed for template checking since value is initially set to null. */ @property({type: Object, notify: true}) - pendingConfirmation: GroupObjectInput | null = null; + pendingConfirmation: SuggestedReviewerGroupInfo | null = null; @property({type: Boolean}) readonly = false; @@ -225,7 +219,7 @@ export class GrAccountList extends PolymerElement { this.removeFromPendingRemoval(account); this.push('accounts', account); itemTypeAdded = 'account'; - } else if (isGroupObjectInput(item)) { + } else if (isSuggestedReviewerGroupInfo(item)) { if (item.confirm) { this.pendingConfirmation = item; return; 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 23e5a72d37c7..d77dec3a5bf2 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 @@ -25,8 +25,9 @@ import { AccountId, AccountInfo, EmailAddress, + GroupBaseInfo, GroupId, - GroupInfo, + GroupName, SuggestedReviewerAccountInfo, Suggestion, } from '../../../types/common'; @@ -64,11 +65,12 @@ suite('gr-account-list tests', () => { _account_id: accountId as AccountId, }; }; - const makeGroup: () => GroupInfo = function () { + const makeGroup: () => GroupBaseInfo = function () { const groupId = `group${++_nextAccountId}`; return { id: groupId as GroupId, _group: true, + name: 'abcd' as GroupName, }; }; @@ -116,7 +118,7 @@ suite('gr-account-list tests', () => { // New accounts are added to end with pendingAdd class. const newAccount = makeAccount(); - handleAdd({account: newAccount}); + handleAdd({account: newAccount, count: 1}); flush(); chips = getChips(); assert.equal(chips.length, 3); @@ -160,7 +162,7 @@ suite('gr-account-list tests', () => { // New groups are added to end with pendingAdd and group classes. const newGroup = makeGroup(); - handleAdd({group: newGroup, confirm: false}); + handleAdd({group: newGroup, confirm: false, count: 1}); flush(); chips = getChips(); assert.equal(chips.length, 2); @@ -301,9 +303,9 @@ suite('gr-account-list tests', () => { assert.equal(element.additions().length, 0); const newAccount = makeAccount(); - handleAdd({account: newAccount}); + handleAdd({account: newAccount, count: 1}); const newGroup = makeGroup(); - handleAdd({group: newGroup, confirm: false}); + handleAdd({group: newGroup, confirm: false, count: 1}); assert.deepEqual(element.additions(), [ { @@ -317,6 +319,7 @@ suite('gr-account-list tests', () => { id: newGroup.id, _group: true, _pendingAdd: true, + name: 'abcd' as GroupName, }, }, ]); @@ -346,6 +349,7 @@ suite('gr-account-list tests', () => { _group: true, _pendingAdd: true, confirmed: true, + name: 'abcd' as GroupName, }, }, ]); @@ -362,7 +366,7 @@ suite('gr-account-list tests', () => { test('max-count', () => { element.maxCount = 1; const acct = makeAccount(); - handleAdd({account: acct}); + handleAdd({account: acct, count: 1}); flush(); assert.isTrue(element.$.entry.hasAttribute('hidden')); });