refactor(core): Decouple emailing and workflow sharing from internal hooks (no-changelog) (#10326)

This commit is contained in:
Iván Ovejero 2024-08-08 12:33:18 +02:00 committed by GitHub
parent ee8c9a5b24
commit ee03400c25
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 47 additions and 47 deletions

View file

@ -1,5 +1,4 @@
import { Service } from 'typedi'; import { Service } from 'typedi';
import type { ITelemetryTrackProperties } from 'n8n-workflow';
import type { User } from '@db/entities/User'; import type { User } from '@db/entities/User';
import { Telemetry } from '@/telemetry'; import { Telemetry } from '@/telemetry';
import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus'; import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus';
@ -23,16 +22,6 @@ export class InternalHooks {
await this.telemetry.init(); 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 }) { onUserInviteEmailClick(userInviteClickData: { inviter: User; invitee: User }) {
this.telemetry.track('User clicked invite link from email', { this.telemetry.track('User clicked invite link from email', {
user_id: userInviteClickData.invitee.id, user_id: userInviteClickData.invitee.id,
@ -63,19 +52,4 @@ export class InternalHooks {
user_id: userPasswordResetData.user.id, 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,
});
}
} }

View file

@ -120,14 +120,10 @@ export class UserManagementMailer {
return result; return result;
} catch (e) { } catch (e) {
Container.get(InternalHooks).onEmailFailed({
user: sharer,
message_type: 'Workflow shared',
public_api: false,
});
Container.get(EventService).emit('email-failed', { Container.get(EventService).emit('email-failed', {
user: sharer, user: sharer,
messageType: 'Workflow shared', messageType: 'Workflow shared',
publicApi: false,
}); });
const error = toError(e); const error = toError(e);
@ -179,14 +175,10 @@ export class UserManagementMailer {
return result; return result;
} catch (e) { } catch (e) {
Container.get(InternalHooks).onEmailFailed({
user: sharer,
message_type: 'Credentials shared',
public_api: false,
});
Container.get(EventService).emit('email-failed', { Container.get(EventService).emit('email-failed', {
user: sharer, user: sharer,
messageType: 'Credentials shared', messageType: 'Credentials shared',
publicApi: false,
}); });
const error = toError(e); const error = toError(e);

View file

@ -120,12 +120,11 @@ export class PasswordResetController {
domain: this.urlService.getInstanceBaseUrl(), domain: this.urlService.getInstanceBaseUrl(),
}); });
} catch (error) { } catch (error) {
this.internalHooks.onEmailFailed({ this.eventService.emit('email-failed', {
user, user,
message_type: 'Reset password', messageType: 'Reset password',
public_api: false, publicApi: false,
}); });
this.eventService.emit('email-failed', { user, messageType: 'Reset password' });
if (error instanceof Error) { if (error instanceof Error) {
throw new InternalServerError(`Please contact your administrator: ${error.message}`); throw new InternalServerError(`Please contact your administrator: ${error.message}`);
} }

View file

@ -835,6 +835,7 @@ describe('LogStreamingEventRelay', () => {
role: 'global:member', role: 'global:member',
}, },
messageType: 'New user invite', messageType: 'New user invite',
publicApi: false,
}; };
eventService.emit('email-failed', event); eventService.emit('email-failed', event);

View file

@ -78,6 +78,12 @@ export type RelayEventMap = {
runData?: IRun; runData?: IRun;
}; };
'workflow-sharing-updated': {
workflowId: string;
userIdSharer: string;
userIdList: string[];
};
// #endregion // #endregion
// #region Node // #region Node
@ -234,6 +240,7 @@ export type RelayEventMap = {
| 'Resend invite' | 'Resend invite'
| 'Workflow shared' | 'Workflow shared'
| 'Credentials shared'; | 'Credentials shared';
publicApi: boolean;
}; };
// #endregion // #endregion

View file

