Move getOverallStatus to bulk-flow-util Both reviewer and voting flow need the overall status which can be moved to util. The method was missing the failure condition which is now added. Release-Notes: skip Google-bug-id: b/216478680 Change-Id: I5c823f98509ecedf06761c9207b4816302941bb7
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts index 039867b..ec2bbe9 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
@@ -33,6 +33,7 @@ import {fireAlert, fireReload} from '../../../utils/event-util'; import '../../shared/gr-dialog/gr-dialog'; import '../../change/gr-label-score-row/gr-label-score-row'; +import {getOverallStatus} from '../../../utils/bulk-flow-util'; @customElement('gr-change-list-bulk-vote-flow') export class GrChangeListBulkVoteFlow extends LitElement { @@ -42,7 +43,7 @@ @state() selectedChanges: ChangeInfo[] = []; - @state() progress: Map<NumericChangeId, ProgressStatus> = new Map(); + @state() progressByChange: Map<NumericChangeId, ProgressStatus> = new Map(); @query('#actionOverlay') actionOverlay!: GrOverlay; @@ -82,7 +83,10 @@ subscribe( this, this.getBulkActionsModel().selectedChanges$, - selectedChanges => (this.selectedChanges = selectedChanges) + selectedChanges => { + this.selectedChanges = selectedChanges; + this.resetFlow(); + } ); subscribe( this, @@ -156,23 +160,25 @@ </div>`; } + private resetFlow() { + this.progressByChange = new Map( + this.selectedChanges.map(change => [ + change._number, + ProgressStatus.NOT_STARTED, + ]) + ); + } + private isConfirmEnabled() { // Action is allowed if none of the changes have any bulk action performed // on them. In case an error happens then we keep the button disabled. - return this.selectedChanges - .map(change => this.getStatus(change._number)) - .every(status => status === ProgressStatus.NOT_STARTED); - } - - private getStatus(changeNum: NumericChangeId) { - return this.progress.get(changeNum) ?? ProgressStatus.NOT_STARTED; + return ( + getOverallStatus(this.progressByChange) === ProgressStatus.NOT_STARTED + ); } private isCancelEnabled() { - for (const status of this.progress.values()) { - if (status === ProgressStatus.RUNNING) return false; - } - return true; + return getOverallStatus(this.progressByChange) !== ProgressStatus.RUNNING; } private handleClose() { @@ -182,14 +188,14 @@ } private handleConfirm() { - this.progress.clear(); + this.progressByChange.clear(); const reviewInput: ReviewInput = { labels: this.getLabelValues( this.computeCommonPermittedLabels(this.computePermittedLabels()) ), }; for (const change of this.selectedChanges) { - this.progress.set(change._number, ProgressStatus.RUNNING); + this.progressByChange.set(change._number, ProgressStatus.RUNNING); } this.requestUpdate(); const promises = this.getBulkActionsModel().voteChanges(reviewInput); @@ -197,10 +203,10 @@ const changeNum = this.selectedChanges[index]._number; promises[index] .then(() => { - this.progress.set(changeNum, ProgressStatus.SUCCESSFUL); + this.progressByChange.set(changeNum, ProgressStatus.SUCCESSFUL); }) .catch(() => { - this.progress.set(changeNum, ProgressStatus.FAILED); + this.progressByChange.set(changeNum, ProgressStatus.FAILED); }) .finally(() => { this.requestUpdate();
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts index 51b7f7e..490d1f1 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow_test.ts
@@ -245,14 +245,15 @@ ); assert.equal( - element.progress.get(1 as NumericChangeId), + element.progressByChange.get(1 as NumericChangeId), ProgressStatus.RUNNING ); saveChangeReview.resolve({...new Response(), status: 200}); await waitUntil( () => - element.progress.get(1 as NumericChangeId) === ProgressStatus.SUCCESSFUL + element.progressByChange.get(1 as NumericChangeId) === + ProgressStatus.SUCCESSFUL ); assert.isTrue( @@ -263,7 +264,7 @@ ); assert.equal( - element.progress.get(1 as NumericChangeId), + element.progressByChange.get(1 as NumericChangeId), ProgressStatus.SUCCESSFUL ); }); @@ -294,7 +295,9 @@ queryAndAssert<GrButton>(query(element, 'gr-dialog'), '#confirm').click(); await waitUntil( - () => element.progress.get(2 as NumericChangeId) === ProgressStatus.FAILED + () => + element.progressByChange.get(2 as NumericChangeId) === + ProgressStatus.FAILED ); assert.isFalse(fireStub.called);
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 6fc8fd5..458fe9a 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
@@ -8,7 +8,7 @@ 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'; +import {AccountInfo, ChangeInfo, NumericChangeId} from '../../../types/common'; import {subscribe} from '../../lit/subscription-controller'; import '../../shared/gr-overlay/gr-overlay'; import '../../shared/gr-dialog/gr-dialog'; @@ -20,6 +20,7 @@ SUGGESTIONS_PROVIDERS_USERS_TYPES, } from '../../../scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider'; import '../../shared/gr-account-list/gr-account-list'; +import {getOverallStatus} from '../../../utils/bulk-flow-util'; const SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE: Record< ReviewerState, @@ -45,7 +46,10 @@ ReviewerSuggestionsProvider > = new Map(); - @state() private progressByChange = new Map<ChangeInfo, ProgressStatus>(); + @state() private progressByChangeNum = new Map< + NumericChangeId, + ProgressStatus + >(); @state() private isOverlayOpen = false; @@ -100,7 +104,7 @@ } private renderDialog() { - const overallStatus = this.getOverallStatus(); + const overallStatus = getOverallStatus(this.progressByChangeNum); return html` <gr-dialog @cancel=${() => this.closeOverlay()} @@ -159,8 +163,11 @@ } private resetFlow() { - this.progressByChange = new Map( - this.selectedChanges.map(change => [change, ProgressStatus.NOT_STARTED]) + this.progressByChangeNum = new Map( + this.selectedChanges.map(change => [ + change._number, + ProgressStatus.NOT_STARTED, + ]) ); for (const state of [ReviewerState.REVIEWER, ReviewerState.CC]) { this.updatedAccountsByReviewerState.set( @@ -180,17 +187,23 @@ private onConfirm(overallStatus: ProgressStatus) { switch (overallStatus) { case ProgressStatus.NOT_STARTED: - this.saveChanges(); + this.saveReviewers(); break; case ProgressStatus.SUCCESSFUL: this.overlay.close(); break; + case ProgressStatus.FAILED: + this.overlay.close(); + break; } } - private saveChanges() { - this.progressByChange = new Map( - this.selectedChanges.map(change => [change, ProgressStatus.RUNNING]) + private saveReviewers() { + this.progressByChangeNum = new Map( + this.selectedChanges.map(change => [ + change._number, + ProgressStatus.RUNNING, + ]) ); const inFlightActions = this.getBulkActionsModel().addReviewers( this.updatedAccountsByReviewerState @@ -199,11 +212,14 @@ const change = this.selectedChanges[index]; inFlightActions[index] .then(() => { - this.progressByChange.set(change, ProgressStatus.SUCCESSFUL); + this.progressByChangeNum.set( + change._number, + ProgressStatus.SUCCESSFUL + ); this.requestUpdate(); }) .catch(() => { - this.progressByChange.set(change, ProgressStatus.FAILED); + this.progressByChangeNum.set(change._number, ProgressStatus.FAILED); this.requestUpdate(); }); } @@ -248,17 +264,6 @@ suggestionsProvider.init(); return suggestionsProvider; } - - private getOverallStatus() { - const statuses = Array.from(this.progressByChange.values()); - if (statuses.every(s => s === ProgressStatus.NOT_STARTED)) { - return ProgressStatus.NOT_STARTED; - } - if (statuses.some(s => s === ProgressStatus.RUNNING)) { - return ProgressStatus.RUNNING; - } - return ProgressStatus.SUCCESSFUL; - } } declare global {
diff --git a/polygerrit-ui/app/utils/bulk-flow-util.ts b/polygerrit-ui/app/utils/bulk-flow-util.ts new file mode 100644 index 0000000..9a6179a --- /dev/null +++ b/polygerrit-ui/app/utils/bulk-flow-util.ts
@@ -0,0 +1,24 @@ +/** + * @license + * Copyright 2022 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {ProgressStatus} from '../constants/constants'; +import {NumericChangeId} from '../api/rest-api'; + +export function getOverallStatus( + progressByChangeNum: Map<NumericChangeId, ProgressStatus> +) { + const statuses = Array.from(progressByChangeNum.values()); + if (statuses.every(s => s === ProgressStatus.NOT_STARTED)) { + return ProgressStatus.NOT_STARTED; + } + if (statuses.some(s => s === ProgressStatus.RUNNING)) { + return ProgressStatus.RUNNING; + } + if (statuses.some(s => s === ProgressStatus.FAILED)) { + return ProgressStatus.FAILED; + } + return ProgressStatus.SUCCESSFUL; +}