Skip to content

Commit

Permalink
refactor(core): Decouple emailing and workflow sharing from internal …
Browse files Browse the repository at this point in the history
…hooks (no-changelog)
  • Loading branch information
ivov committed Aug 8, 2024
1 parent aa95059 commit 23fbc13
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 47 deletions.
26 changes: 0 additions & 26 deletions packages/cli/src/InternalHooks.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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,
});
}
}
12 changes: 2 additions & 10 deletions packages/cli/src/UserManagement/email/UserManagementMailer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 3 additions & 4 deletions packages/cli/src/controllers/passwordReset.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@ describe('LogStreamingEventRelay', () => {
role: 'global:member',
},
messageType: 'New user invite',
publicApi: false,
};

eventService.emit('email-failed', event);
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/src/events/relay-event-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ export type RelayEventMap = {
runData?: IRun;
};

'workflow-sharing-updated': {
workflowId: string;
userIdSharer: string;
userIdList: string[];
};

// #endregion

// #region Node
Expand Down Expand Up @@ -234,6 +240,7 @@ export type RelayEventMap = {
| 'Resend invite'
| 'Workflow shared'
| 'Credentials shared';
publicApi: boolean;
};

// #endregion
Expand Down
26 changes: 26 additions & 0 deletions packages/cli/src/events/telemetry-event-relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
});
}

Expand Down Expand Up @@ -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';

Expand Down Expand Up @@ -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
}
7 changes: 3 additions & 4 deletions packages/cli/src/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions packages/cli/src/workflows/workflows.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit 23fbc13

Please sign in to comment.