refactor(core): Decouple lifecycle events from internal hooks (no-changelog) (#10305)

This commit is contained in:
Iván Ovejero
2024-08-07 16:09:42 +02:00
committed by GitHub
parent b232831f18
commit 9b977e80f6
11 changed files with 129 additions and 119 deletions

View File

@@ -19,6 +19,7 @@ import { UserService } from '@/services/user.service';
import { OwnershipService } from '@/services/ownership.service';
import { mockInstance } from '@test/mocking';
import type { Project } from '@/databases/entities/Project';
import type { EventService } from '@/events/event.service';
describe('WorkflowStatisticsService', () => {
const fakeUser = mock<User>({ id: 'abcde-fghij' });
@@ -44,21 +45,15 @@ describe('WorkflowStatisticsService', () => {
mocked(ownershipService.getProjectOwnerCached).mockResolvedValue(fakeUser);
const updateSettingsMock = jest.spyOn(userService, 'updateSettings').mockImplementation();
const eventService = mock<EventService>();
const workflowStatisticsService = new WorkflowStatisticsService(
mock(),
new WorkflowStatisticsRepository(dataSource, globalConfig),
ownershipService,
userService,
eventService,
);
const onFirstProductionWorkflowSuccess = jest.fn();
const onFirstWorkflowDataLoad = jest.fn();
workflowStatisticsService.on(
'telemetry.onFirstProductionWorkflowSuccess',
onFirstProductionWorkflowSuccess,
);
workflowStatisticsService.on('telemetry.onFirstWorkflowDataLoad', onFirstWorkflowDataLoad);
beforeEach(() => {
jest.clearAllMocks();
});
@@ -97,11 +92,10 @@ describe('WorkflowStatisticsService', () => {
await workflowStatisticsService.workflowExecutionCompleted(workflow, runData);
expect(updateSettingsMock).toHaveBeenCalledTimes(1);
expect(onFirstProductionWorkflowSuccess).toBeCalledTimes(1);
expect(onFirstProductionWorkflowSuccess).toHaveBeenNthCalledWith(1, {
project_id: fakeProject.id,
user_id: fakeUser.id,
workflow_id: workflow.id,
expect(eventService.emit).toHaveBeenCalledWith('first-production-workflow-succeeded', {
projectId: fakeProject.id,
workflowId: workflow.id,
userId: fakeUser.id,
});
});
@@ -124,7 +118,7 @@ describe('WorkflowStatisticsService', () => {
startedAt: new Date(),
};
await workflowStatisticsService.workflowExecutionCompleted(workflow, runData);
expect(onFirstProductionWorkflowSuccess).toBeCalledTimes(0);
expect(eventService.emit).not.toHaveBeenCalled();
});
test('should not send metrics for updated entries', async () => {
@@ -147,7 +141,7 @@ describe('WorkflowStatisticsService', () => {
};
mockDBCall(2);
await workflowStatisticsService.workflowExecutionCompleted(workflow, runData);
expect(onFirstProductionWorkflowSuccess).toBeCalledTimes(0);
expect(eventService.emit).not.toHaveBeenCalled();
});
});
@@ -164,13 +158,12 @@ describe('WorkflowStatisticsService', () => {
parameters: {},
};
await workflowStatisticsService.nodeFetchedData(workflowId, node);
expect(onFirstWorkflowDataLoad).toBeCalledTimes(1);
expect(onFirstWorkflowDataLoad).toHaveBeenNthCalledWith(1, {
user_id: fakeUser.id,
project_id: fakeProject.id,
workflow_id: workflowId,
node_type: node.type,
node_id: node.id,
expect(eventService.emit).toHaveBeenCalledWith('first-workflow-data-loaded', {
userId: fakeUser.id,
project: fakeProject.id,
workflowId,
nodeType: node.type,
nodeId: node.id,
});
});
@@ -192,15 +185,14 @@ describe('WorkflowStatisticsService', () => {
},
};
await workflowStatisticsService.nodeFetchedData(workflowId, node);
expect(onFirstWorkflowDataLoad).toBeCalledTimes(1);
expect(onFirstWorkflowDataLoad).toHaveBeenNthCalledWith(1, {
user_id: fakeUser.id,
project_id: fakeProject.id,
workflow_id: workflowId,
node_type: node.type,
node_id: node.id,
credential_type: 'testCredentials',
credential_id: node.credentials.testCredentials.id,
expect(eventService.emit).toHaveBeenCalledWith('first-workflow-data-loaded', {
userId: fakeUser.id,
project: fakeProject.id,
workflowId,
nodeType: node.type,
nodeId: node.id,
credentialType: 'testCredentials',
credentialId: node.credentials.testCredentials.id,
});
});
@@ -217,7 +209,7 @@ describe('WorkflowStatisticsService', () => {
parameters: {},
};
await workflowStatisticsService.nodeFetchedData(workflowId, node);
expect(onFirstWorkflowDataLoad).toBeCalledTimes(0);
expect(eventService.emit).not.toHaveBeenCalled();
});
});
});

View File

@@ -31,7 +31,7 @@ import { UserManagementMailer } from '@/UserManagement/email';
import type { CommunityPackagesService } from '@/services/communityPackages.service';
import { Logger } from '@/Logger';
import { UrlService } from './url.service';
import { InternalHooks } from '@/InternalHooks';
import { EventService } from '@/events/event.service';
import { isApiEnabled } from '@/PublicApi';
@Service()
@@ -50,7 +50,7 @@ export class FrontendService {
private readonly mailer: UserManagementMailer,
private readonly instanceSettings: InstanceSettings,
private readonly urlService: UrlService,
private readonly internalHooks: InternalHooks,
private readonly eventService: EventService,
) {
loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes());
void this.generateTypes();
@@ -244,7 +244,7 @@ export class FrontendService {
}
getSettings(pushRef?: string): IN8nUISettings {
this.internalHooks.onFrontendSettingsAPI(pushRef);
this.eventService.emit('session-started', { pushRef });
const restEndpoint = this.globalConfig.endpoints.rest;

View File

@@ -6,6 +6,7 @@ import { UserService } from '@/services/user.service';
import { Logger } from '@/Logger';
import { OwnershipService } from './ownership.service';
import { TypedEmitter } from '@/TypedEmitter';
import { EventService } from '@/events/event.service';
type WorkflowStatisticsEvents = {
nodeFetchedData: { workflowId: string; node: INode };
@@ -31,6 +32,7 @@ export class WorkflowStatisticsService extends TypedEmitter<WorkflowStatisticsEv
private readonly repository: WorkflowStatisticsRepository,
private readonly ownershipService: OwnershipService,
private readonly userService: UserService,
private readonly eventService: EventService,
) {
super({ captureRejections: true });
if ('SKIP_STATISTICS_EVENTS' in process.env) return;
@@ -72,12 +74,6 @@ export class WorkflowStatisticsService extends TypedEmitter<WorkflowStatisticsEv
if (project.type === 'personal') {
const owner = await this.ownershipService.getProjectOwnerCached(project.id);
const metrics = {
project_id: project.id,
workflow_id: workflowId,
user_id: owner!.id,
};
if (owner && !owner.settings?.userActivated) {
await this.userService.updateSettings(owner.id, {
firstSuccessfulWorkflowId: workflowId,
@@ -86,8 +82,11 @@ export class WorkflowStatisticsService extends TypedEmitter<WorkflowStatisticsEv
});
}
// Send the metrics
this.emit('telemetry.onFirstProductionWorkflowSuccess', metrics);
this.eventService.emit('first-production-workflow-succeeded', {
projectId: project.id,
workflowId,
userId: owner!.id,
});
}
}
} catch (error) {
@@ -109,24 +108,23 @@ export class WorkflowStatisticsService extends TypedEmitter<WorkflowStatisticsEv
const owner = await this.ownershipService.getProjectOwnerCached(project.id);
let metrics = {
user_id: owner!.id,
project_id: project.id,
workflow_id: workflowId,
node_type: node.type,
node_id: node.id,
userId: owner!.id,
project: project.id,
workflowId,
nodeType: node.type,
nodeId: node.id,
};
// This is probably naive but I can't see a way for a node to have multiple credentials attached so..
if (node.credentials) {
Object.entries(node.credentials).forEach(([credName, credDetails]) => {
metrics = Object.assign(metrics, {
credential_type: credName,
credential_id: credDetails.id,
credentialType: credName,
credentialId: credDetails.id,
});
});
}
// Send metrics to posthog
this.emit('telemetry.onFirstWorkflowDataLoad', metrics);
this.eventService.emit('first-workflow-data-loaded', metrics);
}
}