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 index 70f7085be56..f6d3c88e23f 100644 --- 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 @@ -11,6 +11,7 @@ import { WorkflowEntity, type WorkflowRepository, } from '@n8n/db'; +import { In } from '@n8n/typeorm'; import * as fastGlob from 'fast-glob'; import { mock } from 'jest-mock-extended'; import { type InstanceSettings } from 'n8n-core'; @@ -368,114 +369,6 @@ describe('SourceControlImportService', () => { }); }); - describe('getRemoteFoldersAndMappingsFromFile', () => { - it('should parse folders and mappings file correctly', async () => { - globMock.mockResolvedValue(['/mock/folders.json']); - - const now = new Date(); - - const mockFoldersData: { - folders: ExportableFolder[]; - } = { - folders: [ - { - id: 'folder1', - name: 'folder 1', - parentFolderId: null, - homeProjectId: 'project1', - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }, - ], - }; - - fsReadFile.mockResolvedValue(JSON.stringify(mockFoldersData)); - - const result = await service.getRemoteFoldersAndMappingsFromFile(globalAdminContext); - - expect(result.folders).toEqual(mockFoldersData.folders); - }); - - it('should return empty folders and mappings if no file found', async () => { - globMock.mockResolvedValue([]); - - const result = await service.getRemoteFoldersAndMappingsFromFile(globalAdminContext); - - expect(result.folders).toHaveLength(0); - }); - - it('should return only folder that belong to a project that belongs to the user', async () => { - globMock.mockResolvedValue(['/mock/folders.json']); - - const now = new Date(); - - const foldersToFind: ExportableFolder[] = [ - { - id: 'folder1', - name: 'folder 1', - parentFolderId: null, - homeProjectId: 'project1', - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }, - { - id: 'folder3', - name: 'folder 3', - parentFolderId: null, - homeProjectId: 'project1', - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }, - { - id: 'folder4', - name: 'folder 3', - parentFolderId: null, - homeProjectId: 'project3', - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }, - ]; - - const mockFoldersData: { - folders: ExportableFolder[]; - } = { - folders: [ - { - id: 'folder0', - name: 'folder 0', - parentFolderId: null, - homeProjectId: 'project0', - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }, - ...foldersToFind, - { - id: 'folder2', - name: 'folder 2', - parentFolderId: null, - homeProjectId: 'project2', - createdAt: now.toISOString(), - updatedAt: now.toISOString(), - }, - ], - }; - - sourceControlScopedService.getAuthorizedProjectsFromContext.mockResolvedValue([ - Object.assign(new Project(), { - id: 'project1', - }), - Object.assign(new Project(), { - id: 'project3', - }), - ]); - fsReadFile.mockResolvedValue(JSON.stringify(mockFoldersData)); - - const result = await service.getRemoteFoldersAndMappingsFromFile(globalMemberContext); - - expect(result.folders).toEqual(foldersToFind); - }); - }); - describe('getLocalVersionIdsFromDb', () => { const now = new Date(); jest.useFakeTimers({ now }); @@ -497,26 +390,156 @@ describe('SourceControlImportService', () => { }); }); - describe('getLocalFoldersAndMappingsFromDb', () => { - it('should return data from DB', async () => { - // Arrange + describe('folders', () => { + describe('getRemoteFoldersAndMappingsFromFile', () => { + it('should parse folders and mappings file correctly', async () => { + globMock.mockResolvedValue(['/mock/folders.json']); - folderRepository.find.mockResolvedValue([ - mock({ createdAt: new Date(), updatedAt: new Date() }), - ]); - workflowRepository.find.mockResolvedValue([mock()]); + const now = new Date(); - // Act + const mockFoldersData: { + folders: ExportableFolder[]; + } = { + folders: [ + { + id: 'folder1', + name: 'folder 1', + parentFolderId: null, + homeProjectId: 'project1', + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }, + ], + }; - const result = await service.getLocalFoldersAndMappingsFromDb(globalAdminContext); + fsReadFile.mockResolvedValue(JSON.stringify(mockFoldersData)); - // Assert + const result = await service.getRemoteFoldersAndMappingsFromFile(globalAdminContext); - expect(result.folders).toHaveLength(1); - expect(result.folders[0]).toHaveProperty('id'); - expect(result.folders[0]).toHaveProperty('name'); - expect(result.folders[0]).toHaveProperty('parentFolderId'); - expect(result.folders[0]).toHaveProperty('homeProjectId'); + expect(result.folders).toEqual(mockFoldersData.folders); + }); + + it('should return empty folders and mappings if no file found', async () => { + globMock.mockResolvedValue([]); + + const result = await service.getRemoteFoldersAndMappingsFromFile(globalAdminContext); + + expect(result.folders).toHaveLength(0); + }); + + it('should return only folder that belong to a project that belongs to the user', async () => { + globMock.mockResolvedValue(['/mock/folders.json']); + + const now = new Date(); + + const foldersToFind: ExportableFolder[] = [ + { + id: 'folder1', + name: 'folder 1', + parentFolderId: null, + homeProjectId: 'project1', + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }, + { + id: 'folder3', + name: 'folder 3', + parentFolderId: null, + homeProjectId: 'project1', + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }, + { + id: 'folder4', + name: 'folder 3', + parentFolderId: null, + homeProjectId: 'project3', + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }, + ]; + + const mockFoldersData: { + folders: ExportableFolder[]; + } = { + folders: [ + { + id: 'folder0', + name: 'folder 0', + parentFolderId: null, + homeProjectId: 'project0', + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }, + ...foldersToFind, + { + id: 'folder2', + name: 'folder 2', + parentFolderId: null, + homeProjectId: 'project2', + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }, + ], + }; + + sourceControlScopedService.getAuthorizedProjectsFromContext.mockResolvedValue([ + Object.assign(new Project(), { + id: 'project1', + }), + Object.assign(new Project(), { + id: 'project3', + }), + ]); + fsReadFile.mockResolvedValue(JSON.stringify(mockFoldersData)); + + const result = await service.getRemoteFoldersAndMappingsFromFile(globalMemberContext); + + expect(result.folders).toEqual(foldersToFind); + }); + }); + + describe('getLocalFoldersAndMappingsFromDb', () => { + it('should return data from DB', async () => { + // Arrange + + folderRepository.find.mockResolvedValue([ + mock({ createdAt: new Date(), updatedAt: new Date() }), + ]); + workflowRepository.find.mockResolvedValue([mock()]); + + // Act + + const result = await service.getLocalFoldersAndMappingsFromDb(globalAdminContext); + + // Assert + + expect(result.folders).toHaveLength(1); + expect(result.folders[0]).toHaveProperty('id'); + expect(result.folders[0]).toHaveProperty('name'); + expect(result.folders[0]).toHaveProperty('parentFolderId'); + expect(result.folders[0]).toHaveProperty('homeProjectId'); + }); + }); + + describe('deleteFoldersNotInWorkfolder', () => { + it('should call folderRepository.delete with correct ids', async () => { + const candidates = [ + mock({ id: 'folder1' }), + mock({ id: 'folder2' }), + mock({ id: 'folder3' }), + ]; + await service.deleteFoldersNotInWorkfolder(candidates as any); + + expect(folderRepository.delete).toHaveBeenCalledWith({ + id: In(['folder1', 'folder2', 'folder3']), + }); + }); + + it('should not call folderRepository.delete if candidates is empty', async () => { + await service.deleteFoldersNotInWorkfolder([]); + expect(folderRepository.delete).not.toHaveBeenCalled(); + }); }); }); @@ -873,5 +896,26 @@ describe('SourceControlImportService', () => { }); }); }); + + describe('deleteTeamProjectsNotInWorkfolder', () => { + it('should delete candidate files', async () => { + const candidates = [ + mock({ id: 'project-1' }), + mock({ id: 'project-2' }), + ]; + + await service.deleteTeamProjectsNotInWorkfolder(candidates); + + expect(projectRepository.delete).toHaveBeenCalledWith({ + id: In(['project-1', 'project-2']), + }); + }); + + it('should handle empty candidates array', async () => { + await service.deleteTeamProjectsNotInWorkfolder([]); + + expect(projectRepository.delete).not.toHaveBeenCalled(); + }); + }); }); }); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts index 5195d5167b1..391a191fcd8 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts @@ -17,6 +17,7 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import type { EventService } from '@/events/event.service'; import type { SourceControlGitService } from '../source-control-git.service.ee'; +import * as sourceControlHelper from '../source-control-helper.ee'; import type { SourceControlImportService } from '../source-control-import.service.ee'; import { SourceControlPreferencesService } from '../source-control-preferences.service.ee'; import { SourceControlStatusService } from '../source-control-status.service.ee'; @@ -90,6 +91,8 @@ describe('getStatus', () => { // repositories tagRepository.find.mockResolvedValue([]); folderRepository.find.mockResolvedValue([]); + + jest.spyOn(sourceControlHelper, 'sourceControlFoldersExistCheck').mockReturnValue(true); }); it('ensure updatedAt field for last deleted tag', async () => { @@ -238,13 +241,27 @@ describe('getStatus', () => { ], }); - // Define a project that does only exist locally. - // Pulling this would delete it so it should be marked as a conflict. - // Pushing this is conflict free. - - sourceControlImportService.getRemoteProjectsFromFiles.mockResolvedValue([]); + // Define a project that only exists locally and another that exists both locally and remotely. + const project1 = mock({ + id: 'project-id-1', + name: 'Project 1 Remote', + owner: { + type: 'team', + teamId: 'team-id-1', + teamName: 'Team 1', + }, + }); + sourceControlImportService.getRemoteProjectsFromFiles.mockResolvedValue([ + mock({ ...project1 }), + ]); sourceControlImportService.getLocalTeamProjectsFromDb.mockResolvedValue([ - mock(), + mock({ + ...project1, + name: 'Project 1 Local', + }), + mock({ + id: 'project-id-2', + }), ]); // ACT @@ -269,8 +286,8 @@ describe('getStatus', () => { fail('Expected pushResult to be an array.'); } - expect(pullResult).toHaveLength(6); - expect(pushResult).toHaveLength(6); + expect(pullResult).toHaveLength(7); + expect(pushResult).toHaveLength(7); expect(pullResult.find((i) => i.type === 'workflow')).toHaveProperty('conflict', true); expect(pushResult.find((i) => i.type === 'workflow')).toHaveProperty('conflict', false); @@ -287,8 +304,23 @@ describe('getStatus', () => { expect(pullResult.find((i) => i.type === 'folders')).toHaveProperty('conflict', true); expect(pushResult.find((i) => i.type === 'folders')).toHaveProperty('conflict', false); - expect(pullResult.find((i) => i.type === 'project')).toHaveProperty('conflict', true); - expect(pushResult.find((i) => i.type === 'project')).toHaveProperty('conflict', false); + expect(pullResult.find((i) => i.type === 'project' && i.id === 'project-id-2')).toHaveProperty( + 'conflict', + true, + ); + expect(pushResult.find((i) => i.type === 'project' && i.id === 'project-id-2')).toHaveProperty( + 'conflict', + false, + ); + + expect(pullResult.find((i) => i.type === 'project' && i.id === 'project-id-1')).toHaveProperty( + 'conflict', + true, + ); + expect(pushResult.find((i) => i.type === 'project' && i.id === 'project-id-1')).toHaveProperty( + 'conflict', + true, + ); }); it('should throw `ForbiddenError` if direction is pull and user is not allowed to globally pull', async () => { @@ -704,6 +736,52 @@ describe('getStatus', () => { ); }); }); + + it('should not mark projects for deletion when projects folder does not exist (backward compatibility)', async () => { + // ARRANGE + const user = mockUsers.globalAdmin; + const localProjects = [ + { + id: 'local-project-1', + name: 'Local Project 1', + description: 'Local project description', + icon: null, + type: 'team' as const, + owner: { + type: 'team' as const, + teamId: 'local-project-1', + teamName: 'Local Project 1', + }, + filename: '/mock/n8n/git/projects/local-project-1.json', + }, + ]; + + setupProjectMocks({ + remote: [], + local: localProjects, + }); + + // Override the default mock: folder doesn't exist (backward compatibility scenario) + jest.spyOn(sourceControlHelper, 'sourceControlFoldersExistCheck').mockReturnValue(false); + + // ACT + const result = await sourceControlStatusService.getStatus(user, { + direction: 'pull', + verbose: false, + preferLocalVersion: false, + }); + + // ASSERT + if (!Array.isArray(result)) { + fail('Expected result to be an array.'); + } + + // Should NOT include any project deletion entries + const projectDeletions = result.filter( + (file) => file.type === 'project' && file.status === 'deleted', + ); + expect(projectDeletions).toHaveLength(0); + }); }); describe('workflows', () => { 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 67d6d95fb8d..1b3e48faa96 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 @@ -1,19 +1,20 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { isContainedWithin } from '@n8n/backend-common'; -import { type User, type WorkflowEntity, GLOBAL_ADMIN_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db'; +import { GLOBAL_ADMIN_ROLE, GLOBAL_MEMBER_ROLE, User, type WorkflowEntity } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; import { InstanceSettings } from 'n8n-core'; import type { PushResult } from 'simple-git'; -import type { SourceControlGitService } from '../source-control-git.service.ee'; import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import type { EventService } from '@/events/event.service'; +import type { SourceControlExportService } from '../source-control-export.service.ee'; +import type { SourceControlGitService } from '../source-control-git.service.ee'; import type { SourceControlImportService } from '../source-control-import.service.ee'; import type { SourceControlScopedService } from '../source-control-scoped.service'; -import type { SourceControlExportService } from '../source-control-export.service.ee'; +import type { ExportResult } from '../types/export-result'; // Mock the status service to avoid complex dependency issues const mockStatusService = { @@ -57,6 +58,210 @@ describe('SourceControlService', () => { }); describe('pushWorkfolder', () => { + it('should push the workfolder', async () => { + const mockExportResult = mock(); + // Arrange + const user = Object.assign(new User(), { + role: GLOBAL_ADMIN_ROLE, + }); + + const mockPushResult = mock(); + const now = new Date().toISOString(); + + // Prepare a set of files of all types, some deleted, some not + const files: SourceControlledFile[] = [ + { + file: 'workflow-1.json', + id: 'wf-1', + name: 'Workflow 1', + type: 'workflow', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'credential-1.json', + id: 'cred-1', + name: 'Credential 1', + type: 'credential', + status: 'created', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'project-1.json', + id: 'proj-1', + name: 'Project 1', + type: 'project', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'folders.json', + id: 'folders', + name: 'Folders', + type: 'folders', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'variables.json', + id: 'variables', + name: 'Variables', + type: 'variables', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'tags.json', + id: 'tags', + name: 'Tags', + type: 'tags', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: now, + }, + // Deleted resources + { + file: 'workflow-2.json', + id: 'wf-2', + name: 'Workflow 2', + type: 'workflow', + status: 'deleted', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'credential-2.json', + id: 'cred-2', + name: 'Credential 2', + type: 'credential', + status: 'deleted', + location: 'local', + conflict: false, + updatedAt: now, + }, + { + file: 'project-2.json', + id: 'proj-2', + name: 'Project 2', + type: 'project', + status: 'deleted', + location: 'local', + conflict: false, + updatedAt: now, + }, + ]; + + // The status service should return all these files as allowed + mockStatusService.getStatus.mockResolvedValueOnce(files); + + // Mock all export and delete methods + sourceControlExportService.exportWorkflowsToWorkFolder.mockResolvedValueOnce( + mockExportResult, + ); + sourceControlExportService.exportCredentialsToWorkFolder.mockResolvedValueOnce({ + count: 1, + missingIds: [], + folder: '', + files: [], + }); + sourceControlExportService.exportTeamProjectsToWorkFolder.mockResolvedValueOnce( + mockExportResult, + ); + sourceControlExportService.exportTagsToWorkFolder.mockResolvedValueOnce(mockExportResult); + sourceControlExportService.exportFoldersToWorkFolder.mockResolvedValueOnce(mockExportResult); + sourceControlExportService.exportVariablesToWorkFolder.mockResolvedValueOnce( + mockExportResult, + ); + sourceControlExportService.exportFoldersToWorkFolder.mockResolvedValueOnce(mockExportResult); + sourceControlExportService.exportVariablesToWorkFolder.mockResolvedValueOnce( + mockExportResult, + ); + + (isContainedWithin as jest.Mock).mockReturnValue(true); + + gitService.push.mockResolvedValueOnce(mockPushResult); + + const commitMessage = 'Test commit message'; + + // Act + const result = await sourceControlService.pushWorkfolder(user, { + fileNames: files.map((f) => ({ + file: f.file, + id: f.id, + name: f.name, + type: f.type, + status: f.status, + location: f.location, + conflict: f.conflict, + updatedAt: f.updatedAt, + })), + commitMessage, + }); + + // Assert + // All export methods for non-deleted resources should be called + expect(sourceControlExportService.exportWorkflowsToWorkFolder).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({ id: 'wf-1' })]), + ); + expect(sourceControlExportService.exportCredentialsToWorkFolder).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({ id: 'cred-1' })]), + ); + expect(sourceControlExportService.exportTeamProjectsToWorkFolder).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({ id: 'proj-1' })]), + ); + expect(sourceControlExportService.exportTagsToWorkFolder).toHaveBeenCalled(); + expect(sourceControlExportService.exportFoldersToWorkFolder).toHaveBeenCalled(); + expect(sourceControlExportService.exportVariablesToWorkFolder).toHaveBeenCalled(); + + // Deleted resources should be passed to rmFilesFromExportFolder + expect(sourceControlExportService.rmFilesFromExportFolder).toHaveBeenCalledWith( + new Set([ + `${preferencesService.gitFolder}/workflow-2.json`, + `${preferencesService.gitFolder}/credential-2.json`, + `${preferencesService.gitFolder}/project-2.json`, + ]), + ); + + // Git operations should be called + expect(gitService.stage).toHaveBeenCalledWith( + new Set([ + `${preferencesService.gitFolder}/workflow-1.json`, + `${preferencesService.gitFolder}/credential-1.json`, + `${preferencesService.gitFolder}/project-1.json`, + `${preferencesService.gitFolder}/folders.json`, + `${preferencesService.gitFolder}/variables.json`, + `${preferencesService.gitFolder}/tags.json`, + ]), + new Set([ + `${preferencesService.gitFolder}/workflow-2.json`, + `${preferencesService.gitFolder}/credential-2.json`, + `${preferencesService.gitFolder}/project-2.json`, + ]), + ); + expect(gitService.commit).toHaveBeenCalledWith(commitMessage); + expect(gitService.push).toHaveBeenCalledWith({ + branch: 'main', // default branch + force: false, + }); + + // The result should include the status and push result + expect(result).toMatchObject({ + statusCode: 200, + }); + }); + it('should throw an error if file path validation fails', async () => { const user = mock(); (isContainedWithin as jest.Mock).mockReturnValueOnce(false); @@ -78,12 +283,15 @@ describe('SourceControlService', () => { ], }), ).rejects.toThrow('File path /etc/passwd is invalid'); + + expect(gitService.stage).not.toHaveBeenCalled(); + expect(gitService.commit).not.toHaveBeenCalled(); + expect(gitService.push).not.toHaveBeenCalled(); }); it('should include the tags file even if not explicitly specified', async () => { // ARRANGE const user = mock(); - const mockPushResult = mock(); const mockFile: SourceControlledFile = { file: 'some-workflow.json', id: 'test', @@ -103,7 +311,10 @@ describe('SourceControlService', () => { files: [], }); eventService.emit.mockReturnValueOnce(true); + + const mockPushResult = mock(); gitService.push.mockResolvedValueOnce(mockPushResult); + (isContainedWithin as jest.Mock).mockReturnValueOnce(true); const expectedTagsPath = `${preferencesService.gitFolder}/tags.json`; 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 a109f5e8554..167d4775857 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 @@ -1071,9 +1071,25 @@ export class SourceControlImportService { } async deleteFoldersNotInWorkfolder(candidates: SourceControlledFile[]) { - for (const candidate of candidates) { - await this.folderRepository.delete(candidate.id); + if (candidates.length === 0) { + return; } + const candidateIds = candidates.map((c) => c.id); + + await this.folderRepository.delete({ + id: In(candidateIds), + }); + } + + async deleteTeamProjectsNotInWorkfolder(candidates: SourceControlledFile[]) { + if (candidates.length === 0) { + return; + } + const candidateIds = candidates.map((c) => c.id); + + await this.projectRepository.delete({ + id: In(candidateIds), + }); } /** diff --git a/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts index c7d6090c01e..fcf537b7331 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts @@ -1,6 +1,6 @@ import type { SourceControlledFile } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; -import { type TagEntity, FolderRepository, TagRepository, type User, Variables } from '@n8n/db'; +import { FolderRepository, type TagEntity, TagRepository, type User, Variables } from '@n8n/db'; import { Service } from '@n8n/di'; import { hasGlobalScope } from '@n8n/permissions'; import { UserError } from 'n8n-workflow'; @@ -693,9 +693,16 @@ export class SourceControlStatusService { (remote) => !outOfScopeProjects.some((outOfScope) => outOfScope.id === remote.id), ); - const projectsMissingInRemote = projectsLocal.filter( + // BACKWARD COMPATIBILITY: When there are no remote projects we can't safely delete local projects + // because we don't know if it's the first pull or if all team projects have been removed + // As a downside this means that it's not possible to delete all team projects via source control sync + const areRemoteProjectsEmpty = projectsRemote.length === 0; + let projectsMissingInRemote = projectsLocal.filter( (local) => !projectsRemote.some((remote) => remote.id === local.id), ); + if (options.direction === 'pull' && areRemoteProjectsEmpty) { + projectsMissingInRemote = []; + } const projectsModifiedInEither: ExportableProjectWithFileName[] = []; 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 7ce763b35f4..8986b3e1fa1 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 @@ -28,8 +28,13 @@ import { } from './source-control-helper.ee'; import { SourceControlImportService } from './source-control-import.service.ee'; import { SourceControlPreferencesService } from './source-control-preferences.service.ee'; -import { SourceControlStatusService } from './source-control-status.service.ee'; +import { + filterByType, + getDeletedResources, + getNonDeletedResources, +} from './source-control-resource-helper'; import { SourceControlScopedService } from './source-control-scoped.service'; +import { SourceControlStatusService } from './source-control-status.service.ee'; import type { ImportResult } from './types/import-result'; import { SourceControlContext } from './types/source-control-context'; import type { SourceControlGetStatus } from './types/source-control-get-status'; @@ -38,13 +43,6 @@ import type { SourceControlPreferences } from './types/source-control-preference import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { EventService } from '@/events/event.service'; - -import { - filterByType, - getDeletedResources, - getNonDeletedResources, -} from './source-control-resource-helper'; - import { IWorkflowToImport } from '@/interfaces'; @Service() @@ -297,7 +295,7 @@ export class SourceControlService { we keep track of them in a single file unlike workflows and credentials */ filesToPush - .filter((f) => ['workflow', 'credential'].includes(f.type)) + .filter((f) => ['workflow', 'credential', 'project'].includes(f.type)) .forEach((e) => { if (e.status !== 'deleted') { filesToBePushed.add(e.file); @@ -323,6 +321,9 @@ export class SourceControlService { }); } + const projectsToBeExported = getNonDeletedResources(filesToPush, 'project'); + await this.sourceControlExportService.exportTeamProjectsToWorkFolder(projectsToBeExported); + // The tags file is always re-generated and exported to make sure the workflow-tag mappings are up to date filesToBePushed.add(getTagsPath(this.gitFolder)); await this.sourceControlExportService.exportTagsToWorkFolder(context); @@ -365,10 +366,6 @@ export class SourceControlService { }; } - private getConflicts(files: SourceControlledFile[]): SourceControlledFile[] { - return files.filter((file) => file.conflict || file.status === 'modified'); - } - async pullWorkfolder( user: User, options: PullWorkFolderRequestDto, @@ -382,7 +379,10 @@ export class SourceControlService { })) as SourceControlledFile[]; if (options.force !== true) { - const possibleConflicts = this.getConflicts(statusResult); + const possibleConflicts = statusResult.filter( + (file) => file.conflict || file.status === 'modified', + ); + if (possibleConflicts?.length > 0) { await this.gitService.resetBranch(); return { @@ -392,7 +392,10 @@ export class SourceControlService { } } - // Make sure the folders get processed first as the workflows depend on them + // IMPORTANT: Make sure the projects and folders get processed first as the workflows depend on them + const projectsToBeImported = getNonDeletedResources(statusResult, 'project'); + await this.sourceControlImportService.importTeamProjectsFromWorkFolder(projectsToBeImported); + const foldersToBeImported = getNonDeletedResources(statusResult, 'folders')[0]; if (foldersToBeImported) { await this.sourceControlImportService.importFoldersFromWorkFolder(user, foldersToBeImported); @@ -408,6 +411,7 @@ export class SourceControlService { user, workflowsToBeDeleted, ); + const credentialsToBeImported = getNonDeletedResources(statusResult, 'credential'); await this.sourceControlImportService.importCredentialsFromWorkFolder( credentialsToBeImported, @@ -436,6 +440,9 @@ export class SourceControlService { const foldersToBeDeleted = getDeletedResources(statusResult, 'folders'); await this.sourceControlImportService.deleteFoldersNotInWorkfolder(foldersToBeDeleted); + const projectsToBeDeleted = getDeletedResources(statusResult, 'project'); + await this.sourceControlImportService.deleteTeamProjectsNotInWorkfolder(projectsToBeDeleted); + // #region Tracking Information this.eventService.emit( 'source-control-user-finished-pull-ui', diff --git a/packages/cli/test/integration/environments/source-control.service.test.ts b/packages/cli/test/integration/environments/source-control.service.test.ts index a30593c8048..1af38df1bcf 100644 --- a/packages/cli/test/integration/environments/source-control.service.test.ts +++ b/packages/cli/test/integration/environments/source-control.service.test.ts @@ -874,11 +874,19 @@ describe('SourceControlService', () => { const credentialFiles = result.statusResult .filter((change) => change.type === 'credential' && change.status !== 'deleted') .map((change) => change.file); + + const projectFiles = result.statusResult + .filter((change) => change.type === 'project' && change.status !== 'deleted') + .map((change) => change.file); + expect(workflowFiles).toHaveLength(8); expect(credentialFiles).toHaveLength(2); + expect(projectFiles).toHaveLength(2); expect(gitService.push).toBeCalled(); - expect(fsWriteFile).toBeCalledTimes(workflowFiles.length + credentialFiles.length + 2); // folders + tags + expect(fsWriteFile).toBeCalledTimes( + workflowFiles.length + credentialFiles.length + projectFiles.length + 2, + ); // folders + tags expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(workflowFiles)); expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(credentialFiles)); expect(Object.keys(updatedFiles)).toEqual( @@ -908,14 +916,22 @@ describe('SourceControlService', () => { const credentialFiles = result.statusResult .filter((change) => change.type === 'credential' && change.status !== 'deleted') .map((change) => change.file); + const projectFiles = result.statusResult + .filter((change) => change.type === 'project' && change.status !== 'deleted') + .map((change) => change.file); + expect(workflowFiles).toHaveLength(8); expect(credentialFiles).toHaveLength(2); - const numberFilesToWrite = workflowFiles.length + credentialFiles.length + 2; // folders + tags + expect(projectFiles).toHaveLength(2); + const numberFilesToWrite = + workflowFiles.length + credentialFiles.length + projectFiles.length + 2; // folders + tags + projects const filesToWrite = allChanges.filter( (change) => - (change.type === 'workflow' || change.type === 'credential') && + (change.type === 'workflow' || + change.type === 'credential' || + change.type === 'project') && change.status !== 'deleted', ).length + 2; // folders + tags @@ -924,6 +940,7 @@ describe('SourceControlService', () => { expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(workflowFiles)); expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(credentialFiles)); + expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(projectFiles)); expect(Object.keys(updatedFiles)).toEqual( expect.arrayContaining([expect.stringMatching(SOURCE_CONTROL_FOLDERS_EXPORT_FILE)]), ); @@ -961,11 +978,18 @@ describe('SourceControlService', () => { const credentialFiles = result.statusResult .filter((change) => change.type === 'credential' && change.status !== 'deleted') .map((change) => change.file); + const projectFiles = result.statusResult + .filter((change) => change.type === 'project' && change.status !== 'deleted') + .map((change) => change.file); expect(workflowFiles).toHaveLength(2); expect(credentialFiles).toHaveLength(1); + expect(projectFiles).toHaveLength(1); - expect(fsWriteFile).toBeCalledTimes(workflowFiles.length + credentialFiles.length + 2); // folders + tags + // folders + tags + projects (1) + expect(fsWriteFile).toBeCalledTimes( + workflowFiles.length + credentialFiles.length + projectFiles.length + 2, + ); expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(workflowFiles)); expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(credentialFiles)); expect(Object.keys(updatedFiles)).toEqual( @@ -974,6 +998,7 @@ describe('SourceControlService', () => { expect(Object.keys(updatedFiles)).toEqual( expect.arrayContaining([expect.stringMatching(SOURCE_CONTROL_TAGS_EXPORT_FILE)]), ); + expect(Object.keys(updatedFiles)).toEqual(expect.arrayContaining(projectFiles)); }); it('should throw ForbiddenError when trying to push workflows out of scope', async () => { diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 0504550cb5e..a46edec5d8f 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -125,6 +125,7 @@ "generic.list.clearSelection": "Clear selection", "generic.list.selected": "{count} row selected | {count} rows selected", "generic.project": "Project", + "generic.projects": "Projects", "generic.your": "Your", "generic.apiKey": "API Key", "about.aboutN8n": "About n8n", diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectNavigation.vue b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectNavigation.vue index 503e17e91ea..e800765dd02 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectNavigation.vue +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectNavigation.vue @@ -1,15 +1,16 @@ diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.test.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.test.ts index b6746aeccb7..cb9aa0f10b8 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.test.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.test.ts @@ -298,4 +298,83 @@ describe('SourceControlPullModal', () => { const listItems = getAllByTestId('pull-modal-item'); expect(listItems.length).toBeGreaterThan(0); }); + + it('should display projects in the otherFiles section', () => { + const status: SourceControlledFile[] = [ + { + id: 'project-1', + name: 'Team Project 1', + type: 'project', + status: 'created', + location: 'remote', + conflict: false, + file: '/projects/project-1.json', + updatedAt: '2025-01-09T13:12:24.586Z', + owner: { + type: 'team', + projectId: 'project-1', + projectName: 'Team Project 1', + }, + }, + ]; + + const { getByText } = renderModal({ + pinia, + props: { + data: { + eventBus, + status, + }, + }, + }); + + expect(getByText(/Projects \(1\)/)).toBeInTheDocument(); + }); + + it('should show correct project count in summary', () => { + const status: SourceControlledFile[] = [ + { + id: 'project-1', + name: 'Team Project 1', + type: 'project', + status: 'created', + location: 'remote', + conflict: false, + file: '/projects/project-1.json', + updatedAt: '2025-01-09T13:12:24.586Z', + owner: { + type: 'team', + projectId: 'project-1', + projectName: 'Team Project 1', + }, + }, + { + id: 'project-2', + name: 'Team Project 2', + type: 'project', + status: 'modified', + location: 'remote', + conflict: true, + file: '/projects/project-2.json', + updatedAt: '2025-01-09T13:12:24.586Z', + owner: { + type: 'team', + projectId: 'project-2', + projectName: 'Team Project 2', + }, + }, + ]; + + const { getByText } = renderModal({ + pinia, + props: { + data: { + eventBus, + status, + }, + }, + }); + + expect(getByText(/Projects \(2\)/)).toBeInTheDocument(); + }); }); diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.vue b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.vue index e9805b9e87c..333246133cf 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.vue +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPullModal.vue @@ -192,6 +192,10 @@ const otherFiles = computed(() => { if (folders) { others.push.apply(others, folders); } + const projects = groupedFilesByType.value[SOURCE_CONTROL_FILE_TYPE.project]; + if (projects) { + others.push.apply(others, projects); + } return others; }); @@ -399,7 +403,10 @@ onMounted(() => { Tags ({{ groupedFilesByType[SOURCE_CONTROL_FILE_TYPE.tags]?.length || 0 }}), + diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts index 970b5caeba7..7f94fd36770 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts @@ -313,6 +313,16 @@ describe('SourceControlPushModal', () => { file: '/Users/raul/.n8n/git/folders.json', updatedAt: '2024-12-04T11:29:22.095Z', }, + { + id: 'project-1', + name: 'Team Project 1', + type: 'project', + status: 'created', + location: 'local', + conflict: false, + file: '/projects/project-1.json', + updatedAt: '2025-01-09T13:12:24.586Z', + }, ]; sourceControlStore.getAggregatedStatus.mockResolvedValue(status); @@ -342,10 +352,11 @@ describe('SourceControlPushModal', () => { expect(getByRole('alert').textContent).toContain( [ - 'Changes to variables, tags and folders', + 'Changes to variables, tags, folders and projects', 'Variables : at least one new or modified.', 'Tags : at least one new or modified.', - 'Folders : at least one new or modified. ', + 'Folders : at least one new or modified.', + 'Projects : at least one new or modified.', ].join(' '), ); diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue index 96200bfa6d0..a51b3a7c4c0 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue +++ b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue @@ -138,6 +138,7 @@ type Changes = { workflow: SourceControlledFileWithProject[]; currentWorkflow?: SourceControlledFileWithProject; folders: SourceControlledFileWithProject[]; + projects: SourceControlledFileWithProject[]; }; const classifyFilesByType = (files: SourceControlledFile[], currentWorkflowId?: string): Changes => @@ -185,6 +186,11 @@ const classifyFilesByType = (files: SourceControlledFile[], currentWorkflowId?: return acc; } + if (file.type === SOURCE_CONTROL_FILE_TYPE.project) { + acc.projects.push({ ...file, project }); + return acc; + } + return acc; }, { @@ -193,6 +199,7 @@ const classifyFilesByType = (files: SourceControlledFile[], currentWorkflowId?: credential: [], workflow: [], folders: [], + projects: [], currentWorkflow: undefined, }, ); @@ -221,6 +228,13 @@ const userNotices = computed(() => { }); } + if (changes.value.projects.length) { + messages.push({ + title: 'Projects', + content: 'at least one new or modified', + }); + } + return messages; }); const workflowId = computed( @@ -368,6 +382,7 @@ const isSubmitDisabled = computed(() => { changes.value.tags.length + changes.value.variables.length + changes.value.folders.length + + changes.value.projects.length + selectedWorkflows.size; return toBePushed <= 0; @@ -463,6 +478,10 @@ const successNotificationMessage = () => { messages.push(i18n.baseText('generic.tag_plural')); } + if (changes.value.projects.length) { + messages.push(i18n.baseText('generic.projects')); + } + return [ concatenateWithAnd(messages), i18n.baseText('settings.sourceControl.modals.push.success.description'), @@ -474,6 +493,7 @@ async function commitAndPush() { .concat(changes.value.variables) .concat(changes.value.credential.filter((file) => selectedCredentials.has(file.id))) .concat(changes.value.folders) + .concat(changes.value.projects) .concat(changes.value.workflow.filter((file) => selectedWorkflows.has(file.id))); loadingService.startLoading(i18n.baseText('settings.sourceControl.loading.push')); close(); @@ -920,7 +940,7 @@ onMounted(async () => {