refactor(core,schemas): refactor DataHook code

refactor DataHook code to address some code review comments
logto-io · May 9, 2024 · c965e17 · c965e17
1 parent 85aa000
commit c965e17
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 34 deletions.
diff --git a/packages/core/src/middleware/koa-management-api-hooks.ts b/packages/core/src/middleware/koa-management-api-hooks.ts
@@ -49,7 +49,14 @@ export const koaManagementApiHooks = <StateT, ContextT extends IRouterParamConte
     await next();
 
     // Auto append pre-registered management API hooks if any
-    const { path, method, status, _matchedRoute: matchedRoute, params, response } = ctx;
+    const {
+      path,
+      method,
+      status,
+      _matchedRoute: matchedRoute,
+      params,
+      response: { body },
+    } = ctx;
 
     const hookRegistrationKey = buildManagementApiDataHookRegistrationKey(method, matchedRoute);
 
@@ -58,7 +65,7 @@ export const koaManagementApiHooks = <StateT, ContextT extends IRouterParamConte
 
       dataHooks.appendContext({
         event,
-        data: { path, method, body: response.body, status, params, matchedRoute },
+        data: { path, method, response: { body }, status, params, matchedRoute },
       });
     }
 

diff --git a/packages/integration-tests/src/tests/api/hook/WebhookMockServer.ts b/packages/integration-tests/src/tests/api/hook/WebhookMockServer.ts
@@ -6,16 +6,15 @@ import { createServer, type RequestListener, type Server } from 'node:http';
  * @example
  * const server = new WebhookMockServer(3000);
  * await server.listen();
- *
- * @param port The port to listen on.
- * @param requestCallback A callback that is called with the request body.
  */
 class WebhookMockServer {
   public readonly endpoint = `http://localhost:${this.port}`;
   private readonly server: Server;
 
   constructor(
+    /** The port number to listen on. */
     private readonly port: number,
+    /** A callback that is called with the request body when a request is received. */
     requestCallback?: (body: string) => void
   ) {
     const requestListener: RequestListener = (request, response) => {

diff --git a/packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts b/packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts
@@ -337,22 +337,6 @@ describe('data hook events coverage', () => {
   it.each(keys)('should have test case for %s', async (key) => {
     const webhookResult = await getWebhookResult(key);
     expect(webhookResult).toBeDefined();
+    expect(webhookResult?.signature).toBeDefined();
   });
 });
-
-type SupportedKey = 'a' | 'b';
-
-const map: { [K in SupportedKey]: K } = {
-  a: 'a',
-  b: 'b',
-};
-
-const keyIsKeyOfMap = (key: string): key is SupportedKey => key in map;
-
-const getKeys = (key: string): SupportedKey => {
-  if (keyIsKeyOfMap(key)) {
-    return map[key];
-  }
-
-  return 'a';
-};
diff --git a/packages/integration-tests/src/tests/api/hook/test-cases.ts b/packages/integration-tests/src/tests/api/hook/test-cases.ts
@@ -39,7 +39,7 @@ export const userDataHookTestCases: TestCase[] = [
   },
   {
     route: 'PATCH /users/:userId/is-suspended',
-    event: 'User.SuspendStatus.Updated',
+    event: 'User.SuspensionStatus.Updated',
     method: 'patch',
     endpoint: `users/{userId}/is-suspended`,
     payload: { isSuspended: true },

diff --git a/packages/schemas/src/foundations/jsonb-types/hooks.ts b/packages/schemas/src/foundations/jsonb-types/hooks.ts
@@ -32,15 +32,15 @@ enum DataHookBasicMutationType {
 
 type BasicDataHookEvent = `${DataHookSchema}.${DataHookBasicMutationType}`;
 
-// Mutable property schemas
-type DataHookPropertyMutableSchema =
-  | `${DataHookSchema.User}.SuspendStatus`
+// Custom DataHook mutable schemas
+type CustomDataHookMutableSchema =
+  | `${DataHookSchema.User}.SuspensionStatus`
   | `${DataHookSchema.Role}.Scopes`
   | `${DataHookSchema.Organization}.Membership`
   | `${DataHookSchema.OrganizationRole}.Scopes`;
 
 type DataHookPropertyUpdateEvent =
-  `${DataHookPropertyMutableSchema}.${DataHookBasicMutationType.Updated}`;
+  `${CustomDataHookMutableSchema}.${DataHookBasicMutationType.Updated}`;
 
 export type DataHookEvent = BasicDataHookEvent | DataHookPropertyUpdateEvent;
 
@@ -52,7 +52,7 @@ export const hookEvents = Object.freeze([
   'User.Created',
   'User.Deleted',
   'User.Updated',
-  'User.SuspendStatus.Updated',
+  'User.SuspensionStatus.Updated',
   'Role.Created',
   'Role.Deleted',
   'Role.Updated',
@@ -116,7 +116,7 @@ export const managementApiHooksRegistration = Object.freeze({
   'PATCH /users/:userId/custom-data': 'User.Updated',
   'PATCH /users/:userId/profile': 'User.Updated',
   'PATCH /users/:userId/password': 'User.Updated',
-  'PATCH /users/:userId/is-suspended': 'User.SuspendStatus.Updated',
+  'PATCH /users/:userId/is-suspended': 'User.SuspensionStatus.Updated',
   'POST /roles': 'Role.Created',
   'DELETE /roles/:id': 'Role.Deleted',
   'PATCH /roles/:id': 'Role.Updated',

diff --git a/packages/schemas/src/types/hook.ts b/packages/schemas/src/types/hook.ts
@@ -19,23 +19,24 @@ export type InteractionHookEventPayload = {
 
 /**
  * The payload of a data hook event.
- *
- * @param body: response body
- * @param params: route params
- * @param path: route path
- * @param matchedRoute: matched route used as the identifier to trigger the hook
  */
 export type DataHookEventPayload = {
   event: DataHookEvent;
   createdAt: string;
   hookId: string;
   ip?: string;
   userAgent?: string;
+  /** Response body */
   body?: Record<string, unknown>;
+  /** Route params */
   params?: Record<string, string>;
+  /** Route path */
   path?: string;
+  /** Matched route used as the identifier to trigger the hook */
   matchedRoute?: string;
+  /** Response status code */
   status?: number;
+  /** Request method */
   method?: string;
 } & Record<string, unknown>;