diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts index 4824b85ba6..29bd09fb0c 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts @@ -217,8 +217,10 @@ describe('Source Control', () => { }); it('should get tracking information from pre-push results', () => { - const trackingResult = getTrackingInformationFromPrePushResult(pushResult); + const userId = 'userId'; + const trackingResult = getTrackingInformationFromPrePushResult(userId, pushResult); expect(trackingResult).toEqual({ + userId, workflowsEligible: 3, workflowsEligibleWithConflicts: 1, credsEligible: 1, @@ -228,8 +230,10 @@ describe('Source Control', () => { }); it('should get tracking information from post-push results', () => { - const trackingResult = getTrackingInformationFromPostPushResult(pushResult); + const userId = 'userId'; + const trackingResult = getTrackingInformationFromPostPushResult(userId, pushResult); expect(trackingResult).toEqual({ + userId, workflowsPushed: 2, workflowsEligible: 3, credsPushed: 1, @@ -238,8 +242,10 @@ describe('Source Control', () => { }); it('should get tracking information from pull results', () => { - const trackingResult = getTrackingInformationFromPullResult(pullResult); + const userId = 'userId'; + const trackingResult = getTrackingInformationFromPullResult(userId, pullResult); expect(trackingResult).toEqual({ + userId, credConflicts: 1, workflowConflicts: 1, workflowUpdates: 3, diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control.controller.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control.controller.ee.test.ts new file mode 100644 index 0000000000..7614f9ed33 --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control.controller.ee.test.ts @@ -0,0 +1,103 @@ +import type { PullWorkFolderRequestDto, PushWorkFolderRequestDto } from '@n8n/api-types'; +import type { Response } from 'express'; +import { mock } from 'jest-mock-extended'; + +import type { EventService } from '@/events/event.service'; +import type { AuthenticatedRequest } from '@/requests'; + +import type { SourceControlPreferencesService } from '../source-control-preferences.service.ee'; +import { SourceControlController } from '../source-control.controller.ee'; +import type { SourceControlService } from '../source-control.service.ee'; +import type { SourceControlRequest } from '../types/requests'; +import type { SourceControlGetStatus } from '../types/source-control-get-status'; + +describe('SourceControlController', () => { + let controller: SourceControlController; + let sourceControlService: SourceControlService; + let sourceControlPreferencesService: SourceControlPreferencesService; + let eventService: EventService; + + beforeEach(() => { + sourceControlService = { + pushWorkfolder: jest.fn().mockResolvedValue({ statusCode: 200 }), + pullWorkfolder: jest.fn().mockResolvedValue({ statusCode: 200 }), + getStatus: jest.fn().mockResolvedValue([]), + setGitUserDetails: jest.fn(), + } as unknown as SourceControlService; + + sourceControlPreferencesService = mock(); + eventService = mock(); + + controller = new SourceControlController( + sourceControlService, + sourceControlPreferencesService, + eventService, + ); + }); + + describe('pushWorkfolder', () => { + it('should push workfolder with expected parameters', async () => { + const req = mock({ + user: { firstName: 'John', lastName: 'Doe', email: 'john.doe@example.com' }, + }); + const res = mock(); + const payload = { force: true } as PushWorkFolderRequestDto; + + await controller.pushWorkfolder(req, res, payload); + expect(sourceControlService.setGitUserDetails).toHaveBeenCalledWith( + 'John Doe', + 'john.doe@example.com', + ); + expect(sourceControlService.pushWorkfolder).toHaveBeenCalledWith(req.user, payload); + }); + }); + + describe('pullWorkfolder', () => { + it('should pull workfolder with expected parameters', async () => { + const req = mock({ + user: { firstName: 'John', lastName: 'Doe', email: 'john.doe@example.com' }, + }); + const res = mock(); + const payload = { force: true } as PullWorkFolderRequestDto; + + await controller.pullWorkfolder(req, res, payload); + expect(sourceControlService.pullWorkfolder).toHaveBeenCalledWith(req.user, payload); + }); + }); + + describe('getStatus', () => { + it('should call getStatus with expected parameters', async () => { + const user = { firstName: 'John', lastName: 'Doe', email: 'john.doe@example.com' }; + const query = { + direction: 'pull', + preferLocalVersion: true, + verbose: false, + } as SourceControlGetStatus; + const req = mock({ + query, + user, + }); + + await controller.getStatus(req); + expect(sourceControlService.getStatus).toHaveBeenCalledWith(user, query); + }); + }); + + describe('status', () => { + it('should call getStatus with expected parameters', async () => { + const user = { firstName: 'John', lastName: 'Doe', email: 'john.doe@example.com' }; + const query = { + direction: 'pull', + preferLocalVersion: true, + verbose: false, + } as SourceControlGetStatus; + const req = mock({ + query, + user, + }); + + await controller.status(req); + expect(sourceControlService.getStatus).toHaveBeenCalledWith(user, query); + }); + }); +}); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts index e975a3371b..0de5ca4c7f 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts @@ -45,8 +45,9 @@ describe('SourceControlService', () => { describe('pushWorkfolder', () => { it('should throw an error if a file is given that is not in the workfolder', async () => { + const user = mock(); await expect( - sourceControlService.pushWorkfolder({ + sourceControlService.pushWorkfolder(user, { fileNames: [ { file: '/etc/passwd', @@ -115,8 +116,9 @@ describe('SourceControlService', () => { }); it('should throw an error if a file is given that is not in the workfolder', async () => { + const user = mock(); await expect( - sourceControlService.pushWorkfolder({ + sourceControlService.pushWorkfolder(user, { fileNames: [ { file: '/etc/passwd', @@ -138,6 +140,7 @@ describe('SourceControlService', () => { describe('getStatus', () => { it('conflict depends on the value of `direction`', async () => { // ARRANGE + const user = mock(); // Define a credential that does only exist locally. // Pulling this would delete it so it should be marked as a conflict. @@ -200,13 +203,13 @@ describe('SourceControlService', () => { }); // ACT - const pullResult = await sourceControlService.getStatus({ + const pullResult = await sourceControlService.getStatus(user, { direction: 'pull', verbose: false, preferLocalVersion: false, }); - const pushResult = await sourceControlService.getStatus({ + const pushResult = await sourceControlService.getStatus(user, { direction: 'push', verbose: false, preferLocalVersion: false, diff --git a/packages/cli/src/environments.ee/source-control/source-control-helper.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-helper.ee.ts index 696a9ec566..1dc436ac6f 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-helper.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-helper.ee.ts @@ -131,9 +131,13 @@ function filterSourceControlledFilesUniqueIds(files: SourceControlledFile[]) { ); } -export function getTrackingInformationFromPullResult(result: SourceControlledFile[]) { +export function getTrackingInformationFromPullResult( + userId: string, + result: SourceControlledFile[], +) { const uniques = filterSourceControlledFilesUniqueIds(result); return { + userId, credConflicts: uniques.filter( (file) => file.type === 'credential' && file.status === 'modified' && file.location === 'local', @@ -145,9 +149,13 @@ export function getTrackingInformationFromPullResult(result: SourceControlledFil }; } -export function getTrackingInformationFromPrePushResult(result: SourceControlledFile[]) { +export function getTrackingInformationFromPrePushResult( + userId: string, + result: SourceControlledFile[], +) { const uniques = filterSourceControlledFilesUniqueIds(result); return { + userId, workflowsEligible: uniques.filter((file) => file.type === 'workflow').length, workflowsEligibleWithConflicts: uniques.filter( (file) => file.type === 'workflow' && file.conflict, @@ -160,9 +168,13 @@ export function getTrackingInformationFromPrePushResult(result: SourceControlled }; } -export function getTrackingInformationFromPostPushResult(result: SourceControlledFile[]) { +export function getTrackingInformationFromPostPushResult( + userId: string, + result: SourceControlledFile[], +) { const uniques = filterSourceControlledFilesUniqueIds(result); return { + userId, workflowsPushed: uniques.filter((file) => file.pushed && file.type === 'workflow').length ?? 0, workflowsEligible: uniques.filter((file) => file.type === 'workflow').length ?? 0, credsPushed: diff --git a/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts b/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts index 1f243a1447..88269a199c 100644 --- a/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts @@ -175,7 +175,8 @@ export class SourceControlController { `${req.user.firstName} ${req.user.lastName}`, req.user.email, ); - const result = await this.sourceControlService.pushWorkfolder(payload); + + const result = await this.sourceControlService.pushWorkfolder(req.user, payload); res.statusCode = result.statusCode; return result.statusResult; } catch (error) { @@ -213,6 +214,7 @@ export class SourceControlController { async getStatus(req: SourceControlRequest.GetStatus) { try { const result = (await this.sourceControlService.getStatus( + req.user, new SourceControlGetStatus(req.query), )) as SourceControlledFile[]; return result; @@ -224,7 +226,10 @@ export class SourceControlController { @Get('/status', { middlewares: [sourceControlLicensedMiddleware] }) async status(req: SourceControlRequest.GetStatus) { try { - return await this.sourceControlService.getStatus(new SourceControlGetStatus(req.query)); + return await this.sourceControlService.getStatus( + req.user, + new SourceControlGetStatus(req.query), + ); } catch (error) { throw new BadRequestError((error as { message: string }).message); } diff --git a/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts index cce6ddc6c8..51d7e587f2 100644 --- a/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts @@ -213,7 +213,10 @@ export class SourceControlService { return; } - async pushWorkfolder(options: PushWorkFolderRequestDto): Promise<{ + async pushWorkfolder( + user: User, + options: PushWorkFolderRequestDto, + ): Promise<{ statusCode: number; pushResult: PushResult | undefined; statusResult: SourceControlledFile[]; @@ -239,7 +242,7 @@ export class SourceControlService { // only determine file status if not provided by the frontend let statusResult: SourceControlledFile[] = filesToPush; if (statusResult.length === 0) { - statusResult = (await this.getStatus({ + statusResult = (await this.getStatus(user, { direction: 'push', verbose: false, preferLocalVersion: true, @@ -332,7 +335,7 @@ export class SourceControlService { // #region Tracking Information this.eventService.emit( 'source-control-user-finished-push-ui', - getTrackingInformationFromPostPushResult(statusResult), + getTrackingInformationFromPostPushResult(user.id, statusResult), ); // #endregion @@ -393,7 +396,7 @@ export class SourceControlService { ): Promise<{ statusCode: number; statusResult: SourceControlledFile[] }> { await this.sanityCheck(); - const statusResult = (await this.getStatus({ + const statusResult = (await this.getStatus(user, { direction: 'pull', verbose: false, preferLocalVersion: false, @@ -457,7 +460,7 @@ export class SourceControlService { // #region Tracking Information this.eventService.emit( 'source-control-user-finished-pull-ui', - getTrackingInformationFromPullResult(statusResult), + getTrackingInformationFromPullResult(user.id, statusResult), ); // #endregion @@ -477,7 +480,7 @@ export class SourceControlService { * @returns either SourceControlledFile[] if verbose is false, * or multiple SourceControlledFile[] with all determined differences for debugging purposes */ - async getStatus(options: SourceControlGetStatus) { + async getStatus(user: User, options: SourceControlGetStatus) { await this.sanityCheck(); const sourceControlledFiles: SourceControlledFile[] = []; @@ -514,12 +517,12 @@ export class SourceControlService { if (options.direction === 'push') { this.eventService.emit( 'source-control-user-started-push-ui', - getTrackingInformationFromPrePushResult(sourceControlledFiles), + getTrackingInformationFromPrePushResult(user.id, sourceControlledFiles), ); } else if (options.direction === 'pull') { this.eventService.emit( 'source-control-user-started-pull-ui', - getTrackingInformationFromPullResult(sourceControlledFiles), + getTrackingInformationFromPullResult(user.id, sourceControlledFiles), ); } // #endregion diff --git a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts index 8f73b475ca..1b063af8bf 100644 --- a/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/telemetry-event-relay.test.ts @@ -225,12 +225,14 @@ describe('TelemetryEventRelay', () => { it('should track on `source-control-user-finished-pull-ui` event', () => { const event: RelayEventMap['source-control-user-finished-pull-ui'] = { + userId: 'userId', workflowUpdates: 3, }; eventService.emit('source-control-user-finished-pull-ui', event); expect(telemetry.track).toHaveBeenCalledWith('User finished pull via UI', { + user_id: 'userId', workflow_updates: 3, }); }); @@ -251,6 +253,7 @@ describe('TelemetryEventRelay', () => { it('should track on `source-control-user-started-push-ui` event', () => { const event: RelayEventMap['source-control-user-started-push-ui'] = { + userId: 'userId', workflowsEligible: 10, workflowsEligibleWithConflicts: 2, credsEligible: 5, @@ -261,6 +264,7 @@ describe('TelemetryEventRelay', () => { eventService.emit('source-control-user-started-push-ui', event); expect(telemetry.track).toHaveBeenCalledWith('User started push via UI', { + user_id: 'userId', workflows_eligible: 10, workflows_eligible_with_conflicts: 2, creds_eligible: 5, @@ -271,6 +275,7 @@ describe('TelemetryEventRelay', () => { it('should track on `source-control-user-finished-push-ui` event', () => { const event: RelayEventMap['source-control-user-finished-push-ui'] = { + userId: 'userId', workflowsEligible: 10, workflowsPushed: 8, credsPushed: 5, @@ -280,6 +285,7 @@ describe('TelemetryEventRelay', () => { eventService.emit('source-control-user-finished-push-ui', event); expect(telemetry.track).toHaveBeenCalledWith('User finished push via UI', { + user_id: 'userId', workflows_eligible: 10, workflows_pushed: 8, creds_pushed: 5, diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index b174c66587..a7d0726f08 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -381,12 +381,14 @@ export type RelayEventMap = { }; 'source-control-user-started-pull-ui': { + userId?: string; workflowUpdates: number; workflowConflicts: number; credConflicts: number; }; 'source-control-user-finished-pull-ui': { + userId?: string; workflowUpdates: number; }; @@ -396,6 +398,7 @@ export type RelayEventMap = { }; 'source-control-user-started-push-ui': { + userId?: string; workflowsEligible: number; workflowsEligibleWithConflicts: number; credsEligible: number; @@ -404,6 +407,7 @@ export type RelayEventMap = { }; 'source-control-user-finished-push-ui': { + userId: string; workflowsEligible: number; workflowsPushed: number; credsPushed: number; diff --git a/packages/cli/src/events/relays/telemetry.event-relay.ts b/packages/cli/src/events/relays/telemetry.event-relay.ts index f8102f70ae..1423bffbd1 100644 --- a/packages/cli/src/events/relays/telemetry.event-relay.ts +++ b/packages/cli/src/events/relays/telemetry.event-relay.ts @@ -170,11 +170,13 @@ export class TelemetryEventRelay extends EventRelay { } private sourceControlUserStartedPullUi({ + userId, workflowUpdates, workflowConflicts, credConflicts, }: RelayEventMap['source-control-user-started-pull-ui']) { this.telemetry.track('User started pull via UI', { + user_id: userId, workflow_updates: workflowUpdates, workflow_conflicts: workflowConflicts, cred_conflicts: credConflicts, @@ -182,9 +184,11 @@ export class TelemetryEventRelay extends EventRelay { } private sourceControlUserFinishedPullUi({ + userId, workflowUpdates, }: RelayEventMap['source-control-user-finished-pull-ui']) { this.telemetry.track('User finished pull via UI', { + user_id: userId, workflow_updates: workflowUpdates, }); } @@ -200,6 +204,7 @@ export class TelemetryEventRelay extends EventRelay { } private sourceControlUserStartedPushUi({ + userId, workflowsEligible, workflowsEligibleWithConflicts, credsEligible, @@ -207,6 +212,7 @@ export class TelemetryEventRelay extends EventRelay { variablesEligible, }: RelayEventMap['source-control-user-started-push-ui']) { this.telemetry.track('User started push via UI', { + user_id: userId, workflows_eligible: workflowsEligible, workflows_eligible_with_conflicts: workflowsEligibleWithConflicts, creds_eligible: credsEligible, @@ -216,12 +222,14 @@ export class TelemetryEventRelay extends EventRelay { } private sourceControlUserFinishedPushUi({ + userId, workflowsEligible, workflowsPushed, credsPushed, variablesPushed, }: RelayEventMap['source-control-user-finished-push-ui']) { this.telemetry.track('User finished push via UI', { + user_id: userId, workflows_eligible: workflowsEligible, workflows_pushed: workflowsPushed, creds_pushed: credsPushed, diff --git a/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts b/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts index 4a0729ef6e..60339bafc2 100644 --- a/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts @@ -40,7 +40,7 @@ export = { if (result.statusCode === 200) { Container.get(EventService).emit('source-control-user-pulled-api', { - ...getTrackingInformationFromPullResult(result.statusResult), + ...getTrackingInformationFromPullResult(req.user.id, result.statusResult), forced: payload.force ?? false, }); return res.status(200).send(result.statusResult);