From 25a79ccf405787fd734ae34856865fbdfe4a0f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 10 Jan 2025 16:10:19 +0100 Subject: [PATCH 01/14] refactor(core): Use DI in source-control. add more tests (#12554) --- packages/cli/src/config/schema.ts | 9 - .../source-control-export.service.test.ts | 299 ++++++++++++++---- .../source-control-helper.ee.test.ts | 25 +- .../source-control-import.service.ee.test.ts | 180 +++++++++++ ...rce-control-preferences.service.ee.test.ts | 27 ++ .../__tests__/source-control.service.test.ts | 2 + .../source-control-export.service.ee.ts | 35 +- .../source-control-import.service.ee.ts | 73 ++--- .../source-control-preferences.service.ee.ts | 22 +- .../source-control/source-control.config.ts | 8 + .../variables/environment-helpers.ts | 27 -- .../variables/variables.service.ee.ts | 29 +- packages/cli/src/services/frontend.service.ts | 3 +- .../source-control-import.service.test.ts | 39 ++- ...rol.test.ts => source-control.api.test.ts} | 29 +- 15 files changed, 590 insertions(+), 217 deletions(-) create mode 100644 packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts create mode 100644 packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts create mode 100644 packages/cli/src/environments.ee/source-control/source-control.config.ts delete mode 100644 packages/cli/src/environments.ee/variables/environment-helpers.ts rename packages/cli/test/integration/environments/{source-control.test.ts => source-control.api.test.ts} (76%) diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index c3a604faa4..15c3a59969 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -349,15 +349,6 @@ export const schema = { }, }, - sourceControl: { - defaultKeyPairType: { - doc: 'Default SSH key type to use when generating SSH keys', - format: ['rsa', 'ed25519'] as const, - default: 'ed25519', - env: 'N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE', - }, - }, - workflowHistory: { enabled: { doc: 'Whether to save workflow history versions', diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts index 5f39e1ebf1..96f2f8ecdb 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts @@ -1,86 +1,261 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; -import mock from 'jest-mock-extended/lib/Mock'; +import { mock, captor } from 'jest-mock-extended'; import { Cipher, type InstanceSettings } from 'n8n-core'; -import { ApplicationError, deepCopy } from 'n8n-workflow'; +import fsp from 'node:fs/promises'; -import type { CredentialsEntity } from '@/databases/entities/credentials-entity'; import type { SharedCredentials } from '@/databases/entities/shared-credentials'; -import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; -import { mockInstance } from '@test/mocking'; +import type { SharedWorkflow } from '@/databases/entities/shared-workflow'; +import type { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; +import type { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; +import type { TagRepository } from '@/databases/repositories/tag.repository'; +import type { WorkflowTagMappingRepository } from '@/databases/repositories/workflow-tag-mapping.repository'; +import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import type { VariablesService } from '../../variables/variables.service.ee'; import { SourceControlExportService } from '../source-control-export.service.ee'; -// https://github.com/jestjs/jest/issues/4715 -function deepSpyOn(object: O, methodName: M) { - const spy = jest.fn(); - - const originalMethod = object[methodName]; - - if (typeof originalMethod !== 'function') { - throw new ApplicationError(`${methodName.toString()} is not a function`, { level: 'warning' }); - } - - object[methodName] = function (...args: unknown[]) { - const clonedArgs = deepCopy(args); - spy(...clonedArgs); - return originalMethod.apply(this, args); - } as O[M]; - - return spy; -} - describe('SourceControlExportService', () => { + const cipher = Container.get(Cipher); + const sharedCredentialsRepository = mock(); + const sharedWorkflowRepository = mock(); + const workflowRepository = mock(); + const tagRepository = mock(); + const workflowTagMappingRepository = mock(); + const variablesService = mock(); + const service = new SourceControlExportService( mock(), - mock(), - mock(), - mock({ n8nFolder: '' }), + variablesService, + tagRepository, + sharedCredentialsRepository, + sharedWorkflowRepository, + workflowRepository, + workflowTagMappingRepository, + mock({ n8nFolder: '/mock/n8n' }), ); - describe('exportCredentialsToWorkFolder', () => { - it('should export credentials to work folder', async () => { - /** - * Arrange - */ - // @ts-expect-error Private method - const replaceSpy = deepSpyOn(service, 'replaceCredentialData'); + const fsWriteFile = jest.spyOn(fsp, 'writeFile'); - mockInstance(SharedCredentialsRepository).findByCredentialIds.mockResolvedValue([ + beforeEach(() => jest.clearAllMocks()); + + describe('exportCredentialsToWorkFolder', () => { + const credentialData = { + authUrl: 'test', + accessTokenUrl: 'test', + clientId: 'test', + clientSecret: 'test', + oauthTokenData: { + access_token: 'test', + token_type: 'test', + expires_in: 123, + refresh_token: 'test', + }, + }; + + const mockCredentials = mock({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + data: cipher.encrypt(credentialData), + }); + + it('should export credentials to work folder', async () => { + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ mock({ - credentials: mock({ - data: Container.get(Cipher).encrypt( - JSON.stringify({ - authUrl: 'test', - accessTokenUrl: 'test', - clientId: 'test', - clientSecret: 'test', - oauthTokenData: { - access_token: 'test', - token_type: 'test', - expires_in: 123, - refresh_token: 'test', - }, - }), - ), + credentials: mockCredentials, + project: mock({ + type: 'personal', + projectRelations: [ + { + role: 'project:personalOwner', + user: mock({ email: 'user@example.com' }), + }, + ], }), }), ]); - /** - * Act - */ - await service.exportCredentialsToWorkFolder([mock()]); + // Act + const result = await service.exportCredentialsToWorkFolder([mock()]); - /** - * Assert - */ - expect(replaceSpy).toHaveBeenCalledWith({ - authUrl: 'test', - accessTokenUrl: 'test', - clientId: 'test', - clientSecret: 'test', + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/cred1.json', + dataCaptor, + ); + expect(JSON.parse(dataCaptor.value)).toEqual({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + data: { + authUrl: '', + accessTokenUrl: '', + clientId: '', + clientSecret: '', + }, + ownedBy: { + type: 'personal', + personalEmail: 'user@example.com', + }, }); }); + + it('should handle team project credentials', async () => { + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ + mock({ + credentials: mockCredentials, + project: mock({ + type: 'team', + id: 'team1', + name: 'Test Team', + }), + }), + ]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'cred1' }), + ]); + + // Assert + expect(result.count).toBe(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/cred1.json', + dataCaptor, + ); + expect(JSON.parse(dataCaptor.value)).toEqual({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + data: { + authUrl: '', + accessTokenUrl: '', + clientId: '', + clientSecret: '', + }, + ownedBy: { + type: 'team', + teamId: 'team1', + teamName: 'Test Team', + }, + }); + }); + + it('should handle missing credentials', async () => { + // Arrange + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'cred1' }), + ]); + + // Assert + expect(result.missingIds).toHaveLength(1); + expect(result.missingIds?.[0]).toBe('cred1'); + }); + }); + + describe('exportTagsToWorkFolder', () => { + it('should export tags to work folder', async () => { + // Arrange + tagRepository.find.mockResolvedValue([mock()]); + workflowTagMappingRepository.find.mockResolvedValue([mock()]); + + // Act + const result = await service.exportTagsToWorkFolder(); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + }); + + it('should not export empty tags', async () => { + // Arrange + tagRepository.find.mockResolvedValue([]); + + // Act + const result = await service.exportTagsToWorkFolder(); + + // Assert + expect(result.count).toBe(0); + expect(result.files).toHaveLength(0); + }); + }); + + describe('exportVariablesToWorkFolder', () => { + it('should export variables to work folder', async () => { + // Arrange + variablesService.getAllCached.mockResolvedValue([mock()]); + + // Act + const result = await service.exportVariablesToWorkFolder(); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + }); + + it('should not export empty variables', async () => { + // Arrange + variablesService.getAllCached.mockResolvedValue([]); + + // Act + const result = await service.exportVariablesToWorkFolder(); + + // Assert + expect(result.count).toBe(0); + expect(result.files).toHaveLength(0); + }); + }); + + describe('exportWorkflowsToWorkFolder', () => { + it('should export workflows to work folder', async () => { + // Arrange + workflowRepository.findByIds.mockResolvedValue([mock()]); + sharedWorkflowRepository.findByWorkflowIds.mockResolvedValue([ + mock({ + project: mock({ + type: 'personal', + projectRelations: [{ role: 'project:personalOwner', user: mock() }], + }), + workflow: mock(), + }), + ]); + + // Act + const result = await service.exportWorkflowsToWorkFolder([mock()]); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + }); + + it('should throw an error if workflow has no owner', async () => { + // Arrange + sharedWorkflowRepository.findByWorkflowIds.mockResolvedValue([ + mock({ + project: mock({ + type: 'personal', + projectRelations: [], + }), + workflow: mock({ + display: () => 'TestWorkflow', + }), + }), + ]); + + // Act & Assert + await expect(service.exportWorkflowsToWorkFolder([mock()])).rejects.toThrow( + 'Workflow TestWorkflow has no owner', + ); + }); }); }); 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 0395186a28..4824b85ba6 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 @@ -1,6 +1,7 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; import { constants as fsConstants, accessSync } from 'fs'; +import { mock } from 'jest-mock-extended'; import { InstanceSettings } from 'n8n-core'; import path from 'path'; @@ -16,10 +17,8 @@ import { getTrackingInformationFromPullResult, sourceControlFoldersExistCheck, } 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 { License } from '@/license'; -import { mockInstance } from '@test/mocking'; +import type { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; +import type { License } from '@/license'; const pushResult: SourceControlledFile[] = [ { @@ -151,12 +150,13 @@ const pullResult: SourceControlledFile[] = [ }, ]; -const license = mockInstance(License); +const license = mock(); +const sourceControlPreferencesService = mock(); beforeAll(async () => { jest.resetAllMocks(); license.isSourceControlLicensed.mockReturnValue(true); - Container.get(SourceControlPreferencesService).getPreferences = () => ({ + sourceControlPreferencesService.getPreferences.mockReturnValue({ branchName: 'main', connected: true, repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git', @@ -245,17 +245,4 @@ describe('Source Control', () => { workflowUpdates: 3, }); }); - - it('should class validate correct preferences', async () => { - const validPreferences: Partial = { - branchName: 'main', - repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git', - branchReadOnly: false, - branchColor: '#5296D6', - }; - const validationResult = await Container.get( - SourceControlPreferencesService, - ).validateSourceControlPreferences(validPreferences); - expect(validationResult).toBeTruthy(); - }); }); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts new file mode 100644 index 0000000000..fff60bd566 --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts @@ -0,0 +1,180 @@ +import * as fastGlob from 'fast-glob'; +import { mock } from 'jest-mock-extended'; +import { type InstanceSettings } from 'n8n-core'; +import fsp from 'node:fs/promises'; + +import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; +import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; + +import { SourceControlImportService } from '../source-control-import.service.ee'; + +jest.mock('fast-glob'); + +describe('SourceControlImportService', () => { + const workflowRepository = mock(); + const service = new SourceControlImportService( + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + workflowRepository, + mock(), + mock({ n8nFolder: '/mock/n8n' }), + ); + + const globMock = fastGlob.default as unknown as jest.Mock, string[]>; + const fsReadFile = jest.spyOn(fsp, 'readFile'); + + beforeEach(() => jest.clearAllMocks()); + + describe('getRemoteVersionIdsFromFiles', () => { + const mockWorkflowFile = '/mock/workflow1.json'; + it('should parse workflow files correctly', async () => { + globMock.mockResolvedValue([mockWorkflowFile]); + + const mockWorkflowData = { + id: 'workflow1', + versionId: 'v1', + name: 'Test Workflow', + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockWorkflowData)); + + const result = await service.getRemoteVersionIdsFromFiles(); + expect(fsReadFile).toHaveBeenCalledWith(mockWorkflowFile, { encoding: 'utf8' }); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'workflow1', + versionId: 'v1', + name: 'Test Workflow', + }), + ); + }); + + it('should filter out files without valid workflow data', async () => { + globMock.mockResolvedValue(['/mock/invalid.json']); + + fsReadFile.mockResolvedValue('{}'); + + const result = await service.getRemoteVersionIdsFromFiles(); + + expect(result).toHaveLength(0); + }); + }); + + describe('getRemoteCredentialsFromFiles', () => { + it('should parse credential files correctly', async () => { + globMock.mockResolvedValue(['/mock/credential1.json']); + + const mockCredentialData = { + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockCredentialData)); + + const result = await service.getRemoteCredentialsFromFiles(); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'cred1', + name: 'Test Credential', + type: 'oauth2', + }), + ); + }); + + it('should filter out files without valid credential data', async () => { + globMock.mockResolvedValue(['/mock/invalid.json']); + fsReadFile.mockResolvedValue('{}'); + + const result = await service.getRemoteCredentialsFromFiles(); + + expect(result).toHaveLength(0); + }); + }); + + describe('getRemoteVariablesFromFile', () => { + it('should parse variables file correctly', async () => { + globMock.mockResolvedValue(['/mock/variables.json']); + + const mockVariablesData = [ + { key: 'VAR1', value: 'value1' }, + { key: 'VAR2', value: 'value2' }, + ]; + + fsReadFile.mockResolvedValue(JSON.stringify(mockVariablesData)); + + const result = await service.getRemoteVariablesFromFile(); + + expect(result).toEqual(mockVariablesData); + }); + + it('should return empty array if no variables file found', async () => { + globMock.mockResolvedValue([]); + + const result = await service.getRemoteVariablesFromFile(); + + expect(result).toHaveLength(0); + }); + }); + + describe('getRemoteTagsAndMappingsFromFile', () => { + it('should parse tags and mappings file correctly', async () => { + globMock.mockResolvedValue(['/mock/tags.json']); + + const mockTagsData = { + tags: [{ id: 'tag1', name: 'Tag 1' }], + mappings: [{ workflowId: 'workflow1', tagId: 'tag1' }], + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockTagsData)); + + const result = await service.getRemoteTagsAndMappingsFromFile(); + + expect(result.tags).toEqual(mockTagsData.tags); + expect(result.mappings).toEqual(mockTagsData.mappings); + }); + + it('should return empty tags and mappings if no file found', async () => { + globMock.mockResolvedValue([]); + + const result = await service.getRemoteTagsAndMappingsFromFile(); + + expect(result.tags).toHaveLength(0); + expect(result.mappings).toHaveLength(0); + }); + }); + + describe('getLocalVersionIdsFromDb', () => { + const now = new Date(); + jest.useFakeTimers({ now }); + + it('should replace invalid updatedAt with current timestamp', async () => { + const mockWorkflows = [ + { + id: 'workflow1', + name: 'Test Workflow', + updatedAt: 'invalid-date', + }, + ] as unknown as WorkflowEntity[]; + + workflowRepository.find.mockResolvedValue(mockWorkflows); + + const result = await service.getLocalVersionIdsFromDb(); + + expect(result[0].updatedAt).toBe(now.toISOString()); + }); + }); +}); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts new file mode 100644 index 0000000000..d35c7ac9c7 --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts @@ -0,0 +1,27 @@ +import { mock } from 'jest-mock-extended'; +import type { InstanceSettings } from 'n8n-core'; + +import { SourceControlPreferencesService } from '../source-control-preferences.service.ee'; +import type { SourceControlPreferences } from '../types/source-control-preferences'; + +describe('SourceControlPreferencesService', () => { + const instanceSettings = mock({ n8nFolder: '' }); + const service = new SourceControlPreferencesService( + instanceSettings, + mock(), + mock(), + mock(), + mock(), + ); + + it('should class validate correct preferences', async () => { + const validPreferences: Partial = { + branchName: 'main', + repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git', + branchReadOnly: false, + branchColor: '#5296D6', + }; + const validationResult = await service.validateSourceControlPreferences(validPreferences); + expect(validationResult).toBeTruthy(); + }); +}); 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 9024a0a32c..56b58646c9 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 @@ -10,6 +10,8 @@ describe('SourceControlService', () => { Container.get(InstanceSettings), mock(), mock(), + mock(), + mock(), ); const sourceControlService = new SourceControlService( mock(), diff --git a/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts index d32758cd25..ec8e4efd22 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts @@ -1,5 +1,5 @@ import type { SourceControlledFile } from '@n8n/api-types'; -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; import { rmSync } from 'fs'; import { Credentials, InstanceSettings, Logger } from 'n8n-core'; import { ApplicationError, type ICredentialDataDecryptedObject } from 'n8n-workflow'; @@ -44,6 +44,10 @@ export class SourceControlExportService { private readonly logger: Logger, private readonly variablesService: VariablesService, private readonly tagRepository: TagRepository, + private readonly sharedCredentialsRepository: SharedCredentialsRepository, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly workflowTagMappingRepository: WorkflowTagMappingRepository, instanceSettings: InstanceSettings, ) { this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -106,17 +110,16 @@ export class SourceControlExportService { try { sourceControlFoldersExistCheck([this.workflowExportFolder]); const workflowIds = candidates.map((e) => e.id); - const sharedWorkflows = - await Container.get(SharedWorkflowRepository).findByWorkflowIds(workflowIds); - const workflows = await Container.get(WorkflowRepository).findByIds(workflowIds); + const sharedWorkflows = await this.sharedWorkflowRepository.findByWorkflowIds(workflowIds); + const workflows = await this.workflowRepository.findByIds(workflowIds); // determine owner of each workflow to be exported const owners: Record = {}; - sharedWorkflows.forEach((e) => { - const project = e.project; + sharedWorkflows.forEach((sharedWorkflow) => { + const project = sharedWorkflow.project; if (!project) { - throw new ApplicationError(`Workflow ${e.workflow.display()} has no owner`); + throw new ApplicationError(`Workflow ${sharedWorkflow.workflow.display()} has no owner`); } if (project.type === 'personal') { @@ -124,14 +127,16 @@ export class SourceControlExportService { (pr) => pr.role === 'project:personalOwner', ); if (!ownerRelation) { - throw new ApplicationError(`Workflow ${e.workflow.display()} has no owner`); + throw new ApplicationError( + `Workflow ${sharedWorkflow.workflow.display()} has no owner`, + ); } - owners[e.workflowId] = { + owners[sharedWorkflow.workflowId] = { type: 'personal', personalEmail: ownerRelation.user.email, }; } else if (project.type === 'team') { - owners[e.workflowId] = { + owners[sharedWorkflow.workflowId] = { type: 'team', teamId: project.id, teamName: project.name, @@ -156,6 +161,7 @@ export class SourceControlExportService { })), }; } catch (error) { + if (error instanceof ApplicationError) throw error; throw new ApplicationError('Failed to export workflows to work folder', { cause: error }); } } @@ -204,7 +210,7 @@ export class SourceControlExportService { files: [], }; } - const mappings = await Container.get(WorkflowTagMappingRepository).find(); + const mappings = await this.workflowTagMappingRepository.find(); const fileName = path.join(this.gitFolder, SOURCE_CONTROL_TAGS_EXPORT_FILE); await fsWriteFile( fileName, @@ -260,9 +266,10 @@ export class SourceControlExportService { try { sourceControlFoldersExistCheck([this.credentialExportFolder]); const credentialIds = candidates.map((e) => e.id); - const credentialsToBeExported = await Container.get( - SharedCredentialsRepository, - ).findByCredentialIds(credentialIds, 'credential:owner'); + const credentialsToBeExported = await this.sharedCredentialsRepository.findByCredentialIds( + credentialIds, + 'credential:owner', + ); let missingIds: string[] = []; if (credentialsToBeExported.length !== credentialIds.length) { const foundCredentialIds = credentialsToBeExported.map((e) => e.credentialsId); diff --git a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts index 21de490215..21640dad0e 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts @@ -1,5 +1,5 @@ import type { SourceControlledFile } from '@n8n/api-types'; -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; import glob from 'fast-glob'; @@ -53,7 +53,15 @@ export class SourceControlImportService { private readonly errorReporter: ErrorReporter, private readonly variablesService: VariablesService, private readonly activeWorkflowManager: ActiveWorkflowManager, + private readonly credentialsRepository: CredentialsRepository, + private readonly projectRepository: ProjectRepository, private readonly tagRepository: TagRepository, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly sharedCredentialsRepository: SharedCredentialsRepository, + private readonly userRepository: UserRepository, + private readonly variablesRepository: VariablesRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly workflowTagMappingRepository: WorkflowTagMappingRepository, instanceSettings: InstanceSettings, ) { this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -91,7 +99,7 @@ export class SourceControlImportService { } async getLocalVersionIdsFromDb(): Promise { - const localWorkflows = await Container.get(WorkflowRepository).find({ + const localWorkflows = await this.workflowRepository.find({ select: ['id', 'name', 'versionId', 'updatedAt'], }); return localWorkflows.map((local) => { @@ -146,7 +154,7 @@ export class SourceControlImportService { } async getLocalCredentialsFromDb(): Promise> { - const localCredentials = await Container.get(CredentialsRepository).find({ + const localCredentials = await this.credentialsRepository.find({ select: ['id', 'name', 'type'], }); return localCredentials.map((local) => ({ @@ -201,24 +209,22 @@ export class SourceControlImportService { const localTags = await this.tagRepository.find({ select: ['id', 'name'], }); - const localMappings = await Container.get(WorkflowTagMappingRepository).find({ + const localMappings = await this.workflowTagMappingRepository.find({ select: ['workflowId', 'tagId'], }); return { tags: localTags, mappings: localMappings }; } async importWorkflowFromWorkFolder(candidates: SourceControlledFile[], userId: string) { - const personalProject = - await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); + const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId); const workflowManager = this.activeWorkflowManager; const candidateIds = candidates.map((c) => c.id); - const existingWorkflows = await Container.get(WorkflowRepository).findByIds(candidateIds, { + const existingWorkflows = await this.workflowRepository.findByIds(candidateIds, { fields: ['id', 'name', 'versionId', 'active'], }); - const allSharedWorkflows = await Container.get(SharedWorkflowRepository).findWithFields( - candidateIds, - { select: ['workflowId', 'role', 'projectId'] }, - ); + const allSharedWorkflows = await this.sharedWorkflowRepository.findWithFields(candidateIds, { + select: ['workflowId', 'role', 'projectId'], + }); const importWorkflowsResult = []; // Due to SQLite concurrency issues, we cannot save all workflows at once @@ -235,9 +241,7 @@ export class SourceControlImportService { const existingWorkflow = existingWorkflows.find((e) => e.id === importedWorkflow.id); importedWorkflow.active = existingWorkflow?.active ?? false; this.logger.debug(`Updating workflow id ${importedWorkflow.id ?? 'new'}`); - const upsertResult = await Container.get(WorkflowRepository).upsert({ ...importedWorkflow }, [ - 'id', - ]); + const upsertResult = await this.workflowRepository.upsert({ ...importedWorkflow }, ['id']); if (upsertResult?.identifiers?.length !== 1) { throw new ApplicationError('Failed to upsert workflow', { extra: { workflowId: importedWorkflow.id ?? 'new' }, @@ -253,7 +257,7 @@ export class SourceControlImportService { ? await this.findOrCreateOwnerProject(importedWorkflow.owner) : null; - await Container.get(SharedWorkflowRepository).upsert( + await this.sharedWorkflowRepository.upsert( { workflowId: importedWorkflow.id, projectId: remoteOwnerProject?.id ?? personalProject.id, @@ -276,7 +280,7 @@ export class SourceControlImportService { const error = ensureError(e); this.logger.error(`Failed to activate workflow ${existingWorkflow.id}`, { error }); } finally { - await Container.get(WorkflowRepository).update( + await this.workflowRepository.update( { id: existingWorkflow.id }, { versionId: importedWorkflow.versionId }, ); @@ -295,16 +299,15 @@ export class SourceControlImportService { } async importCredentialsFromWorkFolder(candidates: SourceControlledFile[], userId: string) { - const personalProject = - await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); + const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId); const candidateIds = candidates.map((c) => c.id); - const existingCredentials = await Container.get(CredentialsRepository).find({ + const existingCredentials = await this.credentialsRepository.find({ where: { id: In(candidateIds), }, select: ['id', 'name', 'type', 'data'], }); - const existingSharedCredentials = await Container.get(SharedCredentialsRepository).find({ + const existingSharedCredentials = await this.sharedCredentialsRepository.find({ select: ['credentialsId', 'role'], where: { credentialsId: In(candidateIds), @@ -336,7 +339,7 @@ export class SourceControlImportService { } this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`); - await Container.get(CredentialsRepository).upsert(newCredentialObject, ['id']); + await this.credentialsRepository.upsert(newCredentialObject, ['id']); const isOwnedLocally = existingSharedCredentials.some( (c) => c.credentialsId === credential.id && c.role === 'credential:owner', @@ -352,7 +355,7 @@ export class SourceControlImportService { newSharedCredential.projectId = remoteOwnerProject?.id ?? personalProject.id; newSharedCredential.role = 'credential:owner'; - await Container.get(SharedCredentialsRepository).upsert({ ...newSharedCredential }, [ + await this.sharedCredentialsRepository.upsert({ ...newSharedCredential }, [ 'credentialsId', 'projectId', ]); @@ -388,7 +391,7 @@ export class SourceControlImportService { const existingWorkflowIds = new Set( ( - await Container.get(WorkflowRepository).find({ + await this.workflowRepository.find({ select: ['id'], }) ).map((e) => e.id), @@ -417,7 +420,7 @@ export class SourceControlImportService { await Promise.all( mappedTags.mappings.map(async (mapping) => { if (!existingWorkflowIds.has(String(mapping.workflowId))) return; - await Container.get(WorkflowTagMappingRepository).upsert( + await this.workflowTagMappingRepository.upsert( { tagId: String(mapping.tagId), workflowId: String(mapping.workflowId) }, { skipUpdateIfNoValuesChanged: true, @@ -464,12 +467,12 @@ export class SourceControlImportService { overriddenKeys.splice(overriddenKeys.indexOf(variable.key), 1); } try { - await Container.get(VariablesRepository).upsert({ ...variable }, ['id']); + await this.variablesRepository.upsert({ ...variable }, ['id']); } catch (errorUpsert) { if (isUniqueConstraintError(errorUpsert as Error)) { this.logger.debug(`Variable ${variable.key} already exists, updating instead`); try { - await Container.get(VariablesRepository).update({ key: variable.key }, { ...variable }); + await this.variablesRepository.update({ key: variable.key }, { ...variable }); } catch (errorUpdate) { this.logger.debug(`Failed to update variable ${variable.key}, skipping`); this.logger.debug((errorUpdate as Error).message); @@ -484,11 +487,11 @@ export class SourceControlImportService { if (overriddenKeys.length > 0 && valueOverrides) { for (const key of overriddenKeys) { result.imported.push(key); - const newVariable = Container.get(VariablesRepository).create({ + const newVariable = this.variablesRepository.create({ key, value: valueOverrides[key], }); - await Container.get(VariablesRepository).save(newVariable, { transaction: false }); + await this.variablesRepository.save(newVariable, { transaction: false }); } } @@ -498,32 +501,30 @@ export class SourceControlImportService { } private async findOrCreateOwnerProject(owner: ResourceOwner): Promise { - const projectRepository = Container.get(ProjectRepository); - const userRepository = Container.get(UserRepository); if (typeof owner === 'string' || owner.type === 'personal') { const email = typeof owner === 'string' ? owner : owner.personalEmail; - const user = await userRepository.findOne({ + const user = await this.userRepository.findOne({ where: { email }, }); if (!user) { return null; } - return await projectRepository.getPersonalProjectForUserOrFail(user.id); + return await this.projectRepository.getPersonalProjectForUserOrFail(user.id); } else if (owner.type === 'team') { - let teamProject = await projectRepository.findOne({ + let teamProject = await this.projectRepository.findOne({ where: { id: owner.teamId }, }); if (!teamProject) { try { - teamProject = await projectRepository.save( - projectRepository.create({ + teamProject = await this.projectRepository.save( + this.projectRepository.create({ id: owner.teamId, name: owner.teamName, type: 'team', }), ); } catch (e) { - teamProject = await projectRepository.findOne({ + teamProject = await this.projectRepository.findOne({ where: { id: owner.teamId }, }); if (!teamProject) { diff --git a/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts index e530d9d530..d424a844b2 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts @@ -1,4 +1,4 @@ -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; import type { ValidationError } from 'class-validator'; import { validate } from 'class-validator'; import { rm as fsRm } from 'fs/promises'; @@ -7,7 +7,6 @@ import { ApplicationError, jsonParse } from 'n8n-workflow'; import { writeFile, chmod, readFile } from 'node:fs/promises'; import path from 'path'; -import config from '@/config'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import { @@ -17,6 +16,7 @@ import { SOURCE_CONTROL_PREFERENCES_DB_KEY, } from './constants'; import { generateSshKeyPair, isSourceControlLicensed } from './source-control-helper.ee'; +import { SourceControlConfig } from './source-control.config'; import type { KeyPairType } from './types/key-pair-type'; import { SourceControlPreferences } from './types/source-control-preferences'; @@ -34,6 +34,8 @@ export class SourceControlPreferencesService { private readonly instanceSettings: InstanceSettings, private readonly logger: Logger, private readonly cipher: Cipher, + private readonly settingsRepository: SettingsRepository, + private readonly sourceControlConfig: SourceControlConfig, ) { this.sshFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_SSH_FOLDER); this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -64,9 +66,7 @@ export class SourceControlPreferencesService { } private async getKeyPairFromDatabase() { - const dbSetting = await Container.get(SettingsRepository).findByKey( - 'features.sourceControl.sshKeys', - ); + const dbSetting = await this.settingsRepository.findByKey('features.sourceControl.sshKeys'); if (!dbSetting?.value) return null; @@ -120,7 +120,7 @@ export class SourceControlPreferencesService { async deleteKeyPair() { try { await fsRm(this.sshFolder, { recursive: true }); - await Container.get(SettingsRepository).delete({ key: 'features.sourceControl.sshKeys' }); + await this.settingsRepository.delete({ key: 'features.sourceControl.sshKeys' }); } catch (e) { const error = e instanceof Error ? e : new Error(`${e}`); this.logger.error(`Failed to delete SSH key pair: ${error.message}`); @@ -133,14 +133,12 @@ export class SourceControlPreferencesService { async generateAndSaveKeyPair(keyPairType?: KeyPairType): Promise { if (!keyPairType) { keyPairType = - this.getPreferences().keyGeneratorType ?? - (config.get('sourceControl.defaultKeyPairType') as KeyPairType) ?? - 'ed25519'; + this.getPreferences().keyGeneratorType ?? this.sourceControlConfig.defaultKeyPairType; } const keyPair = await generateSshKeyPair(keyPairType); try { - await Container.get(SettingsRepository).save({ + await this.settingsRepository.save({ key: 'features.sourceControl.sshKeys', value: JSON.stringify({ encryptedPrivateKey: this.cipher.encrypt(keyPair.privateKey), @@ -211,7 +209,7 @@ export class SourceControlPreferencesService { if (saveToDb) { const settingsValue = JSON.stringify(this._sourceControlPreferences); try { - await Container.get(SettingsRepository).save( + await this.settingsRepository.save( { key: SOURCE_CONTROL_PREFERENCES_DB_KEY, value: settingsValue, @@ -229,7 +227,7 @@ export class SourceControlPreferencesService { async loadFromDbAndApplySourceControlPreferences(): Promise< SourceControlPreferences | undefined > { - const loadedPreferences = await Container.get(SettingsRepository).findOne({ + const loadedPreferences = await this.settingsRepository.findOne({ where: { key: SOURCE_CONTROL_PREFERENCES_DB_KEY }, }); if (loadedPreferences) { diff --git a/packages/cli/src/environments.ee/source-control/source-control.config.ts b/packages/cli/src/environments.ee/source-control/source-control.config.ts new file mode 100644 index 0000000000..04d12dce4f --- /dev/null +++ b/packages/cli/src/environments.ee/source-control/source-control.config.ts @@ -0,0 +1,8 @@ +import { Config, Env } from '@n8n/config'; + +@Config +export class SourceControlConfig { + /** Default SSH key type to use when generating SSH keys. */ + @Env('N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE') + defaultKeyPairType: 'ed25519' | 'rsa' = 'ed25519'; +} diff --git a/packages/cli/src/environments.ee/variables/environment-helpers.ts b/packages/cli/src/environments.ee/variables/environment-helpers.ts deleted file mode 100644 index dd9a17c95b..0000000000 --- a/packages/cli/src/environments.ee/variables/environment-helpers.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Container } from '@n8n/di'; - -import { License } from '@/license'; - -export function isVariablesEnabled(): boolean { - const license = Container.get(License); - return license.isVariablesEnabled(); -} - -export function canCreateNewVariable(variableCount: number): boolean { - if (!isVariablesEnabled()) { - return false; - } - const license = Container.get(License); - // This defaults to -1 which is what we want if we've enabled - // variables via the config - const limit = license.getVariablesLimit(); - if (limit === -1) { - return true; - } - return limit > variableCount; -} - -export function getVariablesLimit(): number { - const license = Container.get(License); - return license.getVariablesLimit(); -} diff --git a/packages/cli/src/environments.ee/variables/variables.service.ee.ts b/packages/cli/src/environments.ee/variables/variables.service.ee.ts index ebb134efd3..f59880fe60 100644 --- a/packages/cli/src/environments.ee/variables/variables.service.ee.ts +++ b/packages/cli/src/environments.ee/variables/variables.service.ee.ts @@ -1,4 +1,4 @@ -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; import type { Variables } from '@/databases/entities/variables'; import { VariablesRepository } from '@/databases/repositories/variables.repository'; @@ -6,23 +6,21 @@ import { generateNanoId } from '@/databases/utils/generators'; import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error'; import { VariableValidationError } from '@/errors/variable-validation.error'; import { EventService } from '@/events/event.service'; +import { License } from '@/license'; import { CacheService } from '@/services/cache/cache.service'; -import { canCreateNewVariable } from './environment-helpers'; - @Service() export class VariablesService { constructor( - protected cacheService: CacheService, - protected variablesRepository: VariablesRepository, + private readonly cacheService: CacheService, + private readonly variablesRepository: VariablesRepository, private readonly eventService: EventService, + private readonly license: License, ) {} async getAllCached(state?: 'empty'): Promise { let variables = await this.cacheService.get('variables', { - async refreshFn() { - return await Container.get(VariablesService).findAll(); - }, + refreshFn: async () => await this.findAll(), }); if (variables === undefined) { @@ -77,7 +75,7 @@ export class VariablesService { } async create(variable: Omit): Promise { - if (!canCreateNewVariable(await this.getCount())) { + if (!this.canCreateNewVariable(await this.getCount())) { throw new VariableCountLimitReachedError('Variables limit reached'); } this.validateVariable(variable); @@ -100,4 +98,17 @@ export class VariablesService { await this.updateCache(); return (await this.getCached(id))!; } + + private canCreateNewVariable(variableCount: number): boolean { + if (!this.license.isVariablesEnabled()) { + return false; + } + // This defaults to -1 which is what we want if we've enabled + // variables via the config + const limit = this.license.getVariablesLimit(); + if (limit === -1) { + return true; + } + return limit > variableCount; + } } diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 33da795f27..954c3e9fc3 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -12,7 +12,6 @@ import config from '@/config'; import { inE2ETests, LICENSE_FEATURES, N8N_VERSION } from '@/constants'; import { CredentialTypes } from '@/credential-types'; import { CredentialsOverwrites } from '@/credentials-overwrites'; -import { getVariablesLimit } from '@/environments.ee/variables/environment-helpers'; import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee'; import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; @@ -326,7 +325,7 @@ export class FrontendService { } if (this.license.isVariablesEnabled()) { - this.settings.variables.limit = getVariablesLimit(); + this.settings.variables.limit = this.license.getVariablesLimit(); } if (this.license.isWorkflowHistoryLicensed() && config.getEnv('workflowHistory.enabled')) { diff --git a/packages/cli/test/integration/environments/source-control-import.service.test.ts b/packages/cli/test/integration/environments/source-control-import.service.test.ts index 17f8654aae..8b774f8fa9 100644 --- a/packages/cli/test/integration/environments/source-control-import.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-import.service.test.ts @@ -10,6 +10,7 @@ import fsp from 'node:fs/promises'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { ProjectRepository } from '@/databases/repositories/project.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; +import { UserRepository } from '@/databases/repositories/user.repository'; import { SourceControlImportService } from '@/environments.ee/source-control/source-control-import.service.ee'; import type { ExportableCredential } from '@/environments.ee/source-control/types/exportable-credential'; @@ -21,20 +22,36 @@ import { randomCredentialPayload } from '../shared/random'; import * as testDb from '../shared/test-db'; describe('SourceControlImportService', () => { + let credentialsRepository: CredentialsRepository; + let projectRepository: ProjectRepository; + let sharedCredentialsRepository: SharedCredentialsRepository; + let userRepository: UserRepository; let service: SourceControlImportService; const cipher = mockInstance(Cipher); beforeAll(async () => { + await testDb.init(); + + credentialsRepository = Container.get(CredentialsRepository); + projectRepository = Container.get(ProjectRepository); + sharedCredentialsRepository = Container.get(SharedCredentialsRepository); + userRepository = Container.get(UserRepository); service = new SourceControlImportService( mock(), mock(), mock(), mock(), + credentialsRepository, + projectRepository, + mock(), + mock(), + sharedCredentialsRepository, + userRepository, + mock(), + mock(), mock(), mock({ n8nFolder: '/some-path' }), ); - - await testDb.init(); }); afterEach(async () => { @@ -75,7 +92,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(member); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -112,7 +129,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(importingUser); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -149,7 +166,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(importingUser); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -190,7 +207,7 @@ describe('SourceControlImportService', () => { const personalProject = await getPersonalProject(importingUser); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: personalProject.id, role: 'credential:owner', @@ -223,7 +240,7 @@ describe('SourceControlImportService', () => { cipher.encrypt.mockReturnValue('some-encrypted-data'); { - const project = await Container.get(ProjectRepository).findOne({ + const project = await projectRepository.findOne({ where: [ { id: '1234-asdf', @@ -241,7 +258,7 @@ describe('SourceControlImportService', () => { importingUser.id, ); - const sharing = await Container.get(SharedCredentialsRepository).findOne({ + const sharing = await sharedCredentialsRepository.findOne({ where: { credentialsId: CREDENTIAL_ID, role: 'credential:owner', @@ -288,7 +305,7 @@ describe('SourceControlImportService', () => { importingUser.id, ); - const sharing = await Container.get(SharedCredentialsRepository).findOneBy({ + const sharing = await sharedCredentialsRepository.findOneBy({ credentialsId: CREDENTIAL_ID, projectId: project.id, role: 'credential:owner', @@ -332,7 +349,7 @@ describe('SourceControlImportService', () => { ); await expect( - Container.get(SharedCredentialsRepository).findBy({ + sharedCredentialsRepository.findBy({ credentialsId: credential.id, }), ).resolves.toMatchObject([ @@ -342,7 +359,7 @@ describe('SourceControlImportService', () => { }, ]); await expect( - Container.get(CredentialsRepository).findBy({ + credentialsRepository.findBy({ id: credential.id, }), ).resolves.toMatchObject([ diff --git a/packages/cli/test/integration/environments/source-control.test.ts b/packages/cli/test/integration/environments/source-control.api.test.ts similarity index 76% rename from packages/cli/test/integration/environments/source-control.test.ts rename to packages/cli/test/integration/environments/source-control.api.test.ts index 2dedd53287..6ad771493b 100644 --- a/packages/cli/test/integration/environments/source-control.test.ts +++ b/packages/cli/test/integration/environments/source-control.api.test.ts @@ -1,7 +1,6 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; -import config from '@/config'; import type { User } from '@/databases/entities/user'; import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee'; @@ -21,11 +20,17 @@ const testServer = utils.setupTestServer({ enabledFeatures: ['feat:sourceControl', 'feat:sharing'], }); +let sourceControlPreferencesService: SourceControlPreferencesService; + beforeAll(async () => { owner = await createUser({ role: 'global:owner' }); authOwnerAgent = testServer.authAgentFor(owner); - Container.get(SourceControlPreferencesService).isSourceControlConnected = () => true; + sourceControlPreferencesService = Container.get(SourceControlPreferencesService); + await sourceControlPreferencesService.setPreferences({ + connected: true, + keyGeneratorType: 'rsa', + }); }); describe('GET /sourceControl/preferences', () => { @@ -65,19 +70,11 @@ describe('GET /sourceControl/preferences', () => { }); test('refreshing key pairsshould return new rsa key', async () => { - config.set('sourceControl.defaultKeyPairType', 'rsa'); - await authOwnerAgent - .post('/source-control/generate-key-pair') - .send() - .expect(200) - .expect((res) => { - expect( - Container.get(SourceControlPreferencesService).getPreferences().keyGeneratorType, - ).toBe('rsa'); - expect(res.body.data).toHaveProperty('publicKey'); - expect(res.body.data).toHaveProperty('keyGeneratorType'); - expect(res.body.data.keyGeneratorType).toBe('rsa'); - expect(res.body.data.publicKey).toContain('ssh-rsa'); - }); + const res = await authOwnerAgent.post('/source-control/generate-key-pair').send().expect(200); + + expect(res.body.data).toHaveProperty('publicKey'); + expect(res.body.data).toHaveProperty('keyGeneratorType'); + expect(res.body.data.keyGeneratorType).toBe('rsa'); + expect(res.body.data.publicKey).toContain('ssh-rsa'); }); }); From 562506e92aeb26423145801bff80037e5ce2ac46 Mon Sep 17 00:00:00 2001 From: Dana <152518854+dana-gill@users.noreply.github.com> Date: Fri, 10 Jan 2025 16:11:19 +0100 Subject: [PATCH 02/14] fix(core): Validate values which are intentionally 0 (#12382) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ --- .../validateValueAgainstSchema.test.ts | 63 +++++++++++++++++++ .../utils/validateValueAgainstSchema.ts | 2 +- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts b/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts index e09299a457..d4b96fa41d 100644 --- a/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts +++ b/packages/core/src/node-execution-context/utils/__tests__/validateValueAgainstSchema.test.ts @@ -246,4 +246,67 @@ describe('validateValueAgainstSchema', () => { // value should be type number expect(typeof result).toEqual('number'); }); + + describe('when the mode is in Fixed mode, and the node is a resource mapper', () => { + const nodeType = { + description: { + properties: [ + { + name: 'operation', + type: 'resourceMapper', + typeOptions: { + resourceMapper: { + mode: 'add', + }, + }, + }, + ], + }, + } as unknown as INodeType; + + const node = { + parameters: { + operation: { + schema: [ + { id: 'num', type: 'number', required: true }, + { id: 'str', type: 'string', required: true }, + { id: 'obj', type: 'object', required: true }, + { id: 'arr', type: 'array', required: true }, + ], + attemptToConvertTypes: true, + mappingMode: '', + value: '', + }, + }, + } as unknown as INode; + + const parameterName = 'operation.value'; + + describe('should correctly validate values for', () => { + test.each([ + { num: 0 }, + { num: 23 }, + { num: -0 }, + { num: -Infinity }, + { num: Infinity }, + { str: '' }, + { str: ' ' }, + { str: 'hello' }, + { arr: [] }, + { obj: {} }, + ])('%s', (value) => { + expect(() => + validateValueAgainstSchema(node, nodeType, value, parameterName, 0, 0), + ).not.toThrow(); + }); + }); + + describe('should throw an error for', () => { + test.each([{ num: NaN }, { num: undefined }, { num: null }])('%s', (value) => { + expect(() => + validateValueAgainstSchema(node, nodeType, value, parameterName, 0, 0), + ).toThrow(); + }); + }); + }); }); diff --git a/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts b/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts index d058c50a52..a4c1b211e7 100644 --- a/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts +++ b/packages/core/src/node-execution-context/utils/validateValueAgainstSchema.ts @@ -44,7 +44,7 @@ const validateResourceMapperValue = ( !skipRequiredCheck && schemaEntry?.required === true && schemaEntry.type !== 'boolean' && - !resolvedValue + (resolvedValue === undefined || resolvedValue === null) ) { return { valid: false, From ba6a2f82d23ad7f06161f1d0bdc3f4a267745eb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 10 Jan 2025 17:21:26 +0100 Subject: [PATCH 03/14] docs: Fix the docs for starting n8n with `--tunnel` option (no-changelog) (#12512) --- docker/images/n8n/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/images/n8n/README.md b/docker/images/n8n/README.md index f7b45f9467..73dbb7557b 100644 --- a/docker/images/n8n/README.md +++ b/docker/images/n8n/README.md @@ -73,7 +73,7 @@ docker run -it --rm \ -p 5678:5678 \ -v ~/.n8n:/home/node/.n8n \ docker.n8n.io/n8nio/n8n \ - n8n start --tunnel + start --tunnel ``` ## Persist data From c6b491cdbbae6a3a8e5d45f156c11b576291c7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 10 Jan 2025 17:35:03 +0100 Subject: [PATCH 04/14] ci: Stop publishing custom images to docker hub by default (#12516) --- .github/workflows/docker-base-image.yml | 12 +-- .github/workflows/docker-images-benchmark.yml | 10 ++- .github/workflows/docker-images-custom.yml | 83 ++++++++++++++++++ .github/workflows/docker-images-nightly.yml | 86 ++++--------------- .github/workflows/release-publish.yml | 12 +-- .github/workflows/release-push-to-channel.yml | 4 +- 6 files changed, 123 insertions(+), 84 deletions(-) create mode 100644 .github/workflows/docker-images-custom.yml diff --git a/.github/workflows/docker-base-image.yml b/.github/workflows/docker-base-image.yml index c32160e763..c2b5c9b6e6 100644 --- a/.github/workflows/docker-base-image.yml +++ b/.github/workflows/docker-base-image.yml @@ -20,26 +20,28 @@ jobs: - uses: actions/checkout@v4.1.1 - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 - name: Login to GitHub Container Registry - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Login to DockerHub - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Build - uses: docker/build-push-action@v5.1.0 + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: . file: ./docker/images/n8n-base/Dockerfile diff --git a/.github/workflows/docker-images-benchmark.yml b/.github/workflows/docker-images-benchmark.yml index b0aa6e997d..d4bf2f98a0 100644 --- a/.github/workflows/docker-images-benchmark.yml +++ b/.github/workflows/docker-images-benchmark.yml @@ -19,20 +19,22 @@ jobs: - uses: actions/checkout@v4.1.1 - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 - name: Login to GitHub Container Registry - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build - uses: docker/build-push-action@v5.1.0 + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: . file: ./packages/@n8n/benchmark/Dockerfile diff --git a/.github/workflows/docker-images-custom.yml b/.github/workflows/docker-images-custom.yml new file mode 100644 index 0000000000..dfc9d77a03 --- /dev/null +++ b/.github/workflows/docker-images-custom.yml @@ -0,0 +1,83 @@ +name: Docker Custom Image CI +run-name: Build ${{ inputs.branch }} - ${{ inputs.user }} + +on: + workflow_dispatch: + inputs: + branch: + description: 'GitHub branch to create image off.' + required: true + tag: + description: 'Name of the docker tag to create.' + required: true + merge-master: + description: 'Merge with master.' + type: boolean + required: true + default: false + user: + description: '' + required: false + default: 'none' + start-url: + description: 'URL to call after workflow is kicked off.' + required: false + default: '' + success-url: + description: 'URL to call after Docker Image got built successfully.' + required: false + default: '' + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - name: Call Start URL - optionally + if: ${{ github.event.inputs.start-url != '' }} + run: curl -v -X POST -d 'url=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' ${{github.event.inputs.start-url}} || echo "" + shell: bash + + - name: Checkout + uses: actions/checkout@v4.1.1 + with: + ref: ${{ github.event.inputs.branch }} + + - name: Merge Master - optionally + if: github.event.inputs.merge-master + run: git remote add upstream https://github.com/n8n-io/n8n.git -f; git merge upstream/master --allow-unrelated-histories || echo "" + shell: bash + + - name: Set up QEMU + uses: docker/setup-qemu-action@v3.3.0 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3.8.0 + + - name: Login to GHCR + uses: docker/login-action@v3.3.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Build and push image to GHCR + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false + with: + context: . + file: ./docker/images/n8n-custom/Dockerfile + build-args: | + N8N_RELEASE_TYPE=development + platforms: linux/amd64 + provenance: false + push: true + cache-from: type=gha + cache-to: type=gha,mode=max + tags: ghcr.io/${{ github.repository_owner }}/n8n:${{ inputs.tag }} + + - name: Call Success URL - optionally + if: ${{ github.event.inputs.success-url != '' }} + run: curl -v ${{github.event.inputs.success-url}} || echo "" + shell: bash diff --git a/.github/workflows/docker-images-nightly.yml b/.github/workflows/docker-images-nightly.yml index b6831b6399..c924f8b2c1 100644 --- a/.github/workflows/docker-images-nightly.yml +++ b/.github/workflows/docker-images-nightly.yml @@ -1,74 +1,42 @@ name: Docker Nightly Image CI -run-name: Build ${{ inputs.branch }} - ${{ inputs.user }} on: schedule: - - cron: '0 1 * * *' + - cron: '0 0 * * *' workflow_dispatch: - inputs: - branch: - description: 'GitHub branch to create image off.' - required: true - default: 'master' - tag: - description: 'Name of the docker tag to create.' - required: true - default: 'nightly' - merge-master: - description: 'Merge with master.' - type: boolean - required: true - default: false - user: - description: '' - required: false - default: 'schedule' - start-url: - description: 'URL to call after workflow is kicked off.' - required: false - default: '' - success-url: - description: 'URL to call after Docker Image got built successfully.' - required: false - default: '' - -env: - N8N_TAG: ${{ inputs.tag || 'nightly' }} jobs: build: runs-on: ubuntu-latest - steps: - - name: Call Start URL - optionally - run: | - [[ "${{github.event.inputs.start-url}}" != "" ]] && curl -v -X POST -d 'url=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' ${{github.event.inputs.start-url}} || echo "" - shell: bash - - name: Checkout uses: actions/checkout@v4.1.1 with: - ref: ${{ github.event.inputs.branch || 'master' }} + ref: master - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 + + - name: Login to GHCR + uses: docker/login-action@v3.3.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} - name: Login to DockerHub - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - - name: Merge Master - optionally - run: | - [[ "${{github.event.inputs.merge-master}}" == "true" ]] && git remote add upstream https://github.com/n8n-io/n8n.git -f; git merge upstream/master --allow-unrelated-histories || echo "" - shell: bash - - - name: Build and push to DockerHub - uses: docker/build-push-action@v5.1.0 + - name: Build and push image to GHCR and DockerHub + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: . file: ./docker/images/n8n-custom/Dockerfile @@ -79,24 +47,6 @@ jobs: push: true cache-from: type=gha cache-to: type=gha,mode=max - tags: ${{ secrets.DOCKER_USERNAME }}/n8n:${{ env.N8N_TAG }} - - - name: Login to GitHub Container Registry - if: env.N8N_TAG == 'nightly' - uses: docker/login-action@v3.0.0 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Push image to GHCR - if: env.N8N_TAG == 'nightly' - run: | - docker buildx imagetools create \ - --tag ghcr.io/${{ github.repository_owner }}/n8n:nightly \ + tags: | + ghcr.io/${{ github.repository_owner }}/n8n:nightly ${{ secrets.DOCKER_USERNAME }}/n8n:nightly - - - name: Call Success URL - optionally - run: | - [[ "${{github.event.inputs.success-url}}" != "" ]] && curl -v ${{github.event.inputs.success-url}} || echo "" - shell: bash diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index 9196a7fccb..b95350a577 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -73,26 +73,28 @@ jobs: fetch-depth: 0 - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.3.0 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3.0.0 + uses: docker/setup-buildx-action@v3.8.0 - name: Login to GitHub Container Registry - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Login to DockerHub - uses: docker/login-action@v3.0.0 + uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Build - uses: docker/build-push-action@v5.1.0 + uses: docker/build-push-action@v6.11.0 + env: + DOCKER_BUILD_SUMMARY: false with: context: ./docker/images/n8n build-args: | diff --git a/.github/workflows/release-push-to-channel.yml b/.github/workflows/release-push-to-channel.yml index 3eda6d4ebb..9cb3a99b63 100644 --- a/.github/workflows/release-push-to-channel.yml +++ b/.github/workflows/release-push-to-channel.yml @@ -34,7 +34,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - uses: docker/login-action@v3.0.0 + - uses: docker/login-action@v3.3.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} @@ -46,7 +46,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - uses: docker/login-action@v3.0.0 + - uses: docker/login-action@v3.3.0 with: registry: ghcr.io username: ${{ github.actor }} From 3ec5b2850c47057032e61c2acdbdfc1dcdd931f7 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Mon, 13 Jan 2025 09:05:35 +0100 Subject: [PATCH 05/14] fix(editor): Don't show toolsUnused notice if run had errors (#12529) --- .../editor-ui/src/components/OutputPanel.vue | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/editor-ui/src/components/OutputPanel.vue b/packages/editor-ui/src/components/OutputPanel.vue index aff304a3cf..ac13a67c95 100644 --- a/packages/editor-ui/src/components/OutputPanel.vue +++ b/packages/editor-ui/src/components/OutputPanel.vue @@ -4,7 +4,6 @@ import { NodeConnectionType, type IRunData, type IRunExecutionData, - type NodeError, type Workflow, } from 'n8n-workflow'; import RunData from './RunData.vue'; @@ -120,14 +119,17 @@ const hasAiMetadata = computed(() => { return false; }); +const hasError = computed(() => + Boolean( + workflowRunData.value && + node.value && + workflowRunData.value[node.value.name]?.[props.runIndex]?.error, + ), +); + // Determine the initial output mode to logs if the node has an error and the logs are available const defaultOutputMode = computed(() => { - const hasError = - workflowRunData.value && - node.value && - (workflowRunData.value[node.value.name]?.[props.runIndex]?.error as NodeError); - - return Boolean(hasError) && hasAiMetadata.value ? OUTPUT_TYPE.LOGS : OUTPUT_TYPE.REGULAR; + return hasError.value && hasAiMetadata.value ? OUTPUT_TYPE.LOGS : OUTPUT_TYPE.REGULAR; }); const isNodeRunning = computed(() => { @@ -216,7 +218,7 @@ const canPinData = computed(() => { }); const allToolsWereUnusedNotice = computed(() => { - if (!node.value || runsCount.value === 0) return undefined; + if (!node.value || runsCount.value === 0 || hasError.value) return undefined; // With pinned data there's no clear correct answer for whether // we should use historic or current parents, so we don't show the notice, From 865fc21276727e8d88ccee0355147904b81c4421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20G=C3=B3mez=20Morales?= Date: Mon, 13 Jan 2025 10:24:51 +0100 Subject: [PATCH 06/14] fix(editor): Update filter and feedback for source control (#12504) --- packages/editor-ui/src/api/credentials.ts | 1 + packages/editor-ui/src/api/sourceControl.ts | 2 +- .../src/components/CredentialCard.vue | 8 +- .../MainSidebarSourceControl.test.ts | 129 ++++++++++++- .../components/MainSidebarSourceControl.vue | 123 +++++++++--- .../forms/ResourceFiltersDropdown.vue | 13 +- .../layouts/ResourcesListLayout.vue | 20 +- .../src/plugins/i18n/locales/en.json | 7 + .../src/views/CredentialsView.test.ts | 177 ++++++++++++++++-- .../editor-ui/src/views/CredentialsView.vue | 72 +++++-- 10 files changed, 474 insertions(+), 78 deletions(-) diff --git a/packages/editor-ui/src/api/credentials.ts b/packages/editor-ui/src/api/credentials.ts index 6ddb37c16a..18bb7b3f3d 100644 --- a/packages/editor-ui/src/api/credentials.ts +++ b/packages/editor-ui/src/api/credentials.ts @@ -32,6 +32,7 @@ export async function getAllCredentials( ): Promise { return await makeRestApiRequest(context, 'GET', '/credentials', { ...(includeScopes ? { includeScopes } : {}), + includeData: true, ...(filter ? { filter } : {}), }); } diff --git a/packages/editor-ui/src/api/sourceControl.ts b/packages/editor-ui/src/api/sourceControl.ts index 3aa929b6af..4553856928 100644 --- a/packages/editor-ui/src/api/sourceControl.ts +++ b/packages/editor-ui/src/api/sourceControl.ts @@ -33,7 +33,7 @@ export const pushWorkfolder = async ( export const pullWorkfolder = async ( context: IRestApiContext, data: PullWorkFolderRequestDto, -): Promise => { +): Promise => { return await makeRestApiRequest(context, 'POST', `${sourceControlApiRoot}/pull-workfolder`, data); }; diff --git a/packages/editor-ui/src/components/CredentialCard.vue b/packages/editor-ui/src/components/CredentialCard.vue index db40f0d274..4d4dd131f6 100644 --- a/packages/editor-ui/src/components/CredentialCard.vue +++ b/packages/editor-ui/src/components/CredentialCard.vue @@ -29,6 +29,7 @@ const props = withDefaults( defineProps<{ data: ICredentialsResponse; readOnly?: boolean; + needsSetup?: boolean; }>(), { data: () => ({ @@ -146,6 +147,9 @@ function moveResource() { {{ locale.baseText('credentials.item.readonly') }} + + {{ locale.baseText('credentials.item.needsSetup') }} +
@@ -195,10 +199,6 @@ function moveResource() { .cardHeading { font-size: var(--font-size-s); padding: var(--spacing-s) 0 0; - - span { - color: var(--color-text-light); - } } .cardDescription { diff --git a/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts b/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts index 4c192ffba6..dc3e805b05 100644 --- a/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts +++ b/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts @@ -3,7 +3,7 @@ import { waitFor } from '@testing-library/vue'; import userEvent from '@testing-library/user-event'; import { createTestingPinia } from '@pinia/testing'; import { merge } from 'lodash-es'; -import { SOURCE_CONTROL_PULL_MODAL_KEY, STORES } from '@/constants'; +import { SOURCE_CONTROL_PULL_MODAL_KEY, SOURCE_CONTROL_PUSH_MODAL_KEY, STORES } from '@/constants'; import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils'; import MainSidebarSourceControl from '@/components/MainSidebarSourceControl.vue'; import { useSourceControlStore } from '@/stores/sourceControl.store'; @@ -18,8 +18,9 @@ let rbacStore: ReturnType; const showMessage = vi.fn(); const showError = vi.fn(); +const showToast = vi.fn(); vi.mock('@/composables/useToast', () => ({ - useToast: () => ({ showMessage, showError }), + useToast: () => ({ showMessage, showError, showToast }), })); const renderComponent = createComponentRenderer(MainSidebarSourceControl); @@ -131,5 +132,129 @@ describe('MainSidebarSourceControl', () => { ), ); }); + + it('should open push modal when there are changes', async () => { + const status = [ + { + id: '014da93897f146d2b880-baa374b9d02d', + name: 'vuelfow2', + type: 'workflow' as const, + status: 'created' as const, + location: 'local' as const, + conflict: false, + file: '/014da93897f146d2b880-baa374b9d02d.json', + updatedAt: '2025-01-09T13:12:24.580Z', + }, + ]; + vi.spyOn(sourceControlStore, 'getAggregatedStatus').mockResolvedValueOnce(status); + const openModalSpy = vi.spyOn(uiStore, 'openModalWithData'); + + const { getAllByRole } = renderComponent({ + pinia, + props: { isCollapsed: false }, + }); + + await userEvent.click(getAllByRole('button')[1]); + await waitFor(() => + expect(openModalSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: SOURCE_CONTROL_PUSH_MODAL_KEY, + data: expect.objectContaining({ + status, + }), + }), + ), + ); + }); + + it("should show user's feedback when pulling", async () => { + vi.spyOn(sourceControlStore, 'pullWorkfolder').mockResolvedValueOnce([ + { + id: '014da93897f146d2b880-baa374b9d02d', + name: 'vuelfow2', + type: 'workflow', + status: 'created', + location: 'remote', + conflict: false, + file: '/014da93897f146d2b880-baa374b9d02d.json', + updatedAt: '2025-01-09T13:12:24.580Z', + }, + { + id: 'a102c0b9-28ac-43cb-950e-195723a56d54', + name: 'Gmail account', + type: 'credential', + status: 'created', + location: 'remote', + conflict: false, + file: '/a102c0b9-28ac-43cb-950e-195723a56d54.json', + updatedAt: '2025-01-09T13:12:24.586Z', + }, + { + id: 'variables', + name: 'variables', + type: 'variables', + status: 'modified', + location: 'remote', + conflict: false, + file: '/variable_stubs.json', + updatedAt: '2025-01-09T13:12:24.588Z', + }, + { + id: 'mappings', + name: 'tags', + type: 'tags', + status: 'modified', + location: 'remote', + conflict: false, + file: '/tags.json', + updatedAt: '2024-12-16T12:53:12.155Z', + }, + ]); + + const { getAllByRole } = renderComponent({ + pinia, + props: { isCollapsed: false }, + }); + + await userEvent.click(getAllByRole('button')[0]); + await waitFor(() => { + expect(showToast).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + title: 'Finish setting up your new variables to use in workflows', + }), + ); + expect(showToast).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + title: 'Finish setting up your new credentials to use in workflows', + }), + ); + expect(showToast).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + message: '1 Workflow, 1 Credential, Variables, and Tags were pulled', + }), + ); + }); + }); + + it('should show feedback where there are no change to pull', async () => { + vi.spyOn(sourceControlStore, 'pullWorkfolder').mockResolvedValueOnce([]); + + const { getAllByRole } = renderComponent({ + pinia, + props: { isCollapsed: false }, + }); + + await userEvent.click(getAllByRole('button')[0]); + await waitFor(() => { + expect(showMessage).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Up to date', + }), + ); + }); + }); }); }); diff --git a/packages/editor-ui/src/components/MainSidebarSourceControl.vue b/packages/editor-ui/src/components/MainSidebarSourceControl.vue index 1fa8338a3f..af861e93c9 100644 --- a/packages/editor-ui/src/components/MainSidebarSourceControl.vue +++ b/packages/editor-ui/src/components/MainSidebarSourceControl.vue @@ -1,5 +1,5 @@