Remove the `handleEvent` indirection in js-api-interface
I just stumbled over that and wanted to make it simpler and more
consistent will other js-api events. This also helps with making
the calls type correct any removing the TODOs about `any` type.
Release-Notes: skip
Change-Id: I8c33a7091b9a3010be09bb506385b0d81284b64c
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 834f2b0..ca58dd5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -33,7 +33,7 @@
HttpMethod,
NotifyType,
} from '../../../constants/constants';
-import {EventType as PluginEventType, TargetElement} from '../../../api/plugin';
+import {TargetElement} from '../../../api/plugin';
import {
AccountInfo,
ActionInfo,
@@ -108,6 +108,7 @@
import {createSearchUrl} from '../../../models/views/search';
import {createChangeUrl} from '../../../models/views/change';
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
+import {ShowRevisionActionsDetail} from '../../shared/gr-js-api-interface/gr-js-api-types';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -890,11 +891,8 @@
}
// private but used in test
- sendShowRevisionActions(detail: {
- change: ChangeInfo;
- revisionActions: ActionNameToActionInfoMap;
- }) {
- this.jsAPI.handleEvent(PluginEventType.SHOW_REVISION_ACTIONS, detail);
+ sendShowRevisionActions(detail: ShowRevisionActionsDetail) {
+ this.jsAPI.handleShowRevisionActions(detail);
}
addActionButton(type: ActionType, label: string) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 6a7aeeb..5fff2fd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -65,7 +65,6 @@
isInvolved,
roleDetails,
} from '../../../utils/change-util';
-import {EventType as PluginEventType} from '../../../api/plugin';
import {customElement, property, query, state} from 'lit/decorators.js';
import {GrApplyFixDialog} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
import {GrFileListHeader} from '../gr-file-list-header/gr-file-list-header';
@@ -2258,7 +2257,7 @@
// Private but used in tests.
sendShowChangeEvent() {
assertIsDefined(this.patchRange, 'patchRange');
- this.jsAPI.handleEvent(PluginEventType.SHOW_CHANGE, {
+ this.jsAPI.handleShowChange({
change: this.change,
patchNum: this.patchRange.patchNum,
info: {mergeable: this.mergeable},
@@ -2644,7 +2643,7 @@
return;
}
this.handleLabelRemoved(oldLabels, newLabels);
- this.jsAPI.handleEvent(PluginEventType.LABEL_CHANGE, {
+ this.jsAPI.handleLabelChange({
change: this.change,
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 2491610..ccba81c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -21,7 +21,7 @@
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
-import {EventType, PluginApi} from '../../../api/plugin';
+import {PluginApi} from '../../../api/plugin';
import {
mockPromise,
pressKey,
@@ -76,6 +76,7 @@
DetailedLabelInfo,
RepoName,
QuickLabelInfo,
+ PatchSetNumber,
} from '../../../types/common';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
@@ -2229,13 +2230,12 @@
element.change = {...change};
element.patchRange = {patchNum: 4 as RevisionPatchSetNum};
element.mergeable = true;
- const showStub = sinon.stub(element.jsAPI, 'handleEvent');
+ const showStub = sinon.stub(element.jsAPI, 'handleShowChange');
element.sendShowChangeEvent();
assert.isTrue(showStub.calledOnce);
- assert.equal(showStub.lastCall.args[0], EventType.SHOW_CHANGE);
- assert.deepEqual(showStub.lastCall.args[1], {
+ assert.deepEqual(showStub.lastCall.args[0], {
change,
- patchNum: 4,
+ patchNum: 4 as PatchSetNumber,
info: {mergeable: true},
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
index 6142aa2..2edbf69 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
@@ -33,31 +33,6 @@
finalize() {}
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- handleEvent(type: EventType, detail: any) {
- getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- switch (type) {
- case EventType.SHOW_CHANGE:
- this._handleShowChange(detail);
- break;
- case EventType.LABEL_CHANGE:
- this._handleLabelChange(detail);
- break;
- case EventType.SHOW_REVISION_ACTIONS:
- this._handleShowRevisionActions(detail);
- break;
- default:
- console.warn(
- 'handleEvent called with unsupported event type:',
- type
- );
- break;
- }
- });
- }
-
addElement(key: TargetElement, el: HTMLElement) {
elements[key] = el;
}
@@ -98,7 +73,8 @@
}
}
- _handleShowChange(detail: ShowChangeDetail) {
+ async handleShowChange(detail: ShowChangeDetail) {
+ await getPluginLoader().awaitPluginsLoaded();
// Note (issue 8221) Shallow clone the change object and add a mergeable
// getter with deprecation warning. This makes the change detail appear as
// though SKIP_MERGEABLE was not set, so that plugins that expect it can
@@ -143,7 +119,8 @@
}
}
- _handleShowRevisionActions(detail: ShowRevisionActionsDetail) {
+ async handleShowRevisionActions(detail: ShowRevisionActionsDetail) {
+ await getPluginLoader().awaitPluginsLoaded();
const registeredCallbacks = this._getEventCallbacks(
EventType.SHOW_REVISION_ACTIONS
);
@@ -174,7 +151,8 @@
}
}
- _handleLabelChange(detail: {change: ChangeInfo}) {
+ async handleLabelChange(detail: {change?: ParsedChangeInfo}) {
+ await getPluginLoader().awaitPluginsLoaded();
for (const cb of this._getEventCallbacks(EventType.LABEL_CHANGE)) {
try {
cb(detail.change);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
index 7c09bad..aa91b1d 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
@@ -81,8 +81,9 @@
plugin.on(EventType.SHOW_CHANGE, (change, revision, info) => {
resolve({change, revision, info});
});
- element.handleEvent(EventType.SHOW_CHANGE,
- {change: testChange, patchNum: 1, info: {mergeable: false}});
+ element.handleShowChange(
+ {change: testChange, patchNum: 1, info: {mergeable: false}}
+ );
const {change, revision, info} = await promise;
assert.deepEqual(change, expectedChange);
@@ -102,8 +103,9 @@
plugin.on(EventType.SHOW_REVISION_ACTIONS, (actions, change) => {
resolve({change, actions});
});
- element.handleEvent(EventType.SHOW_REVISION_ACTIONS,
- {change: testChange, revisionActions: {test: {}}});
+ element.handleShowRevisionActions(
+ {change: testChange, revisionActions: {test: {}}}
+ );
const {change, actions} = await promise;
assert.deepEqual(change, testChange);
@@ -111,7 +113,7 @@
assert.isTrue(errorStub.calledOnce);
});
- test('handleEvent awaits plugins load', async () => {
+ test('handleShowChange awaits plugins load', async () => {
const testChange = {
_number: 42,
revisions: {def: {_number: 2}, abc: {_number: 1}},
@@ -119,8 +121,7 @@
const spy = sinon.spy();
getPluginLoader().loadPlugins(['plugins/test.js']);
plugin.on(EventType.SHOW_CHANGE, spy);
- element.handleEvent(EventType.SHOW_CHANGE,
- {change: testChange, patchNum: 1});
+ element.handleShowChange({change: testChange, patchNum: 1});
assert.isFalse(spy.called);
// Timeout on loading plugins
@@ -201,7 +202,7 @@
const testChange = {_number: 42};
plugin.on(EventType.LABEL_CHANGE, throwErrFn);
plugin.on(EventType.LABEL_CHANGE, resolve);
- element.handleEvent(EventType.LABEL_CHANGE, {change: testChange});
+ element.handleLabelChange({change: testChange});
const change = await promise;
assert.deepEqual(change, testChange);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
index 7aad2f0..c7a5eae 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
@@ -17,14 +17,14 @@
import {MenuLink} from '../../../api/admin';
export interface ShowChangeDetail {
- change: ChangeInfo;
- patchNum: PatchSetNum;
- info: {mergeable: boolean};
+ change?: ParsedChangeInfo;
+ patchNum?: PatchSetNum;
+ info: {mergeable: boolean | null};
}
export interface ShowRevisionActionsDetail {
change: ChangeInfo;
- revisionActions: {[key: string]: ActionInfo};
+ revisionActions: {[key: string]: ActionInfo | undefined};
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -38,8 +38,9 @@
revertSubmissionMsg: string,
origMsg: string
): string;
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- handleEvent(eventName: EventType, detail: any): void;
+ handleShowChange(detail: ShowChangeDetail): void;
+ handleShowRevisionActions(detail: ShowRevisionActionsDetail): void;
+ handleLabelChange(detail: {change?: ParsedChangeInfo}): void;
modifyRevertMsg(
change: ChangeInfo,
revertMsg: string,
