feat: Assign default project admin on pull (#23355)

This commit is contained in:
Irénée
2025-12-19 09:04:43 +00:00
committed by GitHub
parent 8ce21cb39b
commit d5c093411a
4 changed files with 144 additions and 7 deletions
@@ -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<WorkflowRepository>();
const folderRepository = mock<FolderRepository>();
const projectRepository = mock<ProjectRepository>();
const projectRelationRepository = mock<ProjectRelationRepository>();
const sharedWorkflowRepository = mock<SharedWorkflowRepository>();
const mockLogger = mock<Logger>();
const sourceControlScopedService = mock<SourceControlScopedService>();
@@ -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<SourceControlledFile>({ 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<ProjectRelation>({
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<SourceControlledFile>({ 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']);
@@ -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 })) ?? [],
);
@@ -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) {
@@ -91,6 +91,7 @@ describe('SourceControlImportService', () => {
mock(),
credentialsRepository,
projectRepository,
mock(),
tagRepository,
sharedWorkflowRepository,
sharedCredentialsRepository,