refactor(core): Use DTOs for source control push/pull requests (n8n-i…

…o#12470)

Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com>

packages/@n8n/api-types/src/dto/index.ts

@@ -36,6 +36,9 @@ export { UserUpdateRequestDto } from './user/user-update-request.dto';
 
 export { CommunityRegisteredRequestDto } from './license/community-registered-request.dto';
 
+export { PullWorkFolderRequestDto } from './source-control/pull-work-folder-request.dto';
+export { PushWorkFolderRequestDto } from './source-control/push-work-folder-request.dto';
+
 export { VariableListRequestDto } from './variables/variables-list-request.dto';
 export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one-request.dto';
 export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto';

packages/@n8n/api-types/src/dto/source-control/__tests__/pull-work-folder-request.dto.test.ts

@@ -0,0 +1,38 @@
+import { PullWorkFolderRequestDto } from '../pull-work-folder-request.dto';
+
+describe('PullWorkFolderRequestDto', () => {
+	describe('Valid requests', () => {
+		test.each([
+			{
+				name: 'with force',
+				request: { force: true },
+			},
+			{
+				name: 'without force',
+				request: {},
+			},
+		])('should validate $name', ({ request }) => {
+			const result = PullWorkFolderRequestDto.safeParse(request);
+			expect(result.success).toBe(true);
+		});
+	});
+
+	describe('Invalid requests', () => {
+		test.each([
+			{
+				name: 'invalid force type',
+				request: {
+					force: 'true', // Should be boolean
+				},
+				expectedErrorPath: ['force'],
+			},
+		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
+			const result = PullWorkFolderRequestDto.safeParse(request);
+			expect(result.success).toBe(false);
+
+			if (expectedErrorPath) {
+				expect(result.error?.issues[0].path).toEqual(expectedErrorPath);
+			}
+		});
+	});
+});

packages/@n8n/api-types/src/dto/source-control/__tests__/push-work-folder-request.dto.test.ts

@@ -0,0 +1,112 @@
+import { PushWorkFolderRequestDto } from '../push-work-folder-request.dto';
+
+describe('PushWorkFolderRequestDto', () => {
+	describe('Valid requests', () => {
+		test.each([
+			{
+				name: 'complete valid push request with all fields',
+				request: {
+					force: true,
+					fileNames: [
+						{
+							file: 'file1.json',
+							id: '1',
+							name: 'File 1',
+							type: 'workflow',
+							status: 'modified',
+							location: 'local',
+							conflict: false,
+							updatedAt: '2023-10-01T12:00:00Z',
+							pushed: true,
+						},
+					],
+					message: 'Initial commit',
+				},
+			},
+			{
+				name: 'push request with only required fields',
+				request: {
+					fileNames: [
+						{
+							file: 'file2.json',
+							id: '2',
+							name: 'File 2',
+							type: 'credential',
+							status: 'new',
+							location: 'remote',
+							conflict: true,
+							updatedAt: '2023-10-02T12:00:00Z',
+						},
+					],
+				},
+			},
+		])('should validate $name', ({ request }) => {
+			const result = PushWorkFolderRequestDto.safeParse(request);
+			expect(result.success).toBe(true);
+		});
+	});
+
+	describe('Invalid requests', () => {
+		test.each([
+			{
+				name: 'missing required fileNames field',
+				request: {
+					force: true,
+					message: 'Initial commit',
+				},
+				expectedErrorPath: ['fileNames'],
+			},
+			{
+				name: 'invalid fileNames type',
+				request: {
+					fileNames: 'not-an-array', // Should be an array
+				},
+				expectedErrorPath: ['fileNames'],
+			},
+			{
+				name: 'invalid fileNames array element',
+				request: {
+					fileNames: [
+						{
+							file: 'file4.json',
+							id: '4',
+							name: 'File 4',
+							type: 'invalid-type', // Invalid type
+							status: 'modified',
+							location: 'local',
+							conflict: false,
+							updatedAt: '2023-10-04T12:00:00Z',
+						},
+					],
+				},
+				expectedErrorPath: ['fileNames', 0, 'type'],
+			},
+			{
+				name: 'invalid force type',
+				request: {
+					force: 'true', // Should be boolean
+					fileNames: [
+						{
+							file: 'file5.json',
+							id: '5',
+							name: 'File 5',
+							type: 'workflow',
+							status: 'modified',
+							location: 'local',
+							conflict: false,
+							updatedAt: '2023-10-05T12:00:00Z',
+						},
+					],
+				},
+				expectedErrorPath: ['force'],
+			},
+		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
+			const result = PushWorkFolderRequestDto.safeParse(request);
+			expect(result.success).toBe(false);
+
+			if (expectedErrorPath) {
+				expect(result.error?.issues[0].path).toEqual(expectedErrorPath);
+			}
+		});
+	});
+});

packages/@n8n/api-types/src/dto/source-control/pull-work-folder-request.dto.ts

@@ -0,0 +1,6 @@
+import { z } from 'zod';
+import { Z } from 'zod-class';
+
+export class PullWorkFolderRequestDto extends Z.class({
+	force: z.boolean().optional(),
+}) {}

packages/@n8n/api-types/src/dto/source-control/push-work-folder-request.dto.ts

@@ -0,0 +1,10 @@
+import { z } from 'zod';
+import { Z } from 'zod-class';
+
+import { SourceControlledFileSchema } from '../../schemas/source-controlled-file.schema';
+
+export class PushWorkFolderRequestDto extends Z.class({
+	force: z.boolean().optional(),
+	commitMessage: z.string().optional(),
+	fileNames: z.array(SourceControlledFileSchema),
+}) {}

packages/@n8n/api-types/src/index.ts

@@ -10,9 +10,17 @@ export type { SendWorkerStatusMessage } from './push/worker';
 
 export type { BannerName } from './schemas/bannerName.schema';
 export { passwordSchema } from './schemas/password.schema';
+
 export {
 	ProjectType,
 	ProjectIcon,
 	ProjectRole,
 	ProjectRelation,
 } from './schemas/project.schema';
+
+export {
+	type SourceControlledFile,
+	SOURCE_CONTROL_FILE_LOCATION,
+	SOURCE_CONTROL_FILE_STATUS,
+	SOURCE_CONTROL_FILE_TYPE,
+} from './schemas/source-controlled-file.schema';

packages/@n8n/api-types/src/schemas/source-controlled-file.schema.ts

@@ -0,0 +1,34 @@
+import { z } from 'zod';
+
+const FileTypeSchema = z.enum(['credential', 'workflow', 'tags', 'variables', 'file']);
+export const SOURCE_CONTROL_FILE_TYPE = FileTypeSchema.Values;
+
+const FileStatusSchema = z.enum([
+	'new',
+	'modified',
+	'deleted',
+	'created',
+	'renamed',
+	'conflicted',
+	'ignored',
+	'staged',
+	'unknown',
+]);
+export const SOURCE_CONTROL_FILE_STATUS = FileStatusSchema.Values;
+
+const FileLocationSchema = z.enum(['local', 'remote']);
+export const SOURCE_CONTROL_FILE_LOCATION = FileLocationSchema.Values;
+
+export const SourceControlledFileSchema = z.object({
+	file: z.string(),
+	id: z.string(),
+	name: z.string(),
+	type: FileTypeSchema,
+	status: FileStatusSchema,
+	location: FileLocationSchema,
+	conflict: z.boolean(),
+	updatedAt: z.string(),
+	pushed: z.boolean().optional(),
+});
+
+export type SourceControlledFile = z.infer<typeof SourceControlledFileSchema>;

packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts

@@ -1,3 +1,4 @@
+import type { SourceControlledFile } from '@n8n/api-types';
 import { Container } from '@n8n/di';
 import mock from 'jest-mock-extended/lib/Mock';
 import { Cipher, type InstanceSettings } from 'n8n-core';
@@ -9,7 +10,6 @@ import { SharedCredentialsRepository } from '@/databases/repositories/shared-cre
 import { mockInstance } from '@test/mocking';
 
 import { SourceControlExportService } from '../source-control-export.service.ee';
-import type { SourceControlledFile } from '../types/source-controlled-file';
 
 // https://github.com/jestjs/jest/issues/4715
 function deepSpyOn<O extends object, M extends keyof O>(object: O, methodName: M) {

packages/cli/src/environments.ee/source-control/__tests__/source-control-helper.ee.test.ts

@@ -1,3 +1,4 @@
+import type { SourceControlledFile } from '@n8n/api-types';
 import { Container } from '@n8n/di';
 import { constants as fsConstants, accessSync } from 'fs';
 import { InstanceSettings } from 'n8n-core';
@@ -17,7 +18,6 @@ import {
 } from '@/environments.ee/source-control/source-control-helper.ee';
 import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee';
 import type { SourceControlPreferences } from '@/environments.ee/source-control/types/source-control-preferences';
-import type { SourceControlledFile } from '@/environments.ee/source-control/types/source-controlled-file';
 import { License } from '@/license';
 import { mockInstance } from '@test/mocking';
 

packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts

@@ -1,3 +1,4 @@
+import type { SourceControlledFile } from '@n8n/api-types';
 import { Container, Service } from '@n8n/di';
 import { rmSync } from 'fs';
 import { Credentials, InstanceSettings, Logger } from 'n8n-core';
@@ -29,7 +30,6 @@ import type { ExportResult } from './types/export-result';
 import type { ExportableCredential } from './types/exportable-credential';
 import type { ExportableWorkflow } from './types/exportable-workflow';
 import type { ResourceOwner } from './types/resource-owner';
-import type { SourceControlledFile } from './types/source-controlled-file';
 import { VariablesService } from '../variables/variables.service.ee';
 
 @Service()

packages/cli/src/environments.ee/source-control/source-control-helper.ee.ts

@@ -1,3 +1,4 @@
+import type { SourceControlledFile } from '@n8n/api-types';
 import { Container } from '@n8n/di';
 import { generateKeyPairSync } from 'crypto';
 import { constants as fsConstants, mkdirSync, accessSync } from 'fs';
@@ -16,7 +17,6 @@ import {
 } from './constants';
 import type { KeyPair } from './types/key-pair';
 import type { KeyPairType } from './types/key-pair-type';
-import type { SourceControlledFile } from './types/source-controlled-file';
 
 export function stringContainsExpression(testString: string): boolean {
 	return /^=.*\{\{.*\}\}/.test(testString);

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts

@@ -1,3 +1,4 @@
+import type { SourceControlledFile } from '@n8n/api-types';
 import { Container, Service } from '@n8n/di';
 // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
 import { In } from '@n8n/typeorm';
@@ -37,7 +38,6 @@ import { getCredentialExportPath, getWorkflowExportPath } from './source-control
 import type { ExportableCredential } from './types/exportable-credential';
 import type { ResourceOwner } from './types/resource-owner';
 import type { SourceControlWorkflowVersionId } from './types/source-control-workflow-version-id';
-import type { SourceControlledFile } from './types/source-controlled-file';
 import { VariablesService } from '../variables/variables.service.ee';
 
 @Service()

packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts

@@ -1,9 +1,12 @@
+import { PullWorkFolderRequestDto, PushWorkFolderRequestDto } from '@n8n/api-types';
+import type { SourceControlledFile } from '@n8n/api-types';
 import express from 'express';
 import type { PullResult } from 'simple-git';
 
-import { Get, Post, Patch, RestController, GlobalScope } from '@/decorators';
+import { Get, Post, Patch, RestController, GlobalScope, Body } from '@/decorators';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { EventService } from '@/events/event.service';
+import { AuthenticatedRequest } from '@/requests';
 
 import { SOURCE_CONTROL_DEFAULT_BRANCH } from './constants';
 import {
@@ -17,7 +20,6 @@ import type { ImportResult } from './types/import-result';
 import { SourceControlRequest } from './types/requests';
 import { SourceControlGetStatus } from './types/source-control-get-status';
 import type { SourceControlPreferences } from './types/source-control-preferences';
-import type { SourceControlledFile } from './types/source-controlled-file';
 
 @RestController('/source-control')
 export class SourceControlController {
@@ -164,19 +166,16 @@ export class SourceControlController {
 	@Post('/push-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] })
 	@GlobalScope('sourceControl:push')
 	async pushWorkfolder(
-		req: SourceControlRequest.PushWorkFolder,
+		req: AuthenticatedRequest,
 		res: express.Response,
+		@Body payload: PushWorkFolderRequestDto,
 	): Promise<SourceControlledFile[]> {
-		if (this.sourceControlPreferencesService.isBranchReadOnly()) {
-			throw new BadRequestError('Cannot push onto read-only branch.');
-		}
-
 		try {
 			await this.sourceControlService.setGitUserDetails(
 				`${req.user.firstName} ${req.user.lastName}`,
 				req.user.email,
 			);
-			const result = await this.sourceControlService.pushWorkfolder(req.body);
+			const result = await this.sourceControlService.pushWorkfolder(payload);
 			res.statusCode = result.statusCode;
 			return result.statusResult;
 		} catch (error) {
@@ -187,15 +186,12 @@ export class SourceControlController {
 	@Post('/pull-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] })
 	@GlobalScope('sourceControl:pull')
 	async pullWorkfolder(
-		req: SourceControlRequest.PullWorkFolder,
+		req: AuthenticatedRequest,
 		res: express.Response,
+		@Body payload: PullWorkFolderRequestDto,
 	): Promise<SourceControlledFile[] | ImportResult | PullResult | undefined> {
 		try {
-			const result = await this.sourceControlService.pullWorkfolder({
-				force: req.body.force,
-				variables: req.body.variables,
-				userId: req.user.id,
-			});
+			const result = await this.sourceControlService.pullWorkfolder(req.user.id, payload);
 			res.statusCode = result.statusCode;
 			return result.statusResult;
 		} catch (error) {