From c16c8d51d23d70da09a752d21e0864071f60b2f8 Mon Sep 17 00:00:00 2001 From: Avnyr Date: Thu, 15 May 2025 20:56:43 +0200 Subject: [PATCH] feat: add collaborator management to projects module Added endpoints to manage collaborators in `ProjectsController`: - Add collaborator - Remove collaborator - Get project collaborators Updated `ProjectsService` with corresponding methods and enhanced `checkUserAccess` to validate user access as owner or collaborator. Included unit tests for new functionality in controllers and services. --- .../controllers/projects.controller.spec.ts | 50 ++++- .../controllers/projects.controller.ts | 28 ++- .../services/projects.service.spec.ts | 190 +++++++++++++++++- .../projects/services/projects.service.ts | 116 ++++++++++- 4 files changed, 367 insertions(+), 17 deletions(-) diff --git a/backend/src/modules/projects/controllers/projects.controller.spec.ts b/backend/src/modules/projects/controllers/projects.controller.spec.ts index 4e3d053..a7a5495 100644 --- a/backend/src/modules/projects/controllers/projects.controller.spec.ts +++ b/backend/src/modules/projects/controllers/projects.controller.spec.ts @@ -20,6 +20,21 @@ describe('ProjectsController', () => { updatedAt: new Date(), }; + const mockUser = { + id: 'user2', + name: 'Test User', + githubId: '12345', + createdAt: new Date(), + updatedAt: new Date(), + }; + + const mockCollaboration = { + id: 'collab1', + projectId: 'project1', + userId: 'user2', + createdAt: new Date(), + }; + beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ controllers: [ProjectsController], @@ -34,6 +49,9 @@ describe('ProjectsController', () => { update: jest.fn().mockResolvedValue(mockProject), remove: jest.fn().mockResolvedValue(mockProject), checkUserAccess: jest.fn().mockResolvedValue(true), + addCollaborator: jest.fn().mockResolvedValue(mockCollaboration), + removeCollaborator: jest.fn().mockResolvedValue(mockCollaboration), + getCollaborators: jest.fn().mockResolvedValue([{ user: mockUser }]), }, }, ], @@ -113,4 +131,34 @@ describe('ProjectsController', () => { expect(service.checkUserAccess).toHaveBeenCalledWith(projectId, userId); }); }); -}); \ No newline at end of file + + describe('addCollaborator', () => { + it('should add a collaborator to a project', async () => { + const projectId = 'project1'; + const userId = 'user2'; + + expect(await controller.addCollaborator(projectId, userId)).toBe(mockCollaboration); + expect(service.addCollaborator).toHaveBeenCalledWith(projectId, userId); + }); + }); + + describe('removeCollaborator', () => { + it('should remove a collaborator from a project', async () => { + const projectId = 'project1'; + const userId = 'user2'; + + expect(await controller.removeCollaborator(projectId, userId)).toBe(mockCollaboration); + expect(service.removeCollaborator).toHaveBeenCalledWith(projectId, userId); + }); + }); + + describe('getCollaborators', () => { + it('should get all collaborators for a project', async () => { + const projectId = 'project1'; + const mockCollaborators = [{ user: mockUser }]; + + expect(await controller.getCollaborators(projectId)).toEqual(mockCollaborators); + expect(service.getCollaborators).toHaveBeenCalledWith(projectId); + }); + }); +}); diff --git a/backend/src/modules/projects/controllers/projects.controller.ts b/backend/src/modules/projects/controllers/projects.controller.ts index bb36b13..e4a3479 100644 --- a/backend/src/modules/projects/controllers/projects.controller.ts +++ b/backend/src/modules/projects/controllers/projects.controller.ts @@ -70,4 +70,30 @@ export class ProjectsController { checkUserAccess(@Param('id') id: string, @Param('userId') userId: string) { return this.projectsService.checkUserAccess(id, userId); } -} \ No newline at end of file + + /** + * Add a collaborator to a project + */ + @Post(':id/collaborators/:userId') + @HttpCode(HttpStatus.CREATED) + addCollaborator(@Param('id') id: string, @Param('userId') userId: string) { + return this.projectsService.addCollaborator(id, userId); + } + + /** + * Remove a collaborator from a project + */ + @Delete(':id/collaborators/:userId') + @HttpCode(HttpStatus.NO_CONTENT) + removeCollaborator(@Param('id') id: string, @Param('userId') userId: string) { + return this.projectsService.removeCollaborator(id, userId); + } + + /** + * Get all collaborators for a project + */ + @Get(':id/collaborators') + getCollaborators(@Param('id') id: string) { + return this.projectsService.getCollaborators(id); + } +} diff --git a/backend/src/modules/projects/services/projects.service.spec.ts b/backend/src/modules/projects/services/projects.service.spec.ts index a729865..43d0513 100644 --- a/backend/src/modules/projects/services/projects.service.spec.ts +++ b/backend/src/modules/projects/services/projects.service.spec.ts @@ -18,6 +18,21 @@ describe('ProjectsService', () => { updatedAt: new Date(), }; + const mockUser = { + id: 'user2', + name: 'Test User', + githubId: '12345', + createdAt: new Date(), + updatedAt: new Date(), + }; + + const mockCollaboration = { + id: 'collab1', + projectId: 'project1', + userId: 'user2', + createdAt: new Date(), + }; + // Mock database operations const mockDbOperations = { select: jest.fn().mockReturnThis(), @@ -28,6 +43,7 @@ describe('ProjectsService', () => { update: jest.fn().mockReturnThis(), set: jest.fn().mockReturnThis(), delete: jest.fn().mockReturnThis(), + innerJoin: jest.fn().mockReturnThis(), returning: jest.fn().mockImplementation(() => { return [mockProject]; }), @@ -182,10 +198,11 @@ describe('ProjectsService', () => { }); describe('checkUserAccess', () => { - it('should return true if user has access to project', async () => { + it('should return true if user is the owner of the project', async () => { const projectId = 'project1'; const userId = 'user1'; + // Mock owner check mockDb.select.mockImplementationOnce(() => mockDbOperations); mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); mockDbOperations.where.mockImplementationOnce(() => [mockProject]); @@ -198,20 +215,181 @@ describe('ProjectsService', () => { expect(result).toBe(true); }); - it('should return false if user does not have access to project', async () => { + it('should return true if user is a collaborator on the project', async () => { const projectId = 'project1'; const userId = 'user2'; + // Mock owner check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => []); + + // Mock collaborator check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => [mockCollaboration]); + + const result = await service.checkUserAccess(projectId, userId); + + expect(mockDb.select).toHaveBeenCalledTimes(2); + expect(mockDb.from).toHaveBeenCalledTimes(2); + expect(mockDb.where).toHaveBeenCalledTimes(2); + expect(result).toBe(true); + }); + + it('should return false if user does not have access to project', async () => { + const projectId = 'project1'; + const userId = 'user3'; + + // Mock owner check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => []); + + // Mock collaborator check mockDb.select.mockImplementationOnce(() => mockDbOperations); mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); mockDbOperations.where.mockImplementationOnce(() => []); const result = await service.checkUserAccess(projectId, userId); - expect(mockDb.select).toHaveBeenCalled(); - expect(mockDb.from).toHaveBeenCalled(); - expect(mockDb.where).toHaveBeenCalled(); + expect(mockDb.select).toHaveBeenCalledTimes(2); + expect(mockDb.from).toHaveBeenCalledTimes(2); + expect(mockDb.where).toHaveBeenCalledTimes(2); expect(result).toBe(false); }); }); -}); \ No newline at end of file + + describe('addCollaborator', () => { + it('should add a collaborator to a project', async () => { + const projectId = 'project1'; + const userId = 'user2'; + + // Mock findById + jest.spyOn(service, 'findById').mockResolvedValueOnce(mockProject); + + // Mock user check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => [mockUser]); + + // Mock relation check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => []); + + // Mock insert + mockDb.insert.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.values.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.returning.mockImplementationOnce(() => [mockCollaboration]); + + const result = await service.addCollaborator(projectId, userId); + + expect(service.findById).toHaveBeenCalledWith(projectId); + expect(mockDb.select).toHaveBeenCalledTimes(2); + expect(mockDb.from).toHaveBeenCalledTimes(2); + expect(mockDb.where).toHaveBeenCalledTimes(2); + expect(mockDb.insert).toHaveBeenCalled(); + expect(mockDb.values).toHaveBeenCalledWith({ + projectId, + userId, + }); + expect(result).toEqual(mockCollaboration); + }); + + it('should return existing collaboration if user is already a collaborator', async () => { + const projectId = 'project1'; + const userId = 'user2'; + + // Mock findById + jest.spyOn(service, 'findById').mockResolvedValueOnce(mockProject); + + // Mock user check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => [mockUser]); + + // Mock relation check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => [mockCollaboration]); + + const result = await service.addCollaborator(projectId, userId); + + expect(service.findById).toHaveBeenCalledWith(projectId); + expect(mockDb.select).toHaveBeenCalledTimes(2); + expect(mockDb.from).toHaveBeenCalledTimes(2); + expect(mockDb.where).toHaveBeenCalledTimes(2); + expect(mockDb.insert).not.toHaveBeenCalled(); + expect(result).toEqual(mockCollaboration); + }); + + it('should throw NotFoundException if user not found', async () => { + const projectId = 'project1'; + const userId = 'nonexistent'; + + // Mock findById + jest.spyOn(service, 'findById').mockResolvedValueOnce(mockProject); + + // Mock user check + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => []); + + await expect(service.addCollaborator(projectId, userId)).rejects.toThrow(NotFoundException); + }); + }); + + describe('removeCollaborator', () => { + it('should remove a collaborator from a project', async () => { + const projectId = 'project1'; + const userId = 'user2'; + + mockDb.delete.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.returning.mockImplementationOnce(() => [mockCollaboration]); + + const result = await service.removeCollaborator(projectId, userId); + + expect(mockDb.delete).toHaveBeenCalled(); + expect(mockDb.where).toHaveBeenCalled(); + expect(result).toEqual(mockCollaboration); + }); + + it('should throw NotFoundException if collaboration not found', async () => { + const projectId = 'project1'; + const userId = 'nonexistent'; + + mockDb.delete.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.returning.mockImplementationOnce(() => []); + + await expect(service.removeCollaborator(projectId, userId)).rejects.toThrow(NotFoundException); + }); + }); + + describe('getCollaborators', () => { + it('should get all collaborators for a project', async () => { + const projectId = 'project1'; + const mockCollaborators = [{ user: mockUser }]; + + // Mock findById + jest.spyOn(service, 'findById').mockResolvedValueOnce(mockProject); + + // Mock get collaborators + mockDb.select.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.from.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.innerJoin.mockImplementationOnce(() => mockDbOperations); + mockDbOperations.where.mockImplementationOnce(() => mockCollaborators); + + const result = await service.getCollaborators(projectId); + + expect(service.findById).toHaveBeenCalledWith(projectId); + expect(mockDb.select).toHaveBeenCalled(); + expect(mockDb.from).toHaveBeenCalled(); + expect(mockDb.innerJoin).toHaveBeenCalled(); + expect(mockDb.where).toHaveBeenCalled(); + }); + }); +}); diff --git a/backend/src/modules/projects/services/projects.service.ts b/backend/src/modules/projects/services/projects.service.ts index 0cbd5b1..0b0f3e2 100644 --- a/backend/src/modules/projects/services/projects.service.ts +++ b/backend/src/modules/projects/services/projects.service.ts @@ -45,11 +45,11 @@ export class ProjectsService { .select() .from(schema.projects) .where(eq(schema.projects.id, id)); - + if (!project) { throw new NotFoundException(`Project with ID ${id} not found`); } - + return project; } @@ -65,11 +65,11 @@ export class ProjectsService { }) .where(eq(schema.projects.id, id)) .returning(); - + if (!project) { throw new NotFoundException(`Project with ID ${id} not found`); } - + return project; } @@ -81,11 +81,11 @@ export class ProjectsService { .delete(schema.projects) .where(eq(schema.projects.id, id)) .returning(); - + if (!project) { throw new NotFoundException(`Project with ID ${id} not found`); } - + return project; } @@ -93,6 +93,7 @@ export class ProjectsService { * Check if a user has access to a project */ async checkUserAccess(projectId: string, userId: string) { + // Check if the user is the owner of the project const [project] = await this.db .select() .from(schema.projects) @@ -102,7 +103,104 @@ export class ProjectsService { eq(schema.projects.ownerId, userId) ) ); - - return !!project; + + if (project) { + return true; + } + + // Check if the user is a collaborator on the project + const [collaboration] = await this.db + .select() + .from(schema.projectCollaborators) + .where( + and( + eq(schema.projectCollaborators.projectId, projectId), + eq(schema.projectCollaborators.userId, userId) + ) + ); + + return !!collaboration; } -} \ No newline at end of file + + /** + * Add a collaborator to a project + */ + async addCollaborator(projectId: string, userId: string) { + // Check if the project exists + await this.findById(projectId); + + // Check if the user exists + const [user] = await this.db + .select() + .from(schema.users) + .where(eq(schema.users.id, userId)); + + if (!user) { + throw new NotFoundException(`User with ID ${userId} not found`); + } + + // Check if the user is already a collaborator on the project + const [existingCollaboration] = await this.db + .select() + .from(schema.projectCollaborators) + .where( + and( + eq(schema.projectCollaborators.projectId, projectId), + eq(schema.projectCollaborators.userId, userId) + ) + ); + + if (existingCollaboration) { + return existingCollaboration; + } + + // Add the user as a collaborator on the project + const [collaboration] = await this.db + .insert(schema.projectCollaborators) + .values({ + projectId, + userId, + }) + .returning(); + + return collaboration; + } + + /** + * Remove a collaborator from a project + */ + async removeCollaborator(projectId: string, userId: string) { + const [collaboration] = await this.db + .delete(schema.projectCollaborators) + .where( + and( + eq(schema.projectCollaborators.projectId, projectId), + eq(schema.projectCollaborators.userId, userId) + ) + ) + .returning(); + + if (!collaboration) { + throw new NotFoundException(`User with ID ${userId} is not a collaborator on project with ID ${projectId}`); + } + + return collaboration; + } + + /** + * Get all collaborators for a project + */ + async getCollaborators(projectId: string) { + // Check if the project exists + await this.findById(projectId); + + // Get all collaborators for the project + return this.db + .select({ + user: schema.users, + }) + .from(schema.projectCollaborators) + .innerJoin(schema.users, eq(schema.projectCollaborators.userId, schema.users.id)) + .where(eq(schema.projectCollaborators.projectId, projectId)); + } +}