@ -71,6 +71,7 @@ export class TelemetryEventRelay extends EventRelay {
'login-failed-due-to-ldap-disabled': (event) => this.loginFailedDueToLdapDisabled(event), 'login-failed-due-to-ldap-disabled': (event) => this.loginFailedDueToLdapDisabled(event),
'workflow-created': (event) => this.workflowCreated(event), 'workflow-created': (event) => this.workflowCreated(event),
'workflow-deleted': (event) => this.workflowDeleted(event), 'workflow-deleted': (event) => this.workflowDeleted(event),
'workflow-sharing-updated': (event) => this.workflowSharingUpdated(event),
'workflow-saved': async (event) => await this.workflowSaved(event), 'workflow-saved': async (event) => await this.workflowSaved(event),
'server-started': async () => await this.serverStarted(), 'server-started': async () => await this.serverStarted(),
'session-started': (event) => this.sessionStarted(event), 'session-started': (event) => this.sessionStarted(event),
@ -93,6 +94,7 @@ export class TelemetryEventRelay extends EventRelay {
'user-signed-up': (event) => this.userSignedUp(event), 'user-signed-up': (event) => this.userSignedUp(event),
'user-submitted-personalization-survey': (event) => 'user-submitted-personalization-survey': (event) =>
this.userSubmittedPersonalizationSurvey(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']) { private async workflowSaved({ user, workflow, publicApi }: RelayEventMap['workflow-saved']) {
const isCloudDeployment = config.getEnv('deployment.type') === 'cloud'; const isCloudDeployment = config.getEnv('deployment.type') === 'cloud';
@ -930,4 +944,16 @@ export class TelemetryEventRelay extends EventRelay {
} }
// #endregion // #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
} }

View file

@ -160,12 +160,11 @@ export class UserService {
}); });
} catch (e) { } catch (e) {
if (e instanceof Error) { if (e instanceof Error) {
Container.get(InternalHooks).onEmailFailed({ this.eventService.emit('email-failed', {
user: owner, user: owner,
message_type: 'New user invite', messageType: 'New user invite',
public_api: false, publicApi: false,
}); });
this.eventService.emit('email-failed', { user: owner, messageType: 'New user invite' });
this.logger.error('Failed to send email', { this.logger.error('Failed to send email', {
userId: owner.id, userId: owner.id,
inviteAcceptUrl, inviteAcceptUrl,

View file

@ -17,7 +17,6 @@ import { validateEntity } from '@/GenericHelpers';
import { ExternalHooks } from '@/ExternalHooks'; import { ExternalHooks } from '@/ExternalHooks';
import { WorkflowService } from './workflow.service'; import { WorkflowService } from './workflow.service';
import { License } from '@/License'; import { License } from '@/License';
import { InternalHooks } from '@/InternalHooks';
import * as utils from '@/utils'; import * as utils from '@/utils';
import { listQueryMiddleware } from '@/middlewares'; import { listQueryMiddleware } from '@/middlewares';
import { TagService } from '@/services/tag.service'; import { TagService } from '@/services/tag.service';
@ -49,7 +48,6 @@ import { GlobalConfig } from '@n8n/config';
export class WorkflowsController { export class WorkflowsController {
constructor( constructor(
private readonly logger: Logger, private readonly logger: Logger,
private readonly internalHooks: InternalHooks,
private readonly externalHooks: ExternalHooks, private readonly externalHooks: ExternalHooks,
private readonly tagRepository: TagRepository, private readonly tagRepository: TagRepository,
private readonly enterpriseWorkflowService: EnterpriseWorkflowService, private readonly enterpriseWorkflowService: EnterpriseWorkflowService,
@ -459,7 +457,11 @@ export class WorkflowsController {
newShareeIds = toShare; 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({ const projectsRelations = await this.projectRelationRepository.findBy({
projectId: In(newShareeIds), projectId: In(newShareeIds),