diff --git a/backend/src/common/exceptions/custom-exceptions/last-admin-delete.exception.ts b/backend/src/common/exceptions/custom-exceptions/last-admin-delete.exception.ts new file mode 100644 index 00000000..cd32a435 --- /dev/null +++ b/backend/src/common/exceptions/custom-exceptions/last-admin-delete.exception.ts @@ -0,0 +1,8 @@ +import { ForbiddenException } from '@nestjs/common'; + +export class LastAdminDeleteException extends ForbiddenException { + constructor() { + super('Cannot delete the last admin in the system'); + this.name = 'LastAdminDeleteException'; + } +} diff --git a/backend/src/common/exceptions/custom-exceptions/user-delete-forbidden.exception.ts b/backend/src/common/exceptions/custom-exceptions/user-delete-forbidden.exception.ts new file mode 100644 index 00000000..2994e476 --- /dev/null +++ b/backend/src/common/exceptions/custom-exceptions/user-delete-forbidden.exception.ts @@ -0,0 +1,8 @@ +import { ForbiddenException } from '@nestjs/common'; + +export class UserDeleteForbiddenException extends ForbiddenException { + constructor() { + super('You can only delete your own account'); + this.name = 'UserDeleteForbiddenException'; + } +} diff --git a/backend/src/modules/users/dto/delete-user-success.dto.ts b/backend/src/modules/users/dto/delete-user-success.dto.ts new file mode 100644 index 00000000..948da00d --- /dev/null +++ b/backend/src/modules/users/dto/delete-user-success.dto.ts @@ -0,0 +1,15 @@ +import { ApiProperty } from '@nestjs/swagger'; + +export class DeleteUserSuccessDto { + @ApiProperty({ + description: 'Indicates if the operation was successful', + example: true, + }) + success: boolean; + + @ApiProperty({ + description: 'Success message with user email', + example: 'User john.doe@fhstp.ac.at has been deleted successfully', + }) + message: string; +} diff --git a/backend/src/modules/users/types/user-operations.types.ts b/backend/src/modules/users/types/user-operations.types.ts new file mode 100644 index 00000000..fd66ec0e --- /dev/null +++ b/backend/src/modules/users/types/user-operations.types.ts @@ -0,0 +1,27 @@ +import { Role } from '@prisma/client'; + +/** + * Type definition for user deletion operation parameters + * + * Provides type safety for the complex user deletion process + * that involves multiple related entities and database operations. + */ +export interface DeleteUserOperation { + userId: string; + userEmail: string; + userRole: Role; + studentId?: string; + supervisorId?: string; +} + +/** + * Type definition for user deletion operation results + * + * Contains statistics about what was deleted/modified during + * the user deletion process for logging and audit purposes. + */ +export interface DeleteUserResult { + deletedTagsCount: number; + deletedBlocksCount: number; + profileUpdated: boolean; +} diff --git a/backend/src/modules/users/users.controller.spec.ts b/backend/src/modules/users/users.controller.spec.ts index fc2e0efd..90f4d5d7 100644 --- a/backend/src/modules/users/users.controller.spec.ts +++ b/backend/src/modules/users/users.controller.spec.ts @@ -36,6 +36,8 @@ describe('UsersController', () => { const USER_UUID = '123e4567-e89b-12d3-a456-426614174000'; const USER_UUID_2 = '123e4567-e89b-12d3-a456-426614174001'; + const USER_UUID_3 = '123e4567-e89b-12d3-a456-426614174003'; + const ADMIN_UUID = '123e4567-e89b-12d3-a456-426614174002'; const TAG_UUID_1 = 'f47ac10b-58cc-4372-a567-0e02b2c3d479'; const TAG_UUID_2 = 'a1b2c3d4-e5f6-7890-1234-567890abcdef'; const CLERK_ID = 'user_2NUj8tGhSFhTLD9sdP0q4P7VoJM'; @@ -56,7 +58,7 @@ describe('UsersController', () => { const mockAdmin: User = { ...mockUser, - id: 'admin-id', + id: ADMIN_UUID, email: 'admin@fhstp.ac.at', role: Role.ADMIN, }; @@ -235,9 +237,16 @@ describe('UsersController', () => { describe('deleteUser', () => { it('should delete a user', async () => { - mockUsersService.deleteUser.mockResolvedValue(mockUser); - await controller.deleteUser(USER_UUID); - expect(mockUsersService.deleteUser).toHaveBeenCalledWith(USER_UUID); + const mockDeleteResult = { + success: true, + message: 'User has been deleted successfully', + }; + mockUsersService.deleteUser.mockResolvedValue(mockDeleteResult); + + const result = await controller.deleteUser(USER_UUID, mockUser); + + expect(result).toEqual(mockDeleteResult); + expect(mockUsersService.deleteUser).toHaveBeenCalledWith(USER_UUID, mockUser); }); }); @@ -340,12 +349,12 @@ describe('UsersController', () => { it("should throw UnauthorizedException when a student tries to view another student's blocked supervisors", async () => { // Arrange - const otherStudentId = 'other-student-id'; // This is different from mockUser.id (USER_UUID) + const otherStudentUserId = USER_UUID_3; const currentUser = mockUser; // Student with id USER_UUID try { // Act - await controller.findBlockedSupervisorsByStudentUserId(otherStudentId, currentUser); + await controller.findBlockedSupervisorsByStudentUserId(otherStudentUserId, currentUser); // If we get here, fail the test fail('Expected an UnauthorizedException to be thrown'); } catch (error) { @@ -399,13 +408,13 @@ describe('UsersController', () => { it('should throw UnauthorizedException when a student tries to block a supervisor on behalf of another student', async () => { // Arrange - const otherStudentId = 'other-student-id'; // This is different from mockUser.id (USER_UUID) + const otherStudentUserId = USER_UUID_3; const dto: CreateUserBlockDto = { blocked_id: USER_UUID_2 }; const currentUser = mockUser; // Student with id USER_UUID try { // Act - await controller.createUserBlock(otherStudentId, dto, currentUser); + await controller.createUserBlock(otherStudentUserId, dto, currentUser); // If we get here, fail the test fail('Expected an UnauthorizedException to be thrown'); } catch (error) { @@ -464,13 +473,13 @@ describe('UsersController', () => { it('should throw UnauthorizedException when a student tries to unblock a supervisor on behalf of another student', async () => { // Arrange - const otherStudentId = 'other-student-id'; + const otherStudentUserId = USER_UUID_3; const supervisorUserId = USER_UUID_2; const currentUser = mockUser; // Student // Act & Assert await expect( - controller.removeUserBlock(otherStudentId, supervisorUserId, currentUser), + controller.removeUserBlock(otherStudentUserId, supervisorUserId, currentUser), ).rejects.toThrow(UnauthorizedException); expect(mockUsersService.deleteUserBlock).not.toHaveBeenCalled(); }); diff --git a/backend/src/modules/users/users.controller.ts b/backend/src/modules/users/users.controller.ts index c90b94d9..13308ce6 100644 --- a/backend/src/modules/users/users.controller.ts +++ b/backend/src/modules/users/users.controller.ts @@ -19,6 +19,7 @@ import { import { UsersService } from './users.service'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; +import { DeleteUserSuccessDto } from './dto/delete-user-success.dto'; import { ApiTags, ApiOperation, @@ -396,12 +397,11 @@ export class UsersController { } @Delete(':id') - @Roles(Role.ADMIN) - @HttpCode(HttpStatus.NO_CONTENT) + @HttpCode(HttpStatus.OK) @ApiOperation({ summary: 'Delete user (Soft Delete)', description: - 'Soft delete a user. Preserves data but marks user as deleted and they will no longer appear in regular queries.', + 'Soft delete a user. Users can delete their own accounts, admins can delete any user. The last admin cannot delete themselves.', }) @ApiParam({ name: 'id', @@ -411,11 +411,22 @@ export class UsersController { required: true, example: '123e4567-e89b-12d3-a456-426614174000', }) - @ApiResponse({ status: 204, description: 'User has been successfully deleted.' }) + @ApiResponse({ + status: 200, + description: 'User has been successfully deleted.', + type: DeleteUserSuccessDto, + }) @ApiResponse({ status: 404, description: 'User not found.' }) @ApiResponse({ status: 400, description: 'Bad request - Invalid User ID format.' }) - deleteUser(@Param('id', ParseUUIDPipe) id: string): Promise { - return this.usersService.deleteUser(id); + @ApiResponse({ + status: 403, + description: 'Forbidden - Not authorized to delete this user or last admin.', + }) + deleteUser( + @Param('id', ParseUUIDPipe) id: string, + @CurrentUser() currentUser: User, + ): Promise { + return this.usersService.deleteUser(id, currentUser); } /** diff --git a/backend/src/modules/users/users.repository.spec.ts b/backend/src/modules/users/users.repository.spec.ts index 2c037373..6fa3ca60 100644 --- a/backend/src/modules/users/users.repository.spec.ts +++ b/backend/src/modules/users/users.repository.spec.ts @@ -14,6 +14,7 @@ describe('UsersRepository', () => { findMany: jest.fn(), findUnique: jest.fn(), update: jest.fn(), + count: jest.fn(), }, userTag: { findMany: jest.fn(), @@ -28,6 +29,13 @@ describe('UsersRepository', () => { create: jest.fn(), delete: jest.fn(), findUnique: jest.fn(), + deleteMany: jest.fn(), + }, + student: { + update: jest.fn(), + }, + supervisor: { + update: jest.fn(), }, }; @@ -726,27 +734,6 @@ describe('UsersRepository', () => { }); }); - describe('softDeleteUser', () => { - it('should mark a user as deleted and return the updated entity', async () => { - // Arrange - const userId = USER_UUID; - const deletedUser = { ...mockUser, is_deleted: true }; - - mockPrismaService.user.update.mockResolvedValue(deletedUser); - - // Act - const result = await repository.softDeleteUser(userId); - - // Assert - expect(result).toEqual(deletedUser); - expect(mockPrismaService.user.update).toHaveBeenCalledWith({ - where: { id: userId }, - data: { is_deleted: true }, - }); - expect(mockPrismaService.user.update).toHaveBeenCalledTimes(1); - }); - }); - describe('findUserTagsByUserId', () => { it('should find all tags for a user', async () => { // Arrange @@ -803,15 +790,12 @@ describe('UsersRepository', () => { mockPrismaService.userTag.deleteMany.mockResolvedValue({ count: 1 }); mockPrismaService.userTag.createMany.mockResolvedValue({ count: 2 }); - mockPrismaService.$transaction.mockImplementation(async ops => { - // Safe access with type checking - if (Array.isArray(ops) && ops.length > 0) { - await ops[0]; - if (ops.length > 1) { - await ops[1]; - } - } + + // Mock $transaction to handle array of operations + mockPrismaService.$transaction.mockImplementation(operations => { + return Promise.all(operations); }); + mockPrismaService.userTag.findMany.mockResolvedValue(expectedNewTags); // Act @@ -839,8 +823,10 @@ describe('UsersRepository', () => { const tagsInput: Array<{ tag_id: string; priority: number }> = []; mockPrismaService.userTag.deleteMany.mockResolvedValue({ count: 1 }); mockPrismaService.userTag.findMany.mockResolvedValue([]); - mockPrismaService.$transaction.mockImplementation(async ops => { - await ops[0]; + + // Mock $transaction to handle array of operations + mockPrismaService.$transaction.mockImplementation(operations => { + return Promise.all(operations); }); // Act @@ -983,4 +969,184 @@ describe('UsersRepository', () => { }); }); }); + + describe('deleteUser', () => { + it('should delete a user using transaction', async () => { + // Arrange + const deleteData = { + userId: USER_UUID, + userEmail: 'test@example.com', + userRole: Role.STUDENT, + studentId: 'student-123', + supervisorId: undefined, + }; + + const txMock = { + user: { + update: jest.fn().mockResolvedValue({}), + }, + userTag: { + deleteMany: jest.fn().mockResolvedValue({ count: 2 }), + }, + userBlock: { + deleteMany: jest.fn().mockResolvedValue({ count: 1 }), + }, + student: { + update: jest.fn().mockResolvedValue({}), + }, + }; + + mockPrismaService.$transaction.mockImplementation( + (callback: (tx: typeof txMock) => unknown) => { + return callback(txMock); + }, + ); + + // Act + const result = await repository.deleteUser(deleteData); + + // Assert + expect(mockPrismaService.$transaction).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + deletedTagsCount: 2, + deletedBlocksCount: 1, + profileUpdated: true, + }); + }); + + it('should handle supervisor deletion correctly', async () => { + // Arrange + const deleteData = { + userId: USER_UUID, + userEmail: 'supervisor@example.com', + userRole: Role.SUPERVISOR, + studentId: undefined, + supervisorId: 'supervisor-123', + }; + + const txMock = { + user: { + update: jest.fn().mockResolvedValue({}), + }, + userTag: { + deleteMany: jest.fn().mockResolvedValue({ count: 0 }), + }, + userBlock: { + deleteMany: jest.fn().mockResolvedValue({ count: 0 }), + }, + supervisor: { + update: jest.fn().mockResolvedValue({}), + }, + }; + + mockPrismaService.$transaction.mockImplementation( + (callback: (tx: typeof txMock) => unknown) => { + return callback(txMock); + }, + ); + + // Act + const result = await repository.deleteUser(deleteData); + + // Assert + expect(txMock.supervisor.update).toHaveBeenCalledWith({ + where: { id: 'supervisor-123' }, + data: { + bio: '', + available_spots: 0, + total_spots: 0, + }, + }); + expect(result).toEqual({ + deletedTagsCount: 0, + deletedBlocksCount: 0, + profileUpdated: true, + }); + }); + }); + + describe('deleteAllUserTags', () => { + it('should delete all user tags and return count', async () => { + // Arrange + const userId = USER_UUID; + const deleteResult = { count: 3 }; + mockPrismaService.userTag.deleteMany.mockResolvedValue(deleteResult); + + // Act + const result = await repository.deleteAllUserTags(userId); + + // Assert + expect(result).toBe(3); + expect(mockPrismaService.userTag.deleteMany).toHaveBeenCalledWith({ + where: { user_id: userId }, + }); + }); + + it('should return 0 when no tags to delete', async () => { + // Arrange + const userId = USER_UUID; + const deleteResult = { count: 0 }; + mockPrismaService.userTag.deleteMany.mockResolvedValue(deleteResult); + + // Act + const result = await repository.deleteAllUserTags(userId); + + // Assert + expect(result).toBe(0); + expect(mockPrismaService.userTag.deleteMany).toHaveBeenCalledWith({ + where: { user_id: userId }, + }); + }); + }); + + describe('deleteAllUserBlocks', () => { + it('should delete all user blocks and return count', async () => { + // Arrange + const userId = USER_UUID; + const deleteResult = { count: 2 }; + mockPrismaService.userBlock.deleteMany.mockResolvedValue(deleteResult); + + // Act + const result = await repository.deleteAllUserBlocks(userId); + + // Assert + expect(result).toBe(2); + expect(mockPrismaService.userBlock.deleteMany).toHaveBeenCalledWith({ + where: { + OR: [{ blocker_id: userId }, { blocked_id: userId }], + }, + }); + }); + }); + + describe('countActiveAdmins', () => { + it('should count active admins correctly', async () => { + // Arrange + const expectedCount = 3; + mockPrismaService.user.count.mockResolvedValue(expectedCount); + + // Act + const result = await repository.countActiveAdmins(); + + // Assert + expect(result).toBe(expectedCount); + expect(mockPrismaService.user.count).toHaveBeenCalledWith({ + where: { + role: Role.ADMIN, + is_deleted: false, + }, + }); + }); + + it('should return 0 when no active admins exist', async () => { + // Arrange + mockPrismaService.user.count.mockResolvedValue(0); + + // Act + const result = await repository.countActiveAdmins(); + + // Assert + expect(result).toBe(0); + }); + }); }); diff --git a/backend/src/modules/users/users.repository.ts b/backend/src/modules/users/users.repository.ts index 68d091da..677b5b8d 100644 --- a/backend/src/modules/users/users.repository.ts +++ b/backend/src/modules/users/users.repository.ts @@ -1,6 +1,7 @@ import { Injectable } from '@nestjs/common'; import { PrismaService } from '../../prisma/prisma.service'; import { Prisma, User, UserTag, Role, UserBlock } from '@prisma/client'; +import { DeleteUserOperation, DeleteUserResult } from './types/user-operations.types'; export interface IUsersRepository { createUser(userData: { @@ -36,7 +37,7 @@ export interface IUsersRepository { clerk_id?: string | null; }, ): Promise; - softDeleteUser(id: string): Promise; + deleteUser(data: DeleteUserOperation): Promise; // User Tag operations findUserTagsByUserId(userId: string): Promise; @@ -44,12 +45,17 @@ export interface IUsersRepository { userId: string, tags: Array<{ tag_id: string; priority: number }>, ): Promise; + deleteAllUserTags(userId: string): Promise; // User Block operations findBlockedSupervisorsByStudentUserId(studentUserId: string): Promise; createUserBlock(blockerUserId: string, blockedUserId: string): Promise; deleteUserBlock(blockerUserId: string, blockedUserId: string): Promise; findUserBlockByIds(blockerUserId: string, blockedUserId: string): Promise; + deleteAllUserBlocks(userId: string): Promise; + + // Admin related operations + countActiveAdmins(): Promise; } @Injectable() @@ -71,6 +77,8 @@ export class UsersRepository implements IUsersRepository { } async searchUsers(searchQuery: string): Promise { + const USER_SEARCH_LIMIT = 15; // TODO [SCRUM-315]: Make this configurable + // If search query is empty, return empty array if (!searchQuery || searchQuery.trim() === '') { return []; @@ -130,7 +138,7 @@ export class UsersRepository implements IUsersRepository { }, }, }, - take: 15, // Limit to 15 results + take: USER_SEARCH_LIMIT, }); } @@ -260,13 +268,6 @@ export class UsersRepository implements IUsersRepository { }); } - async softDeleteUser(id: string): Promise { - return this.prisma.user.update({ - where: { id }, - data: { is_deleted: true }, - }); - } - // User Tag operations implementation async findUserTagsByUserId(userId: string): Promise { return this.prisma.userTag.findMany({ @@ -312,6 +313,13 @@ export class UsersRepository implements IUsersRepository { return this.findUserTagsByUserId(userId); } + async deleteAllUserTags(userId: string): Promise { + const result = await this.prisma.userTag.deleteMany({ + where: { user_id: userId }, + }); + return result.count; + } + // User Block operations async findBlockedSupervisorsByStudentUserId(studentUserId: string): Promise { return this.prisma.userBlock.findMany({ @@ -354,4 +362,77 @@ export class UsersRepository implements IUsersRepository { }, }); } + + async deleteAllUserBlocks(userId: string): Promise { + const result = await this.prisma.userBlock.deleteMany({ + where: { + OR: [{ blocker_id: userId }, { blocked_id: userId }], + }, + }); + return result.count; + } + + async deleteUser(data: DeleteUserOperation): Promise { + return await this.prisma.$transaction(async tx => { + // Update user fields + await tx.user.update({ + where: { id: data.userId }, + data: { + first_name: '', + last_name: '', + profile_image: null, + is_deleted: true, + is_registered: false, + // Keep: email, role, clerk_id unchanged + }, + }); + + // Delete user tags and capture count + const deletedTags = await tx.userTag.deleteMany({ + where: { user_id: data.userId }, + }); + + // Delete user blocks and capture count + const deletedBlocks = await tx.userBlock.deleteMany({ + where: { + OR: [{ blocker_id: data.userId }, { blocked_id: data.userId }], + }, + }); + + let profileUpdated = false; + + // Handle role-specific cleanup + if (data.userRole === Role.STUDENT && data.studentId) { + await tx.student.update({ + where: { id: data.studentId }, + data: { thesis_description: '' }, + }); + profileUpdated = true; + } else if (data.userRole === Role.SUPERVISOR && data.supervisorId) { + await tx.supervisor.update({ + where: { id: data.supervisorId }, + data: { + bio: '', + available_spots: 0, + total_spots: 0, + }, + }); + profileUpdated = true; + } + + return { + deletedTagsCount: deletedTags.count, + deletedBlocksCount: deletedBlocks.count, + profileUpdated, + }; + }); + } + async countActiveAdmins(): Promise { + return this.prisma.user.count({ + where: { + role: Role.ADMIN, + is_deleted: false, + }, + }); + } } diff --git a/backend/src/modules/users/users.service.spec.ts b/backend/src/modules/users/users.service.spec.ts index 77373a3a..d582e1ef 100644 --- a/backend/src/modules/users/users.service.spec.ts +++ b/backend/src/modules/users/users.service.spec.ts @@ -1,13 +1,15 @@ import { Test, TestingModule } from '@nestjs/testing'; import { UsersService } from './users.service'; import { UsersRepository } from './users.repository'; -import { NotFoundException, BadRequestException } from '@nestjs/common'; +import { NotFoundException, BadRequestException, ForbiddenException } from '@nestjs/common'; import { Role, UserTag, User, UserBlock } from '@prisma/client'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; import { SetUserTagsDto } from './dto/set-user-tags.dto'; import { TagsService } from '../tags/tags.service'; import { WinstonLoggerService } from '../../common/logging/winston-logger.service'; +import { UserDeleteForbiddenException } from '../../common/exceptions/custom-exceptions/user-delete-forbidden.exception'; +import { LastAdminDeleteException } from '../../common/exceptions/custom-exceptions/last-admin-delete.exception'; describe('UsersService', () => { let service: UsersService; @@ -34,6 +36,8 @@ describe('UsersService', () => { deleteUserBlock: jest.fn(), findUserBlockByIds: jest.fn(), searchUsers: jest.fn(), + deleteUser: jest.fn(), + countActiveAdmins: jest.fn(), }; const mockLoggerService = { @@ -45,6 +49,8 @@ describe('UsersService', () => { const USER_UUID = '123e4567-e89b-12d3-a456-426614174000'; const USER_UUID_2 = '123e4567-e89b-12d3-a456-426614174001'; + const USER_UUID_3 = '123e4567-e89b-12d3-a456-426614174003'; + const ADMIN_UUID = '123e4567-e89b-12d3-a456-426614174002'; const TAG_UUID_1 = 'f47ac10b-58cc-4372-a567-0e02b2c3d479'; const TAG_UUID_2 = 'a1b2c3d4-e5f6-7890-1234-567890abcdef'; const CLERK_ID = 'user_2NUj8tGhSFhTLD9sdP0q4P7VoJM'; @@ -164,7 +170,8 @@ describe('UsersService', () => { role: Role.STUDENT, profile_image: createUserDto.profile_image, is_registered: true, - clerk_id: CLERK_ID, + is_deleted: false, + clerk_id: authUser.clerk_id, }); }); }); @@ -333,28 +340,130 @@ describe('UsersService', () => { }); describe('deleteUser', () => { - it('should soft delete a user after verifying existence', async () => { - const deletedUser = { ...mockUser, is_deleted: true }; - mockUsersRepository.findUserById.mockResolvedValue(mockUser); - mockUsersRepository.softDeleteUser.mockResolvedValue(deletedUser); + const mockCurrentUser = { + id: USER_UUID, + email: 'current@example.com', + role: Role.STUDENT, + first_name: 'Current', + last_name: 'User', + profile_image: null, + is_registered: true, + is_deleted: false, + clerk_id: 'clerk_123', + created_at: new Date(), + updated_at: new Date(), + }; + + const mockAdminUser = { + ...mockCurrentUser, + id: ADMIN_UUID, + email: 'admin@example.com', + role: Role.ADMIN, + }; + + const mockDeleteResult = { + success: true, + message: 'User test@example.com has been deleted successfully', + }; + + it('should allow user to delete their own account', async () => { + mockUsersRepository.findUserByIdWithRelations.mockResolvedValue({ + ...mockUser, + student_profile: { id: 'student-123' }, + supervisor_profile: null, + tags: [], + }); + mockUsersRepository.deleteUser.mockResolvedValue({ + deletedTagsCount: 2, + deletedBlocksCount: 1, + profileUpdated: true, + }); - const result = await service.deleteUser(USER_UUID); + const result = await service.deleteUser(USER_UUID, mockCurrentUser); - expect(result).toEqual(deletedUser); - expect(mockUsersRepository.findUserById).toHaveBeenCalledWith(USER_UUID); - expect(mockUsersRepository.softDeleteUser).toHaveBeenCalledWith(USER_UUID); + expect(result).toEqual({ + success: true, + message: `User ${mockUser.email} has been deleted successfully`, + }); + expect(mockUsersRepository.findUserByIdWithRelations).toHaveBeenCalledWith(USER_UUID); + expect(mockUsersRepository.deleteUser).toHaveBeenCalledWith({ + userId: USER_UUID, + userEmail: mockUser.email, + userRole: mockUser.role, + studentId: 'student-123', + supervisorId: undefined, + }); + expect(mockLoggerService.log).toHaveBeenCalledWith( + expect.stringContaining('Successfully soft deleted user'), + 'UsersService', + ); + expect(mockLoggerService.log).toHaveBeenCalledWith( + expect.stringContaining('Deleted 2 user tags, 1 blocked users'), + 'UsersService', + ); + }); + + it('should allow admin to delete any user', async () => { + const targetUserId = USER_UUID_3; + const targetUser = { ...mockUser, id: targetUserId, role: Role.SUPERVISOR }; + + mockUsersRepository.findUserById.mockResolvedValue(targetUser); + mockUsersRepository.findUserByIdWithRelations.mockResolvedValue({ + ...targetUser, + student_profile: null, + supervisor_profile: { id: 'supervisor-123' }, + tags: [], + }); + mockUsersRepository.deleteUser.mockResolvedValue({ + deletedTagsCount: 0, + deletedBlocksCount: 0, + profileUpdated: true, + }); + + const result = await service.deleteUser(targetUserId, mockAdminUser); + + expect(result).toEqual({ + success: true, + message: `User ${targetUser.email} has been deleted successfully`, + }); + expect(mockUsersRepository.deleteUser).toHaveBeenCalledWith({ + userId: targetUserId, + userEmail: targetUser.email, + userRole: targetUser.role, + studentId: undefined, + supervisorId: 'supervisor-123', + }); + }); + + it('should throw UserDeleteForbiddenException when user tries to delete another user', async () => { + const otherUserId = USER_UUID_3; + + await expect(service.deleteUser(otherUserId, mockCurrentUser)).rejects.toThrow( + UserDeleteForbiddenException, + ); + expect(mockUsersRepository.deleteUser).not.toHaveBeenCalled(); }); it('should throw NotFoundException if user to delete not found', async () => { - // Arrange const nonExistentId = 'non-existent'; - mockUsersRepository.findUserById.mockResolvedValue(null); + mockUsersRepository.findUserByIdWithRelations.mockResolvedValue(null); - // Act & Assert - await expect(service.deleteUser(nonExistentId)).rejects.toThrow( + await expect(service.deleteUser(nonExistentId, mockAdminUser)).rejects.toThrow( new NotFoundException(`User with ID ${nonExistentId} not found`), ); - expect(mockUsersRepository.softDeleteUser).not.toHaveBeenCalled(); + expect(mockUsersRepository.deleteUser).not.toHaveBeenCalled(); + }); + + it('should prevent last admin from deleting themselves', async () => { + const lastAdmin = { ...mockAdminUser, id: ADMIN_UUID }; + + mockUsersRepository.findUserByIdWithRelations.mockResolvedValue(lastAdmin); + mockUsersRepository.countActiveAdmins.mockResolvedValue(1); + + await expect(service.deleteUser(ADMIN_UUID, lastAdmin)).rejects.toThrow( + LastAdminDeleteException, + ); + expect(mockUsersRepository.deleteUser).not.toHaveBeenCalled(); }); }); diff --git a/backend/src/modules/users/users.service.ts b/backend/src/modules/users/users.service.ts index ad9b3f49..ecd4fdc0 100644 --- a/backend/src/modules/users/users.service.ts +++ b/backend/src/modules/users/users.service.ts @@ -1,13 +1,17 @@ import { Injectable, NotFoundException, BadRequestException } from '@nestjs/common'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; +import { DeleteUserSuccessDto } from './dto/delete-user-success.dto'; import { User, UserTag, Role, UserBlock } from '@prisma/client'; import { UsersRepository } from './users.repository'; import { SetUserTagsDto } from './dto/set-user-tags.dto'; import { UserWithRelations } from './entities/user-with-relations.entity'; import { TagsService } from '../tags/tags.service'; import { UserRegistrationException } from '../../common/exceptions/custom-exceptions/user-registration.exception'; +import { UserDeleteForbiddenException } from '../../common/exceptions/custom-exceptions/user-delete-forbidden.exception'; +import { LastAdminDeleteException } from '../../common/exceptions/custom-exceptions/last-admin-delete.exception'; import { WinstonLoggerService } from '../../common/logging/winston-logger.service'; +import { DeleteUserOperation } from './types/user-operations.types'; @Injectable() export class UsersService { @@ -52,6 +56,7 @@ export class UsersService { role: Role.STUDENT, profile_image: createUserDto.profile_image, is_registered: true, + is_deleted: false, }; return this.usersRepository.createUser(studentData); } else { @@ -84,6 +89,7 @@ export class UsersService { return this.usersRepository.updateUser(existingUser.id, { clerk_id: authUser.clerk_id, is_registered: true, + is_deleted: false, // Update profile details if provided in DTO, otherwise keep existing first_name: createUserDto.first_name || existingUser.first_name, last_name: createUserDto.last_name || existingUser.last_name, @@ -104,6 +110,7 @@ export class UsersService { return this.usersRepository.updateUser(existingUser.id, { clerk_id: authUser.clerk_id, is_registered: true, + is_deleted: false, // Update profile details if provided in DTO, otherwise keep existing first_name: createUserDto.first_name || existingUser.first_name, last_name: createUserDto.last_name || existingUser.last_name, @@ -124,6 +131,7 @@ export class UsersService { return this.usersRepository.updateUser(existingUser.id, { clerk_id: authUser.clerk_id, is_registered: true, + is_deleted: false, // Update profile details if provided in DTO, otherwise keep existing first_name: createUserDto.first_name || existingUser.first_name, last_name: createUserDto.last_name || existingUser.last_name, @@ -238,11 +246,56 @@ export class UsersService { return this.usersRepository.updateUser(id, updateUserDto); } - async deleteUser(id: string): Promise { - // This will throw NotFoundException if user doesn't exist - await this.findUserById(id); - // Soft delete - return this.usersRepository.softDeleteUser(id); + async deleteUser(id: string, currentUser: User): Promise { + // Authorization check + if (currentUser.id !== id && currentUser.role !== Role.ADMIN) { + throw new UserDeleteForbiddenException(); + } + + // Find the user to be deleted + const userToDelete = await this.findUserByIdWithRelations(id); + + // Check if admin is trying to delete themselves and they're the last admin + if (currentUser.id === id && currentUser.role === Role.ADMIN) { + const activeAdminCount = await this.usersRepository.countActiveAdmins(); + + if (activeAdminCount === 1) { + throw new LastAdminDeleteException(); + } + } + + // Get student/supervisor profile IDs if needed + let studentId: string | undefined; + let supervisorId: string | undefined; + + if (userToDelete.role === Role.STUDENT) { + studentId = userToDelete.student_profile?.id; + } else if (userToDelete.role === Role.SUPERVISOR) { + supervisorId = userToDelete.supervisor_profile?.id; + } + + // Use repository method for the transaction + const deleteOperation: DeleteUserOperation = { + userId: id, + userEmail: userToDelete.email, + userRole: userToDelete.role, + studentId, + supervisorId, + }; + + const deletionStats = await this.usersRepository.deleteUser(deleteOperation); + + this.logger.log( + `Successfully soft deleted user ${userToDelete.email} (${id}). ` + + `Deleted ${deletionStats.deletedTagsCount} user tags, ${deletionStats.deletedBlocksCount} blocked users. ` + + `Profile updated: ${deletionStats.profileUpdated}`, + 'UsersService', + ); + + return { + success: true, + message: `User ${userToDelete.email} has been deleted successfully`, + }; } // User Tag operations @@ -341,4 +394,12 @@ export class UsersService { // Delete the block await this.usersRepository.deleteUserBlock(studentUserId, supervisorUserId); } + + async deleteAllUserBlocks(userId: string): Promise { + return this.usersRepository.deleteAllUserBlocks(userId); + } + + async deleteAllUserTags(userId: string): Promise { + return this.usersRepository.deleteAllUserTags(userId); + } }