From 23fbc138e0be9fcda634ad0be292c1f7eb170d1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 8 Aug 2024 11:49:54 +0200 Subject: [PATCH] refactor(core): Decouple emailing and workflow sharing from internal hooks (no-changelog) --- packages/cli/src/InternalHooks.ts | 26 ------------------- .../email/UserManagementMailer.ts | 12 ++------- .../controllers/passwordReset.controller.ts | 7 +++-- .../log-streaming-event-relay.test.ts | 1 + packages/cli/src/events/relay-event-map.ts | 7 +++++ .../cli/src/events/telemetry-event-relay.ts | 26 +++++++++++++++++++ packages/cli/src/services/user.service.ts | 7 +++-- .../cli/src/workflows/workflows.controller.ts | 8 +++--- 8 files changed, 47 insertions(+), 47 deletions(-) diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index c264bb2fbebda..26954a0472c0f 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -1,5 +1,4 @@ import { Service } from 'typedi'; -import type { ITelemetryTrackProperties } from 'n8n-workflow'; import type { User } from '@db/entities/User'; import { Telemetry } from '@/telemetry'; import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus'; @@ -23,16 +22,6 @@ export class InternalHooks { await this.telemetry.init(); } - onWorkflowSharingUpdate(workflowId: string, userId: string, userList: string[]) { - const properties: ITelemetryTrackProperties = { - workflow_id: workflowId, - user_id_sharer: userId, - user_id_list: userList, - }; - - this.telemetry.track('User updated workflow sharing', properties); - } - onUserInviteEmailClick(userInviteClickData: { inviter: User; invitee: User }) { this.telemetry.track('User clicked invite link from email', { user_id: userInviteClickData.invitee.id, @@ -63,19 +52,4 @@ export class InternalHooks { user_id: userPasswordResetData.user.id, }); } - - onEmailFailed(failedEmailData: { - user: User; - message_type: - | 'Reset password' - | 'New user invite' - | 'Resend invite' - | 'Workflow shared' - | 'Credentials shared'; - public_api: boolean; - }) { - this.telemetry.track('Instance failed to send transactional email to user', { - user_id: failedEmailData.user.id, - }); - } } diff --git a/packages/cli/src/UserManagement/email/UserManagementMailer.ts b/packages/cli/src/UserManagement/email/UserManagementMailer.ts index 1e092077b10de..459d8ec1c2668 100644 --- a/packages/cli/src/UserManagement/email/UserManagementMailer.ts +++ b/packages/cli/src/UserManagement/email/UserManagementMailer.ts @@ -120,14 +120,10 @@ export class UserManagementMailer { return result; } catch (e) { - Container.get(InternalHooks).onEmailFailed({ - user: sharer, - message_type: 'Workflow shared', - public_api: false, - }); Container.get(EventService).emit('email-failed', { user: sharer, messageType: 'Workflow shared', + publicApi: false, }); const error = toError(e); @@ -179,14 +175,10 @@ export class UserManagementMailer { return result; } catch (e) { - Container.get(InternalHooks).onEmailFailed({ - user: sharer, - message_type: 'Credentials shared', - public_api: false, - }); Container.get(EventService).emit('email-failed', { user: sharer, messageType: 'Credentials shared', + publicApi: false, }); const error = toError(e); diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index 82506e952bc32..458593d228a33 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -120,12 +120,11 @@ export class PasswordResetController { domain: this.urlService.getInstanceBaseUrl(), }); } catch (error) { - this.internalHooks.onEmailFailed({ + this.eventService.emit('email-failed', { user, - message_type: 'Reset password', - public_api: false, + messageType: 'Reset password', + publicApi: false, }); - this.eventService.emit('email-failed', { user, messageType: 'Reset password' }); if (error instanceof Error) { throw new InternalServerError(`Please contact your administrator: ${error.message}`); } diff --git a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts index c06289d9f816c..7254f5a09bfc6 100644 --- a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts @@ -835,6 +835,7 @@ describe('LogStreamingEventRelay', () => { role: 'global:member', }, messageType: 'New user invite', + publicApi: false, }; eventService.emit('email-failed', event); diff --git a/packages/cli/src/events/relay-event-map.ts b/packages/cli/src/events/relay-event-map.ts index 0081bf6c4288b..8d6463defbe29 100644 --- a/packages/cli/src/events/relay-event-map.ts +++ b/packages/cli/src/events/relay-event-map.ts @@ -78,6 +78,12 @@ export type RelayEventMap = { runData?: IRun; }; + 'workflow-sharing-updated': { + workflowId: string; + userIdSharer: string; + userIdList: string[]; + }; + // #endregion // #region Node @@ -234,6 +240,7 @@ export type RelayEventMap = { | 'Resend invite' | 'Workflow shared' | 'Credentials shared'; + publicApi: boolean; }; // #endregion diff --git a/packages/cli/src/events/telemetry-event-relay.ts b/packages/cli/src/events/telemetry-event-relay.ts index 77e1878e0b04d..a7eaf5ef2c27c 100644 --- a/packages/cli/src/events/telemetry-event-relay.ts +++ b/packages/cli/src/events/telemetry-event-relay.ts @@ -71,6 +71,7 @@ export class TelemetryEventRelay extends EventRelay { 'login-failed-due-to-ldap-disabled': (event) => this.loginFailedDueToLdapDisabled(event), 'workflow-created': (event) => this.workflowCreated(event), 'workflow-deleted': (event) => this.workflowDeleted(event), + 'workflow-sharing-updated': (event) => this.workflowSharingUpdated(event), 'workflow-saved': async (event) => await this.workflowSaved(event), 'server-started': async () => await this.serverStarted(), 'session-started': (event) => this.sessionStarted(event), @@ -93,6 +94,7 @@ export class TelemetryEventRelay extends EventRelay { 'user-signed-up': (event) => this.userSignedUp(event), 'user-submitted-personalization-survey': (event) => this.userSubmittedPersonalizationSurvey(event), + 'email-failed': (event) => this.emailFailed(event), }); } @@ -507,6 +509,18 @@ export class TelemetryEventRelay extends EventRelay { }); } + private workflowSharingUpdated({ + workflowId, + userIdSharer, + userIdList, + }: RelayEventMap['workflow-sharing-updated']) { + this.telemetry.track('User updated workflow sharing', { + workflow_id: workflowId, + user_id_sharer: userIdSharer, + user_id_list: userIdList, + }); + } + private async workflowSaved({ user, workflow, publicApi }: RelayEventMap['workflow-saved']) { const isCloudDeployment = config.getEnv('deployment.type') === 'cloud'; @@ -930,4 +944,16 @@ export class TelemetryEventRelay extends EventRelay { } // #endregion + + // #region Email + + private emailFailed({ user, messageType, publicApi }: RelayEventMap['email-failed']) { + this.telemetry.track('Instance failed to send transactional email to user', { + user_id: user.id, + message_type: messageType, + public_api: publicApi, + }); + } + + // #endregion } diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 61f76bd596d2f..6b17530b7cf57 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -160,12 +160,11 @@ export class UserService { }); } catch (e) { if (e instanceof Error) { - Container.get(InternalHooks).onEmailFailed({ + this.eventService.emit('email-failed', { user: owner, - message_type: 'New user invite', - public_api: false, + messageType: 'New user invite', + publicApi: false, }); - this.eventService.emit('email-failed', { user: owner, messageType: 'New user invite' }); this.logger.error('Failed to send email', { userId: owner.id, inviteAcceptUrl, diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 60dbbf8191c4f..4c03dd9e246c4 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -17,7 +17,6 @@ import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { WorkflowService } from './workflow.service'; import { License } from '@/License'; -import { InternalHooks } from '@/InternalHooks'; import * as utils from '@/utils'; import { listQueryMiddleware } from '@/middlewares'; import { TagService } from '@/services/tag.service'; @@ -49,7 +48,6 @@ import { GlobalConfig } from '@n8n/config'; export class WorkflowsController { constructor( private readonly logger: Logger, - private readonly internalHooks: InternalHooks, private readonly externalHooks: ExternalHooks, private readonly tagRepository: TagRepository, private readonly enterpriseWorkflowService: EnterpriseWorkflowService, @@ -459,7 +457,11 @@ export class WorkflowsController { newShareeIds = toShare; }); - this.internalHooks.onWorkflowSharingUpdate(workflowId, req.user.id, shareWithIds); + this.eventService.emit('workflow-sharing-updated', { + workflowId, + userIdSharer: req.user.id, + userIdList: shareWithIds, + }); const projectsRelations = await this.projectRelationRepository.findBy({ projectId: In(newShareeIds),