From 5e7bee1c8cc7c02ea5c97fbb4059598118de2b10 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Thu, 16 May 2024 14:40:59 +0800 Subject: [PATCH] refactor(core,schemas,test): rename DataHook data update event name (#5876) rename the DataHook Schema data update event name --- .../actions/submit-interaction.test.ts | 6 +- .../interaction/actions/submit-interaction.ts | 24 +++- .../api/hook/hook.trigger.interaction.test.ts | 12 +- .../src/tests/api/hook/test-cases.ts | 18 +-- .../src/tests/console/webhooks/helpers.ts | 2 +- ...31-rename-data-hook-schema-update-event.ts | 120 ++++++++++++++++++ .../src/foundations/jsonb-types/hooks.ts | 36 +++--- 7 files changed, 177 insertions(+), 41 deletions(-) create mode 100644 packages/schemas/alterations/next-1715829731-rename-data-hook-schema-update-event.ts diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts index c34d4a0bc..4c49dc5da 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts @@ -372,7 +372,7 @@ describe('submit action', () => { login: { accountId: 'foo' }, }); expect(ctx.assignDataHookContext).toBeCalledWith({ - event: 'User.Updated', + event: 'User.Data.Updated', user: updateProfile, }); }); @@ -433,7 +433,7 @@ describe('submit action', () => { login: { accountId: 'foo' }, }); expect(ctx.assignDataHookContext).toBeCalledWith({ - event: 'User.Updated', + event: 'User.Data.Updated', user: { primaryEmail: 'email', name: userInfo.name, @@ -459,7 +459,7 @@ describe('submit action', () => { }); expect(assignInteractionResults).not.toBeCalled(); expect(ctx.assignDataHookContext).toBeCalledWith({ - event: 'User.Updated', + event: 'User.Data.Updated', user: { passwordEncrypted: 'passwordEncrypted', passwordEncryptionMethod: 'plain', diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.ts b/packages/core/src/routes/interaction/actions/submit-interaction.ts index c3670cc66..567299c02 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.ts @@ -127,7 +127,9 @@ async function handleSubmitRegister( // If it's Logto Cloud, Check if the new user has any pending invitations, if yes, skip onboarding flow. const invitations = isCloud && userProfile.primaryEmail - ? await organizations.invitations.findEntities({ invitee: userProfile.primaryEmail }) + ? await organizations.invitations.findEntities({ + invitee: userProfile.primaryEmail, + }) : []; const hasPendingInvitations = invitations.some( (invitation) => invitation.status === OrganizationInvitationStatus.Pending @@ -189,7 +191,9 @@ async function handleSubmitRegister( ctx.assignDataHookContext({ event: 'User.Created', user }); log?.append({ userId: id }); - appInsights.client?.trackEvent({ name: getEventName(Component.Core, CoreEvent.Register) }); + appInsights.client?.trackEvent({ + name: getEventName(Component.Core, CoreEvent.Register), + }); void trySafe(postAffiliateLogs(ctx, cloudConnection, id, tenantId), (error) => { getConsoleLogFromContext(ctx).warn('Failed to post affiliate logs', error); @@ -238,10 +242,15 @@ async function handleSubmitSignIn( ctx.assignInteractionHookResult({ userId: accountId }); // Trigger user.updated data hook event if the user profile or mfa data is updated if (hasUpdatedProfile(updateUserProfile) || mfaVerifications.length > 0) { - ctx.assignDataHookContext({ event: 'User.Updated', user: updatedUser }); + ctx.assignDataHookContext({ + event: 'User.Data.Updated', + user: updatedUser, + }); } - appInsights.client?.trackEvent({ name: getEventName(Component.Core, CoreEvent.SignIn) }); + appInsights.client?.trackEvent({ + name: getEventName(Component.Core, CoreEvent.SignIn), + }); } export default async function submitInteraction( @@ -270,9 +279,12 @@ export default async function submitInteraction( profile.password ); - const user = await updateUserById(accountId, { passwordEncrypted, passwordEncryptionMethod }); + const user = await updateUserById(accountId, { + passwordEncrypted, + passwordEncryptionMethod, + }); ctx.assignInteractionHookResult({ userId: accountId }); - ctx.assignDataHookContext({ event: 'User.Updated', user }); + ctx.assignDataHookContext({ event: 'User.Data.Updated', user }); await clearInteractionStorage(ctx, provider); ctx.status = 204; diff --git a/packages/integration-tests/src/tests/api/hook/hook.trigger.interaction.test.ts b/packages/integration-tests/src/tests/api/hook/hook.trigger.interaction.test.ts index 7fe528e69..d5b47148d 100644 --- a/packages/integration-tests/src/tests/api/hook/hook.trigger.interaction.test.ts +++ b/packages/integration-tests/src/tests/api/hook/hook.trigger.interaction.test.ts @@ -190,7 +190,7 @@ describe('interaction api trigger hooks', () => { }); // Assert user updated event is not triggered - await assertHookLogResult(dataHook, 'User.Updated', { + await assertHookLogResult(dataHook, 'User.Data.Updated', { toBeUndefined: true, }); @@ -221,7 +221,7 @@ describe('interaction api trigger hooks', () => { toBeUndefined: true, }); - await assertHookLogResult(dataHook, 'User.Updated', { + await assertHookLogResult(dataHook, 'User.Data.Updated', { toBeUndefined: true, }); }); @@ -257,9 +257,9 @@ describe('interaction api trigger hooks', () => { toBeUndefined: true, }); - await assertHookLogResult(dataHook, 'User.Updated', { + await assertHookLogResult(dataHook, 'User.Data.Updated', { hookPayload: { - event: 'User.Updated', + event: 'User.Data.Updated', interactionEvent: InteractionEvent.SignIn, sessionId: expect.any(String), data: expect.objectContaining({ id: user.id, username, primaryEmail: email }), @@ -286,9 +286,9 @@ describe('interaction api trigger hooks', () => { hookPayload: interactionHookEventPayload, }); - await assertHookLogResult(dataHook, 'User.Updated', { + await assertHookLogResult(dataHook, 'User.Data.Updated', { hookPayload: { - event: 'User.Updated', + event: 'User.Data.Updated', interactionEvent: InteractionEvent.ForgotPassword, sessionId: expect.any(String), data: expect.objectContaining({ id: user.id, username, primaryEmail: email }), diff --git a/packages/integration-tests/src/tests/api/hook/test-cases.ts b/packages/integration-tests/src/tests/api/hook/test-cases.ts index 4acdc1377..4d142eb3b 100644 --- a/packages/integration-tests/src/tests/api/hook/test-cases.ts +++ b/packages/integration-tests/src/tests/api/hook/test-cases.ts @@ -11,28 +11,28 @@ type TestCase = { export const userDataHookTestCases: TestCase[] = [ { route: 'PATCH /users/:userId', - event: 'User.Updated', + event: 'User.Data.Updated', method: 'patch', endpoint: `users/{userId}`, payload: { name: 'new name' }, }, { route: 'PATCH /users/:userId/custom-data', - event: 'User.Updated', + event: 'User.Data.Updated', method: 'patch', endpoint: `users/{userId}/custom-data`, payload: { customData: { foo: 'bar' } }, }, { route: 'PATCH /users/:userId/profile', - event: 'User.Updated', + event: 'User.Data.Updated', method: 'patch', endpoint: `users/{userId}/profile`, payload: { profile: { nickname: 'darcy' } }, }, { route: 'PATCH /users/:userId/password', - event: 'User.Updated', + event: 'User.Data.Updated', method: 'patch', endpoint: `users/{userId}/password`, payload: { password: 'new-password' }, @@ -56,7 +56,7 @@ export const userDataHookTestCases: TestCase[] = [ export const roleDataHookTestCases: TestCase[] = [ { route: 'PATCH /roles/:id', - event: 'Role.Updated', + event: 'Role.Data.Updated', method: 'patch', endpoint: `roles/{roleId}`, payload: { name: 'new name' }, @@ -87,7 +87,7 @@ export const roleDataHookTestCases: TestCase[] = [ export const scopesDataHookTestCases: TestCase[] = [ { route: 'PATCH /resources/:resourceId/scopes/:scopeId', - event: 'Scope.Updated', + event: 'Scope.Data.Updated', method: 'patch', endpoint: `resources/{resourceId}/scopes/{scopeId}`, payload: { name: generateName() }, @@ -104,7 +104,7 @@ export const scopesDataHookTestCases: TestCase[] = [ export const organizationDataHookTestCases: TestCase[] = [ { route: 'PATCH /organizations/:id', - event: 'Organization.Updated', + event: 'Organization.Data.Updated', method: 'patch', endpoint: `organizations/{organizationId}`, payload: { description: 'new org description' }, @@ -142,7 +142,7 @@ export const organizationDataHookTestCases: TestCase[] = [ export const organizationScopeDataHookTestCases: TestCase[] = [ { route: 'PATCH /organization-scopes/:id', - event: 'OrganizationScope.Updated', + event: 'OrganizationScope.Data.Updated', method: 'patch', endpoint: `organization-scopes/{organizationScopeId}`, payload: { description: 'new org scope description' }, @@ -159,7 +159,7 @@ export const organizationScopeDataHookTestCases: TestCase[] = [ export const organizationRoleDataHookTestCases: TestCase[] = [ { route: 'PATCH /organization-roles/:id', - event: 'OrganizationRole.Updated', + event: 'OrganizationRole.Data.Updated', method: 'patch', endpoint: `organization-roles/{organizationRoleId}`, payload: { name: generateName() }, diff --git a/packages/integration-tests/src/tests/console/webhooks/helpers.ts b/packages/integration-tests/src/tests/console/webhooks/helpers.ts index bfdfb03bd..13fb22e54 100644 --- a/packages/integration-tests/src/tests/console/webhooks/helpers.ts +++ b/packages/integration-tests/src/tests/console/webhooks/helpers.ts @@ -3,7 +3,7 @@ import { type Page } from 'puppeteer'; export const expectToCreateWebhook = async (page: Page) => { await expect(page).toClick('div[class$=main] div[class$=headline] > button'); await expect(page).toClick('span[class$=label]', { text: 'PostRegister' }); - await expect(page).toClick('span[class$=label]', { text: 'User.Updated' }); + await expect(page).toClick('span[class$=label]', { text: 'User.Data.Updated' }); await expect(page).toFill('input[name=name]', 'hook_name'); await expect(page).toFill('input[name=url]', 'https://localhost/webhook'); await expect(page).toClick('button[type=submit]'); diff --git a/packages/schemas/alterations/next-1715829731-rename-data-hook-schema-update-event.ts b/packages/schemas/alterations/next-1715829731-rename-data-hook-schema-update-event.ts new file mode 100644 index 000000000..83ee4224f --- /dev/null +++ b/packages/schemas/alterations/next-1715829731-rename-data-hook-schema-update-event.ts @@ -0,0 +1,120 @@ +import { sql } from '@silverhand/slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +enum DataHookSchema { + User = 'User', + Role = 'Role', + Scope = 'Scope', + Organization = 'Organization', + OrganizationRole = 'OrganizationRole', + OrganizationScope = 'OrganizationScope', +} + +type OldSchemaUpdateEvent = `${DataHookSchema}.${'Updated'}`; +type NewSchemaUpdateEvent = `${DataHookSchema}.Data.Updated`; + +const oldSchemaUpdateEvents = Object.freeze([ + 'User.Updated', + 'Role.Updated', + 'Scope.Updated', + 'Organization.Updated', + 'OrganizationRole.Updated', + 'OrganizationScope.Updated', +] satisfies OldSchemaUpdateEvent[]); + +const newSchemaUpdateEvents = Object.freeze([ + 'User.Data.Updated', + 'Role.Data.Updated', + 'Scope.Data.Updated', + 'Organization.Data.Updated', + 'OrganizationRole.Data.Updated', + 'OrganizationScope.Data.Updated', +] as const satisfies NewSchemaUpdateEvent[]); + +const updateMap: { [key in OldSchemaUpdateEvent]: NewSchemaUpdateEvent } = { + 'User.Updated': 'User.Data.Updated', + 'Role.Updated': 'Role.Data.Updated', + 'Scope.Updated': 'Scope.Data.Updated', + 'Organization.Updated': 'Organization.Data.Updated', + 'OrganizationRole.Updated': 'OrganizationRole.Data.Updated', + 'OrganizationScope.Updated': 'OrganizationScope.Data.Updated', +}; + +const reverseMap: { [key in NewSchemaUpdateEvent]: OldSchemaUpdateEvent } = { + 'User.Data.Updated': 'User.Updated', + 'Role.Data.Updated': 'Role.Updated', + 'Scope.Data.Updated': 'Scope.Updated', + 'Organization.Data.Updated': 'Organization.Updated', + 'OrganizationRole.Data.Updated': 'OrganizationRole.Updated', + 'OrganizationScope.Data.Updated': 'OrganizationScope.Updated', +}; + +// This alteration script filters all the hook's events jsonb column to replace all the old schema update events with the new schema update events. + +const isOldSchemaUpdateEvent = (event: string): event is OldSchemaUpdateEvent => + // eslint-disable-next-line no-restricted-syntax + oldSchemaUpdateEvents.includes(event as OldSchemaUpdateEvent); + +const isNewSchemaUpdateEvent = (event: string): event is NewSchemaUpdateEvent => + // eslint-disable-next-line no-restricted-syntax + newSchemaUpdateEvents.includes(event as NewSchemaUpdateEvent); + +const alteration: AlterationScript = { + up: async (pool) => { + const { rows: hooks } = await pool.query<{ id: string; events: string[] }>(sql` + select id, events + from hooks + `); + + const hooksToBeUpdate = hooks.filter(({ events }) => { + return oldSchemaUpdateEvents.some((oldEvent) => events.includes(oldEvent)); + }); + + await Promise.all( + hooksToBeUpdate.map(async ({ id, events }) => { + const updateEvents = events.reduce((accumulator, event) => { + if (isOldSchemaUpdateEvent(event)) { + return [...accumulator, updateMap[event]]; + } + return [...accumulator, event]; + }, []); + + await pool.query(sql` + update hooks + set events = ${JSON.stringify(updateEvents)} + where id = ${id}; + `); + }) + ); + }, + down: async (pool) => { + const { rows: hooks } = await pool.query<{ id: string; events: string[] }>(sql` + select id, events + from hooks + `); + + const hooksToBeUpdate = hooks.filter(({ events }) => { + return newSchemaUpdateEvents.some((newEvent) => events.includes(newEvent)); + }); + + await Promise.all( + hooksToBeUpdate.map(async ({ id, events }) => { + const updateEvents = events.reduce((accumulator, event) => { + if (isNewSchemaUpdateEvent(event)) { + return [...accumulator, reverseMap[event]]; + } + return [...accumulator, event]; + }, []); + + await pool.query(sql` + update hooks + set events = ${JSON.stringify(updateEvents)} + where id = ${id}; + `); + }) + ); + }, +}; + +export default alteration; diff --git a/packages/schemas/src/foundations/jsonb-types/hooks.ts b/packages/schemas/src/foundations/jsonb-types/hooks.ts index 3505b8107..fc47556d8 100644 --- a/packages/schemas/src/foundations/jsonb-types/hooks.ts +++ b/packages/schemas/src/foundations/jsonb-types/hooks.ts @@ -27,6 +27,9 @@ export enum DataHookSchema { enum DataHookBasicMutationType { Created = 'Created', Deleted = 'Deleted', +} + +enum DataHookDetailMutationType { Updated = 'Updated', } @@ -34,13 +37,14 @@ type BasicDataHookEvent = `${DataHookSchema}.${DataHookBasicMutationType}`; // Custom DataHook mutable schemas type CustomDataHookMutableSchema = + | `${DataHookSchema}.Data` | `${DataHookSchema.User}.SuspensionStatus` | `${DataHookSchema.Role}.Scopes` | `${DataHookSchema.Organization}.Membership` | `${DataHookSchema.OrganizationRole}.Scopes`; type DataHookPropertyUpdateEvent = - `${CustomDataHookMutableSchema}.${DataHookBasicMutationType.Updated}`; + `${CustomDataHookMutableSchema}.${DataHookDetailMutationType.Updated}`; export type DataHookEvent = BasicDataHookEvent | DataHookPropertyUpdateEvent; @@ -51,26 +55,26 @@ export const hookEvents = Object.freeze([ InteractionHookEvent.PostResetPassword, 'User.Created', 'User.Deleted', - 'User.Updated', + 'User.Data.Updated', 'User.SuspensionStatus.Updated', 'Role.Created', 'Role.Deleted', - 'Role.Updated', + 'Role.Data.Updated', 'Role.Scopes.Updated', 'Scope.Created', 'Scope.Deleted', - 'Scope.Updated', + 'Scope.Data.Updated', 'Organization.Created', 'Organization.Deleted', - 'Organization.Updated', + 'Organization.Data.Updated', 'Organization.Membership.Updated', 'OrganizationRole.Created', 'OrganizationRole.Deleted', - 'OrganizationRole.Updated', + 'OrganizationRole.Data.Updated', 'OrganizationRole.Scopes.Updated', 'OrganizationScope.Created', 'OrganizationScope.Deleted', - 'OrganizationScope.Updated', + 'OrganizationScope.Data.Updated', ] as const satisfies Array); /** The type of hook event values that can be registered. */ @@ -114,31 +118,31 @@ type ApiMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; export const managementApiHooksRegistration = Object.freeze({ 'POST /users': 'User.Created', 'DELETE /users/:userId': 'User.Deleted', - 'PATCH /users/:userId': 'User.Updated', - 'PATCH /users/:userId/custom-data': 'User.Updated', - 'PATCH /users/:userId/profile': 'User.Updated', - 'PATCH /users/:userId/password': 'User.Updated', + 'PATCH /users/:userId': 'User.Data.Updated', + 'PATCH /users/:userId/custom-data': 'User.Data.Updated', + 'PATCH /users/:userId/profile': 'User.Data.Updated', + 'PATCH /users/:userId/password': 'User.Data.Updated', 'PATCH /users/:userId/is-suspended': 'User.SuspensionStatus.Updated', 'POST /roles': 'Role.Created', 'DELETE /roles/:id': 'Role.Deleted', - 'PATCH /roles/:id': 'Role.Updated', + 'PATCH /roles/:id': 'Role.Data.Updated', 'POST /roles/:id/scopes': 'Role.Scopes.Updated', 'DELETE /roles/:id/scopes/:scopeId': 'Role.Scopes.Updated', 'POST /resources/:resourceId/scopes': 'Scope.Created', 'DELETE /resources/:resourceId/scopes/:scopeId': 'Scope.Deleted', - 'PATCH /resources/:resourceId/scopes/:scopeId': 'Scope.Updated', + 'PATCH /resources/:resourceId/scopes/:scopeId': 'Scope.Data.Updated', 'POST /organizations': 'Organization.Created', 'DELETE /organizations/:id': 'Organization.Deleted', - 'PATCH /organizations/:id': 'Organization.Updated', + 'PATCH /organizations/:id': 'Organization.Data.Updated', 'PUT /organizations/:id/users': 'Organization.Membership.Updated', 'POST /organizations/:id/users': 'Organization.Membership.Updated', 'DELETE /organizations/:id/users/:userId': 'Organization.Membership.Updated', 'POST /organization-roles': 'OrganizationRole.Created', 'DELETE /organization-roles/:id': 'OrganizationRole.Deleted', - 'PATCH /organization-roles/:id': 'OrganizationRole.Updated', + 'PATCH /organization-roles/:id': 'OrganizationRole.Data.Updated', 'POST /organization-scopes': 'OrganizationScope.Created', 'DELETE /organization-scopes/:id': 'OrganizationScope.Deleted', - 'PATCH /organization-scopes/:id': 'OrganizationScope.Updated', + 'PATCH /organization-scopes/:id': 'OrganizationScope.Data.Updated', 'PUT /organization-roles/:id/scopes': 'OrganizationRole.Scopes.Updated', 'POST /organization-roles/:id/scopes': 'OrganizationRole.Scopes.Updated', 'DELETE /organization-roles/:id/scopes/:organizationScopeId': 'OrganizationRole.Scopes.Updated',