Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add accepted and rejected events to ApprovalController #5127

Closed
60 changes: 57 additions & 3 deletions packages/approval-controller/src/ApprovalController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
AddApprovalOptions,
ApprovalControllerActions,
ApprovalControllerEvents,
ApprovalControllerMessenger,
ErrorOptions,
StartFlowOptions,
SuccessOptions,
Expand Down Expand Up @@ -243,15 +244,17 @@ function getRestrictedMessenger() {
describe('approval controller', () => {
let approvalController: ApprovalController;
let showApprovalRequest: jest.Mock;
let controllerMessenger: ApprovalControllerMessenger;

beforeEach(() => {
nanoidMock.mockReturnValue('TestId');
jest.spyOn(global.console, 'info').mockImplementation(() => undefined);

controllerMessenger = getRestrictedMessenger();
showApprovalRequest = jest.fn();

approvalController = new ApprovalController({
messenger: getRestrictedMessenger(),
messenger: controllerMessenger,
showApprovalRequest,
});
});
Expand Down Expand Up @@ -828,6 +831,32 @@ describe('approval controller', () => {
expect(result).toBe('success');
});

it('emits "accepted" event', async () => {
const acceptedEvent = jest.fn();
controllerMessenger.subscribe('ApprovalController:accepted', acceptedEvent);

const approvalPromise = approvalController.add({
id: 'foo',
origin: 'bar.baz',
type: 'myType',
});

// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
approvalController.accept('foo', 'success');

await approvalPromise;

expect(acceptedEvent.mock.calls[0][0]).toEqual({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For maintainability and modularity, should we have distinct tests to verify the events are fired?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

request: expect.objectContaining({
id: 'foo',
origin: 'bar.baz',
type: 'myType',
time: expect.any(Number),
}),
});
});

it('resolves multiple approval promises out of order', async () => {
const approvalPromise1 = approvalController.add({
id: 'foo1',
Expand Down Expand Up @@ -1067,13 +1096,38 @@ describe('approval controller', () => {

describe('reject', () => {
it('rejects approval promise', async () => {
const rejectedError = new Error('failure');
const approvalPromise = approvalController.add({
id: 'foo',
origin: 'bar.baz',
type: TYPE,
});
approvalController.reject('foo', new Error('failure'));
await expect(approvalPromise).rejects.toThrow('failure');
approvalController.reject('foo', rejectedError);
await expect(approvalPromise).rejects.toThrow(rejectedError);
});

it('emits "rejected" event', async () => {
const rejectedEvent = jest.fn();
controllerMessenger.subscribe('ApprovalController:rejected', rejectedEvent);

const rejectedError = new Error('failure');
const approvalPromise = approvalController.add({
id: 'foo',
origin: 'bar.baz',
type: TYPE,
});
approvalController.reject('foo', rejectedError);

await expect(approvalPromise).rejects.toThrow(rejectedError);

expect(rejectedEvent.mock.calls[0][0]).toEqual({
request: expect.objectContaining({
id: 'foo',
origin: 'bar.baz',
type: TYPE,
}),
error: rejectedError,
});
});

it('rejects multiple approval promises out of order', async () => {
Expand Down
34 changes: 33 additions & 1 deletion packages/approval-controller/src/ApprovalController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ export type ApprovalStateChange = ControllerStateChangeEvent<
ApprovalControllerState
>;

export type ApprovalControllerEvents = ApprovalStateChange;
export type ApprovalControllerEvents =
| ApprovalAcceptedEvent
| ApprovalRejectedEvent
| ApprovalStateChange;

// Action Types

Expand Down Expand Up @@ -325,6 +328,25 @@ export type ShowError = {
handler: ApprovalController['error'];
};

export type ApprovalRejectedEvent = {
type: `${typeof controllerName}:rejected`;
payload: [
{
request: ApprovalRequest<ApprovalRequestData>;
error: Error;
},
];
};

export type ApprovalAcceptedEvent = {
type: `${typeof controllerName}:accepted`;
payload: [
{
request: ApprovalRequest<ApprovalRequestData>;
},
];
};

export type ApprovalControllerActions =
| GetApprovalsState
| ClearApprovalRequests
Expand Down Expand Up @@ -729,6 +751,9 @@ export class ApprovalController extends BaseController<
if (!requestDeleted) {
this.#delete(id);
}
this.messagingSystem.publish(`${controllerName}:accepted`, {
request: approval,
});
});
}

Expand All @@ -740,9 +765,16 @@ export class ApprovalController extends BaseController<
* @param error - The error to reject the approval promise with.
*/
reject(id: string, error: unknown): void {
const rejectedApproval = this.get(
id,
) as ApprovalRequest<ApprovalRequestData>;
const callbacks = this.#getCallbacks(id);
this.#delete(id);
callbacks.reject(error);
this.messagingSystem.publish(`${controllerName}:rejected`, {
request: rejectedApproval,
error: error as Error,
});
}

/**
Expand Down
Loading