From d5c093411aa4f643c46c062a41740f10e72b324a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ir=C3=A9n=C3=A9e?= Date: Fri, 19 Dec 2025 09:04:43 +0000 Subject: [PATCH] feat: Assign default project admin on pull (#23355) --- .../source-control-import.service.ee.test.ts | 110 +++++++++++++++++- .../source-control-import.service.ee.ts | 35 +++++- .../source-control.service.ee.ts | 5 +- .../source-control-import.service.test.ts | 1 + 4 files changed, 144 insertions(+), 7 deletions(-) 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 cbd258907ea..0a4daf5f009 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 @@ -7,6 +7,8 @@ import { GLOBAL_ADMIN_ROLE, GLOBAL_MEMBER_ROLE, Project, + type ProjectRelation, + type ProjectRelationRepository, type ProjectRepository, type SharedWorkflowRepository, User, @@ -47,6 +49,7 @@ describe('SourceControlImportService', () => { const workflowRepository = mock(); const folderRepository = mock(); const projectRepository = mock(); + const projectRelationRepository = mock(); const sharedWorkflowRepository = mock(); const mockLogger = mock(); const sourceControlScopedService = mock(); @@ -60,6 +63,7 @@ describe('SourceControlImportService', () => { activeWorkflowManager, mock(), projectRepository, + projectRelationRepository, mock(), sharedWorkflowRepository, mock(), @@ -857,6 +861,8 @@ describe('SourceControlImportService', () => { }); describe('projects', () => { + const mockPullingUserId = 'pulling-user-id'; + describe('importTeamProjectsFromWorkFolder', () => { it('should import team projects from work folder', async () => { // Arrange @@ -895,9 +901,13 @@ describe('SourceControlImportService', () => { .mockResolvedValueOnce(JSON.stringify(mockProjectData2)); variableService.getAllCached.mockResolvedValue([]); + projectRepository.findOne.mockResolvedValue(null); // Act - const result = await service.importTeamProjectsFromWorkFolder(candidates); + const result = await service.importTeamProjectsFromWorkFolder( + candidates, + mockPullingUserId, + ); // Assert expect(fsReadFile).toHaveBeenCalledWith(mockProjectFile1, { encoding: 'utf8' }); @@ -931,6 +941,17 @@ describe('SourceControlImportService', () => { ['id'], ); + expect(projectRelationRepository.save).toHaveBeenCalledWith({ + projectId: mockProjectData1.id, + userId: mockPullingUserId, + role: { slug: 'project:admin' }, + }); + expect(projectRelationRepository.save).toHaveBeenCalledWith({ + projectId: mockProjectData2.id, + userId: mockPullingUserId, + role: { slug: 'project:admin' }, + }); + expect(result).toEqual([ { id: mockProjectData1.id, @@ -943,6 +964,82 @@ describe('SourceControlImportService', () => { ]); }); + it('should NOT assign pulling user as project admin for existing projects with an admin', async () => { + // Arrange + const mockProjectFile = '/mock/team-project.json'; + const mockProjectData = { + id: 'existing-project', + name: 'Existing Team Project', + icon: 'icon.png', + description: 'An existing team project', + type: 'team', + owner: { + type: 'team', + teamId: 'existing-project', + }, + }; + const candidates = [ + mock({ file: mockProjectFile, id: mockProjectData.id }), + ]; + + fsReadFile.mockResolvedValueOnce(JSON.stringify(mockProjectData)); + variableService.getAllCached.mockResolvedValue([]); + // Project already exists + projectRepository.findOne.mockResolvedValue( + Object.assign(new Project(), { id: mockProjectData.id }), + ); + // Project already has an admin + projectRelationRepository.findOne.mockResolvedValue( + mock({ + projectId: mockProjectData.id, + userId: 'existing-admin-user-id', + }), + ); + + // Act + await service.importTeamProjectsFromWorkFolder(candidates, mockPullingUserId); + + // Assert - project relation should NOT be created for existing projects with admin + expect(projectRelationRepository.save).not.toHaveBeenCalled(); + }); + + it('should assign pulling user as project admin for existing projects without an admin', async () => { + // Arrange + const mockProjectFile = '/mock/team-project.json'; + const mockProjectData = { + id: 'orphaned-project', + name: 'Orphaned Team Project', + icon: 'icon.png', + description: 'An existing team project without admin', + type: 'team', + owner: { + type: 'team', + teamId: 'orphaned-project', + }, + }; + const candidates = [ + mock({ file: mockProjectFile, id: mockProjectData.id }), + ]; + + fsReadFile.mockResolvedValueOnce(JSON.stringify(mockProjectData)); + variableService.getAllCached.mockResolvedValue([]); + projectRepository.findOne.mockResolvedValue( + Object.assign(new Project(), { id: mockProjectData.id }), + ); + // Project has no admin + projectRelationRepository.findOne.mockResolvedValue(null); + + // Act + await service.importTeamProjectsFromWorkFolder(candidates, mockPullingUserId); + + // Assert - pulling user should be assigned as admin for orphaned projects + expect(projectRelationRepository.save).toHaveBeenCalledWith({ + projectId: mockProjectData.id, + userId: mockPullingUserId, + role: { slug: 'project:admin' }, + }); + }); + it('should import only valid team projects and skip invalid ones', async () => { const mockTeamProjectFile = '/mock/project-team-valid.json'; const mockTeamProjectData = { @@ -998,7 +1095,12 @@ describe('SourceControlImportService', () => { .mockResolvedValueOnce(JSON.stringify(mockNonTeamProjectData)) .mockResolvedValueOnce(JSON.stringify(mockInconsistentOwnerData)); - const result = await service.importTeamProjectsFromWorkFolder(candidates); + projectRepository.findOne.mockResolvedValue(null); + + const result = await service.importTeamProjectsFromWorkFolder( + candidates, + mockPullingUserId, + ); expect(fsReadFile).toHaveBeenCalledWith(mockTeamProjectFile, { encoding: 'utf8' }); expect(fsReadFile).toHaveBeenCalledWith(mockNonTeamProjectFile, { encoding: 'utf8' }); @@ -1055,8 +1157,10 @@ describe('SourceControlImportService', () => { } as Variables, ]); + projectRepository.findOne.mockResolvedValue(null); + // Act - await service.importTeamProjectsFromWorkFolder(candidates); + await service.importTeamProjectsFromWorkFolder(candidates, mockPullingUserId); // Assert expect(variableService.deleteByIds).toHaveBeenCalledWith(['var2']); 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 f873fdb7d7c..a453f077542 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 @@ -12,6 +12,7 @@ import type { import { CredentialsRepository, FolderRepository, + ProjectRelationRepository, ProjectRepository, SharedCredentialsRepository, SharedWorkflowRepository, @@ -23,7 +24,7 @@ import { WorkflowPublishHistoryRepository, } from '@n8n/db'; import { Service } from '@n8n/di'; -import { PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; +import { PROJECT_ADMIN_ROLE_SLUG, PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; import { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; @@ -144,6 +145,7 @@ export class SourceControlImportService { private readonly activeWorkflowManager: ActiveWorkflowManager, private readonly credentialsRepository: CredentialsRepository, private readonly projectRepository: ProjectRepository, + private readonly projectRelationRepository: ProjectRelationRepository, private readonly tagRepository: TagRepository, private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly sharedCredentialsRepository: SharedCredentialsRepository, @@ -659,7 +661,7 @@ export class SourceControlImportService { // as project creation might cause constraint issues. // We must iterate over the array and run the whole process workflow by workflow for (const candidate of candidates) { - this.logger.debug(`Parsing workflow file ${candidate.file}`); + this.logger.debug(`Importing workflow file ${candidate.file}`); const importedWorkflow = await this.parseWorkflowFromFile(candidate.file); @@ -1067,8 +1069,14 @@ export class SourceControlImportService { * Only team projects are supported. * Personal project are not supported because they are not stable across instances * (different ids across instances). + * + * @param candidates - The project files to import + * @param pullingUserId - The ID of the user pulling the changes (will be assigned as project admin for new projects) */ - async importTeamProjectsFromWorkFolder(candidates: SourceControlledFile[]) { + async importTeamProjectsFromWorkFolder( + candidates: SourceControlledFile[], + pullingUserId: string, + ) { const importResults = []; const existingProjectVariables = (await this.variablesService.getAllCached()).filter( (v) => v.project, @@ -1104,6 +1112,27 @@ export class SourceControlImportService { ['id'], ); + const existingProject = await this.projectRepository.findOne({ + where: { id: project.id }, + }); + + // For newly created projects OR existing projects without an admin, + // assign the pulling user as project admin + const hasExistingAdmin = + existingProject && + (await this.projectRelationRepository.findOne({ + where: { projectId: project.id, role: { slug: PROJECT_ADMIN_ROLE_SLUG } }, + })); + + if (!hasExistingAdmin) { + await this.projectRelationRepository.save({ + projectId: project.id, + userId: pullingUserId, + role: { slug: PROJECT_ADMIN_ROLE_SLUG }, + }); + this.logger.debug(`Assigned user ${pullingUserId} as admin for project ${project.name}`); + } + await this.importVariables( project.variableStubs?.map((v) => ({ ...v, projectId: project.id })) ?? [], ); 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 15df47c3876..749668b9945 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 @@ -420,7 +420,10 @@ export class SourceControlService { // 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); + await this.sourceControlImportService.importTeamProjectsFromWorkFolder( + projectsToBeImported, + user.id, + ); const foldersToBeImported = getNonDeletedResources(statusResult, 'folders')[0]; if (foldersToBeImported) { 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 67565a4dc82..792d0e4480a 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 @@ -91,6 +91,7 @@ describe('SourceControlImportService', () => { mock(), credentialsRepository, projectRepository, + mock(), tagRepository, sharedWorkflowRepository, sharedCredentialsRepository,