Enable bulk abandon button if change is abandoned
In case user selects an already abandoned change, then let the button
remain enabled and resolve the promise as successful immediately.
Release-Notes: skip
Change-Id: I8434f0c265b6513683240df88dcb94be273fdb40
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
index e6e9e08..dc67c1b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
@@ -8,7 +8,7 @@
import {LitElement, html, css} from 'lit';
import {resolve} from '../../../models/dependency';
import {bulkActionsModelToken} from '../../../models/bulk-actions/bulk-actions-model';
-import {NumericChangeId, ChangeInfo} from '../../../api/rest-api';
+import {NumericChangeId, ChangeInfo, ChangeStatus} from '../../../api/rest-api';
import {subscribe} from '../../lit/subscription-controller';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {ProgressStatus} from '../../../constants/constants';
@@ -96,8 +96,7 @@
private isEnabled() {
return this.selectedChanges.every(
change =>
- // TODO: Add handling for when change is already abandoned
- !!change.actions?.abandon
+ !!change.actions?.abandon || change.status === ChangeStatus.ABANDONED
);
}
@@ -108,18 +107,20 @@
}
this.requestUpdate();
const errFn = (changeNum: NumericChangeId) => {
- this.progress.set(changeNum, ProgressStatus.FAILED);
- this.requestUpdate();
+ throw new Error(`request for ${changeNum} failed`);
};
const promises = this.getBulkActionsModel().abandonChanges('', errFn);
for (let index = 0; index < promises.length; index++) {
- promises[index].then(result => {
- const changeNum = this.selectedChanges[index]._number;
- if (result?.status !== 200)
+ const changeNum = this.selectedChanges[index]._number;
+ promises[index]
+ .then(() => {
+ this.progress.set(changeNum, ProgressStatus.SUCCESSFUL);
+ this.requestUpdate();
+ })
+ .catch(() => {
this.progress.set(changeNum, ProgressStatus.FAILED);
- else this.progress.set(changeNum, ProgressStatus.SUCCESSFUL);
- this.requestUpdate();
- });
+ this.requestUpdate();
+ });
}
}
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow_test.ts
index b678f6f..ad7281b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow_test.ts
@@ -5,7 +5,13 @@
*/
import {createChange} from '../../../test/test-data-generators';
-import {NumericChangeId, ChangeInfo} from '../../../api/rest-api';
+import {
+ NumericChangeId,
+ ChangeInfo,
+ ChangeStatus,
+ HttpMethod,
+ PatchSetNum,
+} from '../../../api/rest-api';
import {GrChangeListBulkAbandonFlow} from './gr-change-list-bulk-abandon-flow';
import '../../../test/common-test-setup-karma';
import {
@@ -28,6 +34,8 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {tap} from '@polymer/iron-test-helpers/mock-interactions';
import {ProgressStatus} from '../../../constants/constants';
+import {RequestPayload} from '../../../types/common';
+import {ErrorCallback} from '../../../api/rest';
const change1: ChangeInfo = {...createChange(), _number: 1 as NumericChangeId};
const change2: ChangeInfo = {...createChange(), _number: 2 as NumericChangeId};
@@ -89,6 +97,32 @@
assert.isTrue(queryAndAssert<GrButton>(element, '#abandon').disabled);
});
+ test('abandon button is enabled if change is already abandoned', async () => {
+ const changes: ChangeInfo[] = [
+ {...change1, actions: {}, status: ChangeStatus.ABANDONED},
+ ];
+ getChangesStub.returns(changes);
+ model.sync(changes);
+ await waitUntilObserved(
+ model.loadingState$,
+ state => state === LoadingState.LOADED
+ );
+ await selectChange(change1);
+ await element.updateComplete;
+ await flush();
+ assert.isFalse(queryAndAssert<GrButton>(element, '#abandon').disabled);
+
+ tap(queryAndAssert(query(element, 'gr-dialog'), '#confirm'));
+
+ await waitUntil(
+ () =>
+ queryAndAssert<HTMLTableDataCellElement>(
+ element,
+ '#status'
+ ).innerText.trim() === `Status: ${ProgressStatus.SUCCESSFUL}`
+ );
+ });
+
test('progress updates as request is resolved', async () => {
const changes: ChangeInfo[] = [{...change1, actions: {abandon: {}}}];
getChangesStub.returns(changes);
@@ -156,8 +190,20 @@
`Status: ${ProgressStatus.NOT_STARTED}`
);
- const executeChangeAction = mockPromise<Response>();
- stubRestApi('executeChangeAction').returns(executeChangeAction);
+ stubRestApi('executeChangeAction').callsFake(
+ (
+ _changeNum: NumericChangeId,
+ _method: HttpMethod | undefined,
+ _endpoint: string,
+ _patchNum?: PatchSetNum,
+ _payload?: RequestPayload,
+ errFn?: ErrorCallback
+ ) =>
+ Promise.resolve(new Response()).then(res => {
+ errFn && errFn();
+ return res;
+ })
+ );
tap(queryAndAssert(query(element, 'gr-dialog'), '#confirm'));
await element.updateComplete;
@@ -170,7 +216,6 @@
`Status: ${ProgressStatus.RUNNING}`
);
- executeChangeAction.resolve({...new Response(), status: 500});
await waitUntil(
() => element.progress.get(1 as NumericChangeId) === ProgressStatus.FAILED
);
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
index d483803..5ab1617 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
-import {ChangeInfo, NumericChangeId} from '../../api/rest-api';
+import {ChangeInfo, NumericChangeId, ChangeStatus} from '../../api/rest-api';
import {Model} from '../model';
import {Finalizable} from '../../services/registry';
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
@@ -108,6 +108,9 @@
if (!current.allChanges.get(changeNum))
throw new Error('invalid change id');
const change = current.allChanges.get(changeNum)!;
+ if (change.status === ChangeStatus.ABANDONED) {
+ return Promise.resolve(new Response());
+ }
return this.restApiService.executeChangeAction(
change._number,
change.actions!.abandon!.method,
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
index acf0462..a2c4bac 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
@@ -5,12 +5,19 @@
*/
import {createChange} from '../../test/test-data-generators';
-import {ChangeInfo, NumericChangeId} from '../../api/rest-api';
+import {
+ ChangeInfo,
+ NumericChangeId,
+ ChangeStatus,
+ HttpMethod,
+} from '../../api/rest-api';
import {BulkActionsModel, LoadingState} from './bulk-actions-model';
import {getAppContext} from '../../services/app-context';
import '../../test/common-test-setup-karma';
import {stubRestApi, waitUntilObserved} from '../../test/test-utils';
import {mockPromise} from '../../test/test-utils';
+import {SinonStubbedMember} from 'sinon';
+import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
suite('bulk actions model test', () => {
let bulkActionsModel: BulkActionsModel;
@@ -102,6 +109,44 @@
assert.equal(totalChangeCount, 2);
});
+ suite('abandon changes', () => {
+ let detailedActionsStub: SinonStubbedMember<
+ RestApiService['getDetailedChangesWithActions']
+ >;
+ setup(async () => {
+ detailedActionsStub = stubRestApi('getDetailedChangesWithActions');
+ const c1 = createChange();
+ c1._number = 1 as NumericChangeId;
+ const c2 = createChange();
+ c2._number = 2 as NumericChangeId;
+
+ detailedActionsStub.returns(
+ Promise.resolve([
+ {...c1, actions: {abandon: {method: HttpMethod.POST}}},
+ {...c2, status: ChangeStatus.ABANDONED},
+ ])
+ );
+
+ bulkActionsModel.sync([c1, c2]);
+
+ bulkActionsModel.addSelectedChangeNum(c1._number);
+ bulkActionsModel.addSelectedChangeNum(c2._number);
+ });
+
+ test('already abandoned change does not call executeChangeAction', () => {
+ const actionStub = stubRestApi('executeChangeAction');
+ bulkActionsModel.abandonChanges();
+ assert.equal(actionStub.callCount, 1);
+ assert.deepEqual(actionStub.lastCall.args.slice(0, 5), [
+ 1 as NumericChangeId,
+ HttpMethod.POST,
+ '/abandon',
+ undefined,
+ {message: ''},
+ ]);
+ });
+ });
+
test('stale changes are removed from the model', async () => {
const c1 = createChange();
c1._number = 1 as NumericChangeId;
