Skip to content

Commit

Permalink
refactor(core): Tear down internal hooks (no-changelog) (#10340)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov authored Aug 12, 2024
1 parent 7898498 commit 6b52beb
Show file tree
Hide file tree
Showing 23 changed files with 95 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type { ExternalSecretsSettings } from '@/Interfaces';
import { License } from '@/License';
import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
import { ExternalSecretsProviders } from '@/ExternalSecrets/ExternalSecretsProviders.ee';
import { InternalHooks } from '@/InternalHooks';
import { mockInstance } from '@test/mocking';
import {
DummyProvider,
Expand All @@ -22,7 +21,6 @@ describe('External Secrets Manager', () => {
const mockProvidersInstance = new MockProviders();
const license = mockInstance(License);
const settingsRepo = mockInstance(SettingsRepository);
mockInstance(InternalHooks);
const cipher = Container.get(Cipher);

let providersMock: ExternalSecretsProviders;
Expand Down
35 changes: 2 additions & 33 deletions packages/cli/src/InternalHooks.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Service } from 'typedi';
import type { User } from '@db/entities/User';
import { Telemetry } from '@/telemetry';
import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus';

/**
* @deprecated Do not add to this class. To add log streaming or telemetry events, use
* @deprecated Do not add to this class. It will be removed once we remove
* further dep cycles. To add log streaming or telemetry events, use
* `EventService` to emit the event and then use the `LogStreamingEventRelay` or
* `TelemetryEventRelay` to forward them to the event bus or telemetry.
*/
Expand All @@ -21,35 +21,4 @@ export class InternalHooks {
async init() {
await this.telemetry.init();
}

onUserInviteEmailClick(userInviteClickData: { inviter: User; invitee: User }) {
this.telemetry.track('User clicked invite link from email', {
user_id: userInviteClickData.invitee.id,
});
}

onUserPasswordResetEmailClick(userPasswordResetData: { user: User }) {
this.telemetry.track('User clicked password reset link from email', {
user_id: userPasswordResetData.user.id,
});
}

onUserTransactionalEmail(userTransactionalEmailData: {
user_id: string;
message_type:
| 'Reset password'
| 'New user invite'
| 'Resend invite'
| 'Workflow shared'
| 'Credentials shared';
public_api: boolean;
}) {
this.telemetry.track('Instance sent transactional email to user', userTransactionalEmailData);
}

onUserPasswordResetRequestClick(userPasswordResetData: { user: User }) {
this.telemetry.track('User requested password reset while logged out', {
user_id: userPasswordResetData.user.id,
});
}
}
17 changes: 8 additions & 9 deletions packages/cli/src/UserManagement/email/UserManagementMailer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { GlobalConfig } from '@n8n/config';
import type { User } from '@db/entities/User';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import { UserRepository } from '@db/repositories/user.repository';
import { InternalHooks } from '@/InternalHooks';
import { Logger } from '@/Logger';
import { UrlService } from '@/services/url.service';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
Expand Down Expand Up @@ -112,10 +111,10 @@ export class UserManagementMailer {

this.logger.info('Sent workflow shared email successfully', { sharerId: sharer.id });

Container.get(InternalHooks).onUserTransactionalEmail({
user_id: sharer.id,
message_type: 'Workflow shared',
public_api: false,
Container.get(EventService).emit('user-transactional-email-sent', {
userId: sharer.id,
messageType: 'Workflow shared',
publicApi: false,
});

return result;
Expand Down Expand Up @@ -167,10 +166,10 @@ export class UserManagementMailer {

this.logger.info('Sent credentials shared email successfully', { sharerId: sharer.id });

Container.get(InternalHooks).onUserTransactionalEmail({
user_id: sharer.id,
message_type: 'Credentials shared',
public_api: false,
Container.get(EventService).emit('user-transactional-email-sent', {
userId: sharer.id,
messageType: 'Credentials shared',
publicApi: false,
});

return result;
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { ExternalHooks } from '@/ExternalHooks';
import { NodeTypes } from '@/NodeTypes';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import type { N8nInstanceType } from '@/Interfaces';
import { InternalHooks } from '@/InternalHooks';
import { PostHogClient } from '@/posthog';
import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
import { initExpressionEvaluator } from '@/ExpressionEvaluator';
Expand Down
2 changes: 0 additions & 2 deletions packages/cli/src/controllers/__tests__/me.controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { AUTH_COOKIE_NAME } from '@/constants';
import type { AuthenticatedRequest, MeRequest } from '@/requests';
import { UserService } from '@/services/user.service';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { UserRepository } from '@/databases/repositories/user.repository';
Expand All @@ -21,7 +20,6 @@ const browserId = 'test-browser-id';

describe('MeController', () => {
const externalHooks = mockInstance(ExternalHooks);
mockInstance(InternalHooks);
const eventService = mockInstance(EventService);
const userService = mockInstance(UserService);
const userRepository = mockInstance(UserRepository);
Expand Down
3 changes: 0 additions & 3 deletions packages/cli/src/controllers/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
isLdapCurrentAuthenticationMethod,
isSamlCurrentAuthenticationMethod,
} from '@/sso/ssoHelpers';
import { InternalHooks } from '../InternalHooks';
import { License } from '@/License';
import { UserService } from '@/services/user.service';
import { MfaService } from '@/Mfa/mfa.service';
Expand All @@ -30,7 +29,6 @@ import { EventService } from '@/events/event.service';
export class AuthController {
constructor(
private readonly logger: Logger,
private readonly internalHooks: InternalHooks,
private readonly authService: AuthService,
private readonly mfaService: MfaService,
private readonly userService: UserService,
Expand Down Expand Up @@ -179,7 +177,6 @@ export class AuthController {
throw new BadRequestError('Invalid request');
}

this.internalHooks.onUserInviteEmailClick({ inviter, invitee });
this.eventService.emit('user-invite-email-click', { inviter, invitee });

const { firstName, lastName } = inviter;
Expand Down
12 changes: 4 additions & 8 deletions packages/cli/src/controllers/passwordReset.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { MfaService } from '@/Mfa/mfa.service';
import { Logger } from '@/Logger';
import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks';
import { UrlService } from '@/services/url.service';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
Expand All @@ -28,7 +27,6 @@ export class PasswordResetController {
constructor(
private readonly logger: Logger,
private readonly externalHooks: ExternalHooks,
private readonly internalHooks: InternalHooks,
private readonly mailer: UserManagementMailer,
private readonly authService: AuthService,
private readonly userService: UserService,
Expand Down Expand Up @@ -131,13 +129,12 @@ export class PasswordResetController {
}

this.logger.info('Sent password reset email successfully', { userId: user.id, email });
this.internalHooks.onUserTransactionalEmail({
user_id: id,
message_type: 'Reset password',
public_api: false,
this.eventService.emit('user-transactional-email-sent', {
userId: id,
messageType: 'Reset password',
publicApi: false,
});

this.internalHooks.onUserPasswordResetRequestClick({ user });
this.eventService.emit('user-password-reset-request-click', { user });
}

Expand Down Expand Up @@ -170,7 +167,6 @@ export class PasswordResetController {
}

this.logger.info('Reset-password token resolved successfully', { userId: user.id });
this.internalHooks.onUserPasswordResetEmailClick({ user });
this.eventService.emit('user-password-reset-email-click', { user });
}

Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/events/relay-event-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,17 @@ export type RelayEventMap = {
user: UserLike;
};

'user-transactional-email-sent': {
userId: string;
messageType:
| 'Reset password'
| 'New user invite'
| 'Resend invite'
| 'Workflow shared'
| 'Credentials shared';
publicApi: boolean;
};

// #endregion

// #region Public API
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/src/events/telemetry-event-relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ export class TelemetryEventRelay extends EventRelay {
'user-submitted-personalization-survey': (event) =>
this.userSubmittedPersonalizationSurvey(event),
'email-failed': (event) => this.emailFailed(event),
'user-transactional-email-sent': (event) => this.userTransactionalEmailSent(event),
'user-invite-email-click': (event) => this.userInviteEmailClick(event),
'user-password-reset-email-click': (event) => this.userPasswordResetEmailClick(event),
'user-password-reset-request-click': (event) => this.userPasswordResetRequestClick(event),
});
}

Expand Down Expand Up @@ -955,5 +959,41 @@ export class TelemetryEventRelay extends EventRelay {
});
}

private userTransactionalEmailSent({
userId,
messageType,
publicApi,
}: RelayEventMap['user-transactional-email-sent']) {
this.telemetry.track('User sent transactional email', {
user_id: userId,
message_type: messageType,
public_api: publicApi,
});
}

// #endregion

// #region Click

private userInviteEmailClick({ invitee }: RelayEventMap['user-invite-email-click']) {
this.telemetry.track('User clicked invite link from email', {
user_id: invitee.id,
});
}

private userPasswordResetEmailClick({ user }: RelayEventMap['user-password-reset-email-click']) {
this.telemetry.track('User clicked password reset link from email', {
user_id: user.id,
});
}

private userPasswordResetRequestClick({
user,
}: RelayEventMap['user-password-reset-request-click']) {
this.telemetry.track('User requested password reset while logged out', {
user_id: user.id,
});
}

// #endregion
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { OrchestrationService } from '@/services/orchestration.service';
import config from '@/config';
import { ExecutionRecoveryService } from '@/executions/execution-recovery.service';
import { ExecutionRepository } from '@/databases/repositories/execution.repository';
import { InternalHooks } from '@/InternalHooks';
import { Push } from '@/push';
import { ARTIFICIAL_TASK_DATA } from '@/constants';
import { NodeCrashedError } from '@/errors/node-crashed.error';
Expand All @@ -26,7 +25,6 @@ import type { EventMessageTypes as EventMessage } from '@/eventbus/EventMessageC

describe('ExecutionRecoveryService', () => {
const push = mockInstance(Push);
mockInstance(InternalHooks);
const instanceSettings = new InstanceSettings();

let executionRecoveryService: ExecutionRecoveryService;
Expand Down
12 changes: 6 additions & 6 deletions packages/cli/src/services/user.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Container, Service } from 'typedi';
import { Service } from 'typedi';
import type { IUserSettings } from 'n8n-workflow';
import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';

Expand All @@ -8,7 +8,6 @@ import type { Invitation, PublicUser } from '@/Interfaces';
import type { PostHogClient } from '@/posthog';
import { Logger } from '@/Logger';
import { UserManagementMailer } from '@/UserManagement/email';
import { InternalHooks } from '@/InternalHooks';
import { UrlService } from '@/services/url.service';
import type { UserRequest } from '@/requests';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
Expand Down Expand Up @@ -144,10 +143,11 @@ export class UserService {
if (result.emailSent) {
invitedUser.user.emailSent = true;
delete invitedUser.user?.inviteAcceptUrl;
Container.get(InternalHooks).onUserTransactionalEmail({
user_id: id,
message_type: 'New user invite',
public_api: false,

this.eventService.emit('user-transactional-email-sent', {
userId: id,
messageType: 'New user invite',
publicApi: false,
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { nanoid } from 'nanoid';

import { InternalHooks } from '@/InternalHooks';
import { ImportCredentialsCommand } from '@/commands/import/credentials';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';

Expand All @@ -11,7 +10,6 @@ import { getAllCredentials, getAllSharedCredentials } from '../shared/db/credent
import { createMember, createOwner } from '../shared/db/users';
import { getPersonalProject } from '../shared/db/projects';

mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(ImportCredentialsCommand);

Expand Down
2 changes: 0 additions & 2 deletions packages/cli/test/integration/commands/import.cmd.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { nanoid } from 'nanoid';

import { InternalHooks } from '@/InternalHooks';
import { ImportWorkflowsCommand } from '@/commands/import/workflow';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';

Expand All @@ -11,7 +10,6 @@ import { getAllSharedWorkflows, getAllWorkflows } from '../shared/db/workflows';
import { createMember, createOwner } from '../shared/db/users';
import { getPersonalProject } from '../shared/db/projects';

mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(ImportWorkflowsCommand);

Expand Down
2 changes: 0 additions & 2 deletions packages/cli/test/integration/commands/ldap/reset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { EntityNotFoundError } from '@n8n/typeorm';

import { Reset } from '@/commands/ldap/reset';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { InternalHooks } from '@/InternalHooks';
import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { CredentialsRepository } from '@db/repositories/credentials.repository';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
Expand All @@ -26,7 +25,6 @@ import { createTeamProject, findProject, getPersonalProject } from '../../shared
mockInstance(Telemetry);

mockInstance(Push);
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(Reset);

Expand Down
2 changes: 0 additions & 2 deletions packages/cli/test/integration/commands/license.cmd.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { InternalHooks } from '@/InternalHooks';
import { License } from '@/License';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { ClearLicenseCommand } from '@/commands/license/clear';

import { setupTestCommand } from '@test-integration/utils/testCommand';
import { mockInstance } from '../../shared/mocking';

mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const license = mockInstance(License);
const command = setupTestCommand(ClearLicenseCommand);
Expand Down
2 changes: 0 additions & 2 deletions packages/cli/test/integration/commands/reset.cmd.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Container } from 'typedi';

import { Reset } from '@/commands/user-management/reset';
import { InternalHooks } from '@/InternalHooks';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { NodeTypes } from '@/NodeTypes';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
Expand All @@ -20,7 +19,6 @@ import { getPersonalProject } from '../shared/db/projects';
import { encryptCredentialData, saveCredential } from '../shared/db/credentials';
import { randomCredentialPayload } from '../shared/random';

mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
mockInstance(NodeTypes);
const command = setupTestCommand(Reset);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { InternalHooks } from '@/InternalHooks';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { UpdateWorkflowCommand } from '@/commands/update/workflow';

Expand All @@ -7,7 +6,6 @@ import * as testDb from '../../shared/testDb';
import { createWorkflowWithTrigger, getAllWorkflows } from '../../shared/db/workflows';
import { mockInstance } from '../../../shared/mocking';

mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
const command = setupTestCommand(UpdateWorkflowCommand);

Expand Down
Loading

0 comments on commit 6b52beb

Please sign in to comment.