From f7f5f5e95c474f2834fbb71a5906e7b3115e0e7a Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Wed, 26 Feb 2025 07:01:22 -0500 Subject: [PATCH] feat(core): Add endpoint PATCH `/projects/:projectId/folders/:folderId` (no-changelog) (#13454) --- .../update-folder.request.dto.test.ts | 42 ++++++ .../src/dto/folders/create-folder.dto.ts | 4 +- .../src/dto/folders/update-folder.dto.ts | 7 + packages/@n8n/api-types/src/dto/index.ts | 1 + .../api-types/src/schemas/folder.schema.ts | 3 + packages/@n8n/permissions/src/constants.ee.ts | 2 +- .../cli/src/controllers/folder.controller.ts | 23 +++- .../cli/src/permissions.ee/project-roles.ts | 3 + packages/cli/src/services/folder.service.ts | 7 +- .../folder/folder.controller.test.ts | 130 ++++++++++++++++++ 10 files changed, 217 insertions(+), 5 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/folders/__tests__/update-folder.request.dto.test.ts create mode 100644 packages/@n8n/api-types/src/dto/folders/update-folder.dto.ts create mode 100644 packages/@n8n/api-types/src/schemas/folder.schema.ts diff --git a/packages/@n8n/api-types/src/dto/folders/__tests__/update-folder.request.dto.test.ts b/packages/@n8n/api-types/src/dto/folders/__tests__/update-folder.request.dto.test.ts new file mode 100644 index 0000000000..23213ecbe9 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/folders/__tests__/update-folder.request.dto.test.ts @@ -0,0 +1,42 @@ +import { UpdateFolderDto } from '../update-folder.dto'; + +describe('UpdateFolderDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'name without parentId', + request: { + name: 'test', + }, + }, + ])('should validate $name', ({ request }) => { + const result = UpdateFolderDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'missing name', + request: {}, + expectedErrorPath: ['name'], + }, + { + name: 'empty name', + request: { + name: '', + }, + expectedErrorPath: ['name'], + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = UpdateFolderDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/folders/create-folder.dto.ts b/packages/@n8n/api-types/src/dto/folders/create-folder.dto.ts index d0c59eaf54..098292045a 100644 --- a/packages/@n8n/api-types/src/dto/folders/create-folder.dto.ts +++ b/packages/@n8n/api-types/src/dto/folders/create-folder.dto.ts @@ -1,7 +1,9 @@ import { z } from 'zod'; import { Z } from 'zod-class'; +import { folderNameSchema } from '../../schemas/folder.schema'; + export class CreateFolderDto extends Z.class({ - name: z.string().trim().min(1).max(128), + name: folderNameSchema, parentFolderId: z.string().optional(), }) {} diff --git a/packages/@n8n/api-types/src/dto/folders/update-folder.dto.ts b/packages/@n8n/api-types/src/dto/folders/update-folder.dto.ts new file mode 100644 index 0000000000..13d6c03203 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/folders/update-folder.dto.ts @@ -0,0 +1,7 @@ +import { Z } from 'zod-class'; + +import { folderNameSchema } from '../../schemas/folder.schema'; + +export class UpdateFolderDto extends Z.class({ + name: folderNameSchema, +}) {} diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index dd3fd20fac..a4467d65c6 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -56,3 +56,4 @@ export { UpdateApiKeyRequestDto } from './api-keys/update-api-key-request.dto'; export { CreateApiKeyRequestDto } from './api-keys/create-api-key-request.dto'; export { CreateFolderDto } from './folders/create-folder.dto'; +export { UpdateFolderDto } from './folders/update-folder.dto'; diff --git a/packages/@n8n/api-types/src/schemas/folder.schema.ts b/packages/@n8n/api-types/src/schemas/folder.schema.ts new file mode 100644 index 0000000000..40b59d4faa --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/folder.schema.ts @@ -0,0 +1,3 @@ +import { z } from 'zod'; + +export const folderNameSchema = z.string().trim().min(1).max(128); diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index eda49a605b..19b39df8fc 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -22,5 +22,5 @@ export const RESOURCES = { variable: [...DEFAULT_OPERATIONS] as const, workersView: ['manage'] as const, workflow: ['share', 'execute', 'move', ...DEFAULT_OPERATIONS] as const, - folder: ['create', 'read'] as const, + folder: ['create', 'read', 'update'] as const, } as const; diff --git a/packages/cli/src/controllers/folder.controller.ts b/packages/cli/src/controllers/folder.controller.ts index 4ae355180a..17fb4ddb7e 100644 --- a/packages/cli/src/controllers/folder.controller.ts +++ b/packages/cli/src/controllers/folder.controller.ts @@ -1,7 +1,7 @@ -import { CreateFolderDto } from '@n8n/api-types'; +import { CreateFolderDto, UpdateFolderDto } from '@n8n/api-types'; import { Response } from 'express'; -import { Post, RestController, ProjectScope, Body, Get } from '@/decorators'; +import { Post, RestController, ProjectScope, Body, Get, Patch } from '@/decorators'; import { FolderNotFoundError } from '@/errors/folder-not-found.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -48,4 +48,23 @@ export class ProjectController { throw new InternalServerError(); } } + + @Patch('/:folderId') + @ProjectScope('folder:update') + async updateFolder( + req: AuthenticatedRequest<{ projectId: string; folderId: string }>, + _res: Response, + @Body payload: UpdateFolderDto, + ) { + const { projectId, folderId } = req.params; + + try { + await this.folderService.updateFolder(folderId, projectId, payload); + } catch (e) { + if (e instanceof FolderNotFoundError) { + throw new NotFoundError(e.message); + } + throw new InternalServerError(); + } + } } diff --git a/packages/cli/src/permissions.ee/project-roles.ts b/packages/cli/src/permissions.ee/project-roles.ts index 5e3c41ac46..cd5f285e2f 100644 --- a/packages/cli/src/permissions.ee/project-roles.ts +++ b/packages/cli/src/permissions.ee/project-roles.ts @@ -27,6 +27,7 @@ export const REGULAR_PROJECT_ADMIN_SCOPES: Scope[] = [ 'project:delete', 'folder:create', 'folder:read', + 'folder:update', ]; export const PERSONAL_PROJECT_OWNER_SCOPES: Scope[] = [ @@ -49,6 +50,7 @@ export const PERSONAL_PROJECT_OWNER_SCOPES: Scope[] = [ 'project:read', 'folder:create', 'folder:read', + 'folder:update', ]; export const PROJECT_EDITOR_SCOPES: Scope[] = [ @@ -67,6 +69,7 @@ export const PROJECT_EDITOR_SCOPES: Scope[] = [ 'project:read', 'folder:create', 'folder:read', + 'folder:update', ]; export const PROJECT_VIEWER_SCOPES: Scope[] = [ diff --git a/packages/cli/src/services/folder.service.ts b/packages/cli/src/services/folder.service.ts index 54ecbe04d9..0b135360a9 100644 --- a/packages/cli/src/services/folder.service.ts +++ b/packages/cli/src/services/folder.service.ts @@ -1,4 +1,4 @@ -import type { CreateFolderDto } from '@n8n/api-types'; +import type { CreateFolderDto, UpdateFolderDto } from '@n8n/api-types'; import { Service } from '@n8n/di'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import type { EntityManager } from '@n8n/typeorm'; @@ -39,6 +39,11 @@ export class FolderService { return folder; } + async updateFolder(folderId: string, projectId: string, { name }: UpdateFolderDto) { + await this.getFolderInProject(folderId, projectId); + return await this.folderRepository.update({ id: folderId }, { name }); + } + async getFolderInProject(folderId: string, projectId: string, em?: EntityManager) { try { return await this.folderRepository.findOneOrFailFolderInProject(folderId, projectId, em); diff --git a/packages/cli/test/integration/folder/folder.controller.test.ts b/packages/cli/test/integration/folder/folder.controller.test.ts index 5981750659..bd7955d51f 100644 --- a/packages/cli/test/integration/folder/folder.controller.test.ts +++ b/packages/cli/test/integration/folder/folder.controller.test.ts @@ -279,3 +279,133 @@ describe('GET /projects/:projectId/folders/:folderId/tree', () => { ); }); }); + +describe('PATCH /projects/:projectId/folders/:folderId', () => { + test('should not update folder when project does not exist', async () => { + const payload = { + name: 'Updated Folder Name', + }; + + await authOwnerAgent + .patch('/projects/non-existing-id/folders/some-folder-id') + .send(payload) + .expect(403); + }); + + test('should not update folder when folder does not exist', async () => { + const project = await createTeamProject('test project', owner); + + const payload = { + name: 'Updated Folder Name', + }; + + await authOwnerAgent + .patch(`/projects/${project.id}/folders/non-existing-folder`) + .send(payload) + .expect(404); + }); + + test('should not update folder when name is empty', async () => { + const project = await createTeamProject(undefined, owner); + const folder = await createFolder(project, { name: 'Original Name' }); + + const payload = { + name: '', + }; + + await authOwnerAgent + .patch(`/projects/${project.id}/folders/${folder.id}`) + .send(payload) + .expect(400); + + const folderInDb = await folderRepository.findOneBy({ id: folder.id }); + expect(folderInDb?.name).toBe('Original Name'); + }); + + test('should not update folder if user has project:viewer role in team project', async () => { + const project = await createTeamProject(undefined, owner); + const folder = await createFolder(project, { name: 'Original Name' }); + await linkUserToProject(member, project, 'project:viewer'); + + const payload = { + name: 'Updated Folder Name', + }; + + await authMemberAgent + .patch(`/projects/${project.id}/folders/${folder.id}`) + .send(payload) + .expect(403); + + const folderInDb = await folderRepository.findOneBy({ id: folder.id }); + expect(folderInDb?.name).toBe('Original Name'); + }); + + test("should not allow updating folder in another user's personal project", async () => { + const ownerPersonalProject = await projectRepository.getPersonalProjectForUserOrFail(owner.id); + const folder = await createFolder(ownerPersonalProject, { name: 'Original Name' }); + + const payload = { + name: 'Updated Folder Name', + }; + + await authMemberAgent + .patch(`/projects/${ownerPersonalProject.id}/folders/${folder.id}`) + .send(payload) + .expect(403); + + const folderInDb = await folderRepository.findOneBy({ id: folder.id }); + expect(folderInDb?.name).toBe('Original Name'); + }); + + test('should update folder if user has project:editor role in team project', async () => { + const project = await createTeamProject(undefined, owner); + const folder = await createFolder(project, { name: 'Original Name' }); + await linkUserToProject(member, project, 'project:editor'); + + const payload = { + name: 'Updated Folder Name', + }; + + await authMemberAgent + .patch(`/projects/${project.id}/folders/${folder.id}`) + .send(payload) + .expect(200); + + const folderInDb = await folderRepository.findOneBy({ id: folder.id }); + expect(folderInDb?.name).toBe('Updated Folder Name'); + }); + + test('should update folder if user has project:admin role in team project', async () => { + const project = await createTeamProject(undefined, owner); + const folder = await createFolder(project, { name: 'Original Name' }); + + const payload = { + name: 'Updated Folder Name', + }; + + await authOwnerAgent + .patch(`/projects/${project.id}/folders/${folder.id}`) + .send(payload) + .expect(200); + + const folderInDb = await folderRepository.findOneBy({ id: folder.id }); + expect(folderInDb?.name).toBe('Updated Folder Name'); + }); + + test('should update folder in personal project', async () => { + const personalProject = await projectRepository.getPersonalProjectForUserOrFail(owner.id); + const folder = await createFolder(personalProject, { name: 'Original Name' }); + + const payload = { + name: 'Updated Folder Name', + }; + + await authOwnerAgent + .patch(`/projects/${personalProject.id}/folders/${folder.id}`) + .send(payload) + .expect(200); + + const folderInDb = await folderRepository.findOneBy({ id: folder.id }); + expect(folderInDb?.name).toBe('Updated Folder Name'); + }); +});