From b3729712fa9416996b37d5aae1b3ce2f1acdc920 Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Sun, 31 May 2026 14:46:24 -0400 Subject: [PATCH 1/8] promote admin to volunteer feature --- apps/backend/src/auth/auth.service.ts | 40 ++++++ .../src/users/dtos/update-user-role.dto.ts | 8 ++ .../src/users/users.controller.spec.ts | 71 ++++++++++- apps/backend/src/users/users.controller.ts | 14 +++ apps/backend/src/users/users.service.spec.ts | 114 ++++++++++++++++++ apps/backend/src/users/users.service.ts | 50 +++++++- apps/frontend/src/api/apiClient.ts | 6 + .../forms/promoteVolunteerModal.tsx | 78 ++++++++++++ .../src/containers/volunteerManagement.tsx | 104 +++++++++++++--- 9 files changed, 462 insertions(+), 23 deletions(-) create mode 100644 apps/backend/src/users/dtos/update-user-role.dto.ts create mode 100644 apps/frontend/src/components/forms/promoteVolunteerModal.tsx diff --git a/apps/backend/src/auth/auth.service.ts b/apps/backend/src/auth/auth.service.ts index d03fcfde7..762edceaa 100644 --- a/apps/backend/src/auth/auth.service.ts +++ b/apps/backend/src/auth/auth.service.ts @@ -8,6 +8,11 @@ import { AdminCreateUserCommand, } from '@aws-sdk/client-cognito-identity-provider'; +import { + AdminAddUserToGroupCommand, + AdminRemoveUserFromGroupCommand, +} from '@aws-sdk/client-cognito-identity-provider'; + import CognitoAuthConfig from './aws-exports'; import { SignUpDto } from './dtos/sign-up.dto'; import { createHmac } from 'crypto'; @@ -70,4 +75,39 @@ export class AuthService { } } } + + async addUserToGroup(username: string, groupName: string): Promise { + const command = new AdminAddUserToGroupCommand({ + UserPoolId: CognitoAuthConfig.userPoolId, + Username: username, + GroupName: groupName, + }); + + try { + await this.providerClient.send(command); + } catch (error) { + throw new InternalServerErrorException( + `Failed to add user to group ${groupName}`, + ); + } + } + + async removeUserFromGroup( + username: string, + groupName: string, + ): Promise { + const command = new AdminRemoveUserFromGroupCommand({ + UserPoolId: CognitoAuthConfig.userPoolId, + Username: username, + GroupName: groupName, + }); + + try { + await this.providerClient.send(command); + } catch (error) { + throw new InternalServerErrorException( + `Failed to remove user from group ${groupName}`, + ); + } + } } diff --git a/apps/backend/src/users/dtos/update-user-role.dto.ts b/apps/backend/src/users/dtos/update-user-role.dto.ts new file mode 100644 index 000000000..f9cebe9f0 --- /dev/null +++ b/apps/backend/src/users/dtos/update-user-role.dto.ts @@ -0,0 +1,8 @@ +import { IsEnum, IsNotEmpty } from 'class-validator'; +import { Role } from '../types'; + +export class UpdateUserRoleDto { + @IsEnum(Role) + @IsNotEmpty() + role!: Role; +} diff --git a/apps/backend/src/users/users.controller.spec.ts b/apps/backend/src/users/users.controller.spec.ts index fd923d516..47ea754a1 100644 --- a/apps/backend/src/users/users.controller.spec.ts +++ b/apps/backend/src/users/users.controller.spec.ts @@ -6,7 +6,8 @@ import { userSchemaDto } from './dtos/userSchema.dto'; import { Test, TestingModule } from '@nestjs/testing'; import { mock } from 'jest-mock-extended'; import { UpdateUserInfoDto } from './dtos/update-user-info.dto'; -import { BadRequestException } from '@nestjs/common'; +import { UpdateUserRoleDto } from './dtos/update-user-role.dto'; +import { BadRequestException, NotFoundException } from '@nestjs/common'; import { AuthenticatedRequest } from '../auth/authenticated-request'; const mockUserService = mock(); @@ -31,6 +32,7 @@ describe('UsersController', () => { mockUserService.create.mockReset(); mockUserService.getUserDashboardStats.mockReset(); mockUserService.getRecentPendingApplications.mockReset(); + mockUserService.promoteVolunteerToAdmin.mockReset(); const module: TestingModule = await Test.createTestingModule({ controllers: [UsersController], @@ -211,4 +213,71 @@ describe('UsersController', () => { expect(result).toEqual([]); }); }); + + describe('PATCH /:id/role', () => { + it('should promote volunteer to admin successfully', async () => { + const promotedUser: Partial = { + ...mockUser1, + role: Role.ADMIN, + }; + + const dto: UpdateUserRoleDto = { role: Role.ADMIN }; + + mockUserService.promoteVolunteerToAdmin.mockResolvedValueOnce( + promotedUser as User, + ); + + const result = await controller.promoteToAdmin(1, dto); + + expect(result).toEqual(promotedUser); + expect(result.role).toBe(Role.ADMIN); + expect(mockUserService.promoteVolunteerToAdmin).toHaveBeenCalledWith(1); + }); + + it('should throw BadRequestException when role is not admin', async () => { + const dto: UpdateUserRoleDto = { role: Role.VOLUNTEER }; + + await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow( + new BadRequestException('Only promotion to admin is supported'), + ); + + expect(mockUserService.promoteVolunteerToAdmin).not.toHaveBeenCalled(); + }); + + it('should throw BadRequestException when role is pantry', async () => { + const dto: UpdateUserRoleDto = { role: Role.PANTRY }; + + await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow( + new BadRequestException('Only promotion to admin is supported'), + ); + + expect(mockUserService.promoteVolunteerToAdmin).not.toHaveBeenCalled(); + }); + + it('should throw NotFoundException from service when user not found', async () => { + const dto: UpdateUserRoleDto = { role: Role.ADMIN }; + + mockUserService.promoteVolunteerToAdmin.mockRejectedValueOnce( + new NotFoundException('User 999 not found'), + ); + + await expect(controller.promoteToAdmin(999, dto)).rejects.toThrow( + new NotFoundException('User 999 not found'), + ); + }); + + it('should throw BadRequestException from service when user is not a volunteer', async () => { + const dto: UpdateUserRoleDto = { role: Role.ADMIN }; + + mockUserService.promoteVolunteerToAdmin.mockRejectedValueOnce( + new BadRequestException( + 'User 1 is not a volunteer. Current role: admin', + ), + ); + + await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow( + BadRequestException, + ); + }); + }); }); diff --git a/apps/backend/src/users/users.controller.ts b/apps/backend/src/users/users.controller.ts index cf27de69b..713694b7a 100644 --- a/apps/backend/src/users/users.controller.ts +++ b/apps/backend/src/users/users.controller.ts @@ -1,4 +1,5 @@ import { + BadRequestException, Controller, Delete, Get, @@ -14,6 +15,7 @@ import { UsersService } from './users.service'; import { User } from './users.entity'; import { userSchemaDto } from './dtos/userSchema.dto'; import { UpdateUserInfoDto } from './dtos/update-user-info.dto'; +import { UpdateUserRoleDto } from './dtos/update-user-role.dto'; import { PendingApplication, Role } from './types'; import { AuthenticatedRequest } from '../auth/authenticated-request'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; @@ -53,6 +55,18 @@ export class UsersController { return this.usersService.update(id, dto); } + @Patch('/:id/role') + @Roles(Role.ADMIN) + async promoteToAdmin( + @Param('id', ParseIntPipe) id: number, + @Body() dto: UpdateUserRoleDto, + ): Promise { + if (dto.role !== Role.ADMIN) { + throw new BadRequestException('Only promotion to admin is supported'); + } + return this.usersService.promoteVolunteerToAdmin(id); + } + // Keeping these two as functionality seems useful @Post('/') async createUser(@Body() createUserDto: userSchemaDto): Promise { diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index c81a6664b..306485866 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -37,6 +37,8 @@ jest.setTimeout(60000); const mockAuthService = { adminCreateUser: jest.fn().mockResolvedValue('mock-sub'), + addUserToGroup: jest.fn().mockResolvedValue(undefined), + removeUserFromGroup: jest.fn().mockResolvedValue(undefined), }; const mockEmailsService = mock(); @@ -125,6 +127,8 @@ describe('UsersService', () => { beforeEach(async () => { mockAuthService.adminCreateUser.mockClear(); + mockAuthService.addUserToGroup.mockClear(); + mockAuthService.removeUserFromGroup.mockClear(); mockEmailsService.sendEmails.mockClear(); await testDataSource.runMigrations(); }); @@ -730,4 +734,114 @@ describe('UsersService', () => { expect(types).toContain('food_manufacturer'); }); }); + + describe('promoteVolunteerToAdmin', () => { + it('should promote volunteer to admin successfully', async () => { + const volunteers = await testDataSource.getRepository(User).find({ + where: { role: Role.VOLUNTEER }, + }); + expect(volunteers.length).toBeGreaterThan(0); + const volunteer = volunteers[0]; + + const result = await service.promoteVolunteerToAdmin(volunteer.id); + + expect(result.role).toBe(Role.ADMIN); + expect(result.id).toBe(volunteer.id); + }); + + it('should clear volunteer pantry assignments after promotion', async () => { + const volunteer = await testDataSource.getRepository(User).findOne({ + where: { role: Role.VOLUNTEER }, + relations: ['pantries'], + }); + expect(volunteer).toBeDefined(); + + await service.promoteVolunteerToAdmin(volunteer!.id); + + const assignments = await testDataSource.query( + `SELECT * FROM volunteer_assignments WHERE volunteer_id = $1`, + [volunteer!.id], + ); + expect(assignments).toHaveLength(0); + }); + + it('should call Cognito addUserToGroup and removeUserFromGroup', async () => { + const volunteer = await testDataSource.getRepository(User).findOne({ + where: { role: Role.VOLUNTEER }, + }); + expect(volunteer).toBeDefined(); + + await service.promoteVolunteerToAdmin(volunteer!.id); + + if (volunteer!.userCognitoSub) { + expect(mockAuthService.addUserToGroup).toHaveBeenCalledWith( + volunteer!.email, + 'admin', + ); + expect(mockAuthService.removeUserFromGroup).toHaveBeenCalledWith( + volunteer!.email, + 'volunteer', + ); + } + }); + + it('should throw NotFoundException when user does not exist', async () => { + await expect(service.promoteVolunteerToAdmin(99999)).rejects.toThrow( + NotFoundException, + ); + }); + + it('should throw BadRequestException when user is already admin', async () => { + const admin = await testDataSource.getRepository(User).findOne({ + where: { role: Role.ADMIN }, + }); + expect(admin).toBeDefined(); + + await expect(service.promoteVolunteerToAdmin(admin!.id)).rejects.toThrow( + BadRequestException, + ); + }); + + it('should throw BadRequestException when user is pantry', async () => { + const pantryUser = await testDataSource.getRepository(User).findOne({ + where: { role: Role.PANTRY }, + }); + expect(pantryUser).toBeDefined(); + + await expect( + service.promoteVolunteerToAdmin(pantryUser!.id), + ).rejects.toThrow(BadRequestException); + }); + + it('should throw BadRequestException when user is food manufacturer', async () => { + const fmUser = await testDataSource.getRepository(User).findOne({ + where: { role: Role.FOODMANUFACTURER }, + }); + expect(fmUser).toBeDefined(); + + await expect(service.promoteVolunteerToAdmin(fmUser!.id)).rejects.toThrow( + BadRequestException, + ); + }); + + it('should rollback if Cognito fails', async () => { + const volunteer = await testDataSource.getRepository(User).findOne({ + where: { role: Role.VOLUNTEER }, + }); + expect(volunteer).toBeDefined(); + + mockAuthService.addUserToGroup.mockRejectedValueOnce( + new Error('Cognito error'), + ); + + await expect( + service.promoteVolunteerToAdmin(volunteer!.id), + ).rejects.toThrow(InternalServerErrorException); + + const userAfter = await testDataSource.getRepository(User).findOne({ + where: { id: volunteer!.id }, + }); + expect(userAfter!.role).toBe(Role.VOLUNTEER); + }); + }); }); diff --git a/apps/backend/src/users/users.service.ts b/apps/backend/src/users/users.service.ts index a5ea7db2d..65794e358 100644 --- a/apps/backend/src/users/users.service.ts +++ b/apps/backend/src/users/users.service.ts @@ -7,8 +7,8 @@ import { InternalServerErrorException, NotFoundException, } from '@nestjs/common'; -import { InjectRepository } from '@nestjs/typeorm'; -import { Between, In, Repository } from 'typeorm'; +import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; +import { Between, DataSource, In, Repository } from 'typeorm'; import { User } from './users.entity'; import { PendingApplication, Role } from './types'; import { validateId } from '../utils/validation.utils'; @@ -44,6 +44,8 @@ export class UsersService { private pantryRepo: Repository, @InjectRepository(FoodManufacturer) private fmRepo: Repository, + @InjectDataSource() + private dataSource: DataSource, private authService: AuthService, private emailsService: EmailsService, @Inject(forwardRef(() => PantriesService)) @@ -298,4 +300,48 @@ export class UsersService { throw new BadRequestException(`Unsupported role: ${user.role}`); } } + + async promoteVolunteerToAdmin(userId: number): Promise { + validateId(userId, 'User'); + + const user = await this.repo.findOne({ + where: { id: userId }, + relations: ['pantries'], + }); + + if (!user) { + throw new NotFoundException(`User ${userId} not found`); + } + + if (user.role !== Role.VOLUNTEER) { + throw new BadRequestException( + `User ${userId} is not a volunteer. Current role: ${user.role}`, + ); + } + + return this.dataSource.transaction(async (transactionManager) => { + const userRepo = transactionManager.getRepository(User); + + user.role = Role.ADMIN; + const savedUser = await userRepo.save(user); + + await transactionManager.query( + `DELETE FROM volunteer_assignments WHERE volunteer_id = $1`, + [userId], + ); + + if (user.userCognitoSub) { + try { + await this.authService.addUserToGroup(user.email, 'admin'); + await this.authService.removeUserFromGroup(user.email, 'volunteer'); + } catch (error) { + throw new InternalServerErrorException( + 'Failed to update Cognito groups. Please try again.', + ); + } + } + + return savedUser; + }); + } } diff --git a/apps/frontend/src/api/apiClient.ts b/apps/frontend/src/api/apiClient.ts index 5e822dfb1..6d68018c2 100644 --- a/apps/frontend/src/api/apiClient.ts +++ b/apps/frontend/src/api/apiClient.ts @@ -282,6 +282,12 @@ export class ApiClient { .then((response) => response.data); } + public async promoteVolunteerToAdmin(userId: number): Promise { + return this.axiosInstance + .patch(`/api/users/${userId}/role`, { role: 'admin' }) + .then((response) => response.data); + } + public async getFoodRequest(requestId: number): Promise { return this.axiosInstance .get(`/api/requests/${requestId}`) diff --git a/apps/frontend/src/components/forms/promoteVolunteerModal.tsx b/apps/frontend/src/components/forms/promoteVolunteerModal.tsx new file mode 100644 index 000000000..53d574391 --- /dev/null +++ b/apps/frontend/src/components/forms/promoteVolunteerModal.tsx @@ -0,0 +1,78 @@ +import { Dialog, Text, Button, CloseButton } from '@chakra-ui/react'; +import { useModalBodyCleanup } from '../../hooks/modalBodyCleanup'; + +interface PromoteVolunteerModalProps { + isOpen: boolean; + onClose: () => void; + onConfirm: () => void; + volunteerName: string; +} + +const PromoteVolunteerModal: React.FC = ({ + isOpen, + onClose, + onConfirm, + volunteerName, +}) => { + useModalBodyCleanup(); + + return ( + !e.open && onClose()} + > + + + + + + Promote user + + + + + + + + + Are you sure you want to promote {volunteerName} to admin status? + + + + + + + + + + + ); +}; + +export default PromoteVolunteerModal; diff --git a/apps/frontend/src/containers/volunteerManagement.tsx b/apps/frontend/src/containers/volunteerManagement.tsx index b0a2cb681..5bc70920e 100644 --- a/apps/frontend/src/containers/volunteerManagement.tsx +++ b/apps/frontend/src/containers/volunteerManagement.tsx @@ -12,11 +12,19 @@ import { ButtonGroup, IconButton, Link, + Menu, + Portal, } from '@chakra-ui/react'; -import { SearchIcon, ChevronRight, ChevronLeft } from 'lucide-react'; +import { + SearchIcon, + ChevronRight, + ChevronLeft, + EllipsisVertical, +} from 'lucide-react'; import { User } from '../types/types'; import ApiClient from '@api/apiClient'; import NewVolunteerModal from '@components/forms/addNewVolunteerModal'; +import PromoteVolunteerModal from '@components/forms/promoteVolunteerModal'; import { FloatingAlert } from '@components/floatingAlert'; import { useAlert } from '../hooks/alert'; import { getInitials, USER_ICON_COLORS } from '@utils/utils'; @@ -25,22 +33,24 @@ const VolunteerManagement: React.FC = () => { const [currentPage, setCurrentPage] = useState(1); const [volunteers, setVolunteers] = useState([]); const [searchName, setSearchName] = useState(''); + const [selectedVolunteer, setSelectedVolunteer] = useState(null); + const [isPromoteModalOpen, setIsPromoteModalOpen] = useState(false); const [errorAlertState, setErrorMessage] = useAlert(); const [successAlertState, setSuccessMessage] = useAlert(); const pageSize = 8; - useEffect(() => { - const fetchVolunteers = async () => { - try { - const allVolunteers = await ApiClient.getVolunteers(); - setVolunteers(allVolunteers); - } catch { - setErrorMessage('Error fetching volunteers'); - } - }; + const fetchVolunteers = async () => { + try { + const allVolunteers = await ApiClient.getVolunteers(); + setVolunteers(allVolunteers); + } catch { + setErrorMessage('Error fetching volunteers'); + } + }; + useEffect(() => { fetchVolunteers(); }, [setErrorMessage]); @@ -48,6 +58,20 @@ const VolunteerManagement: React.FC = () => { setCurrentPage(1); }, [searchName]); + const handlePromote = async () => { + if (!selectedVolunteer) return; + + try { + await ApiClient.promoteVolunteerToAdmin(selectedVolunteer.id); + setSuccessMessage( + `${selectedVolunteer.firstName} ${selectedVolunteer.lastName} has been promoted to admin.`, + ); + fetchVolunteers(); // Refresh list - promoted user will disappear + } catch { + setErrorMessage('Failed to promote volunteer to admin.'); + } + }; + const filteredVolunteers = volunteers.filter((a) => { const fullName = `${a.firstName} ${a.lastName}`.toLowerCase(); return fullName.includes(searchName.toLowerCase()); @@ -180,16 +204,44 @@ const VolunteerManagement: React.FC = () => { {volunteer.email} - - View Assigned Pantries - + + + View Assigned Pantries + + + + + + + + + + + { + setSelectedVolunteer(volunteer); + setIsPromoteModalOpen(true); + }} + > + Promote to Admin + + + + + + ))} @@ -249,6 +301,18 @@ const VolunteerManagement: React.FC = () => { + + {selectedVolunteer && ( + { + setIsPromoteModalOpen(false); + setSelectedVolunteer(null); + }} + onConfirm={handlePromote} + volunteerName={`${selectedVolunteer.firstName} ${selectedVolunteer.lastName}`} + /> + )} ); }; From 5b60321b97337a8afa12c1efc81288076640193e Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Sun, 31 May 2026 15:07:53 -0400 Subject: [PATCH 2/8] fix pantries.service.spec.ts missing DataSource --- apps/backend/src/pantries/pantries.service.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/backend/src/pantries/pantries.service.spec.ts b/apps/backend/src/pantries/pantries.service.spec.ts index 65a267ad6..28b2cbfe8 100644 --- a/apps/backend/src/pantries/pantries.service.spec.ts +++ b/apps/backend/src/pantries/pantries.service.spec.ts @@ -1,7 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { PantriesService } from './pantries.service'; import { getRepositoryToken } from '@nestjs/typeorm'; -import { In } from 'typeorm'; +import { DataSource, In } from 'typeorm'; import { Pantry } from './pantries.entity'; import { BadRequestException, @@ -145,6 +145,10 @@ describe('PantriesService', () => { provide: getRepositoryToken(FoodManufacturer), useValue: testDataSource.getRepository(FoodManufacturer), }, + { + provide: DataSource, + useValue: testDataSource, + }, ], }).compile(); From d932f0a3472a4c090bae15a6117a6c6b0c9e9149 Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Sun, 31 May 2026 16:05:35 -0400 Subject: [PATCH 3/8] fix transaction rollback when Cognito fails --- apps/backend/src/users/users.service.spec.ts | 9 +++-- apps/backend/src/users/users.service.ts | 38 ++++++++++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index 306485866..1f7625d47 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -825,11 +825,16 @@ describe('UsersService', () => { }); it('should rollback if Cognito fails', async () => { - const volunteer = await testDataSource.getRepository(User).findOne({ + const userRepo = testDataSource.getRepository(User); + const volunteer = await userRepo.findOne({ where: { role: Role.VOLUNTEER }, }); expect(volunteer).toBeDefined(); + // Set userCognitoSub so the Cognito code path is triggered + volunteer!.userCognitoSub = 'test-cognito-sub'; + await userRepo.save(volunteer!); + mockAuthService.addUserToGroup.mockRejectedValueOnce( new Error('Cognito error'), ); @@ -838,7 +843,7 @@ describe('UsersService', () => { service.promoteVolunteerToAdmin(volunteer!.id), ).rejects.toThrow(InternalServerErrorException); - const userAfter = await testDataSource.getRepository(User).findOne({ + const userAfter = await userRepo.findOne({ where: { id: volunteer!.id }, }); expect(userAfter!.role).toBe(Role.VOLUNTEER); diff --git a/apps/backend/src/users/users.service.ts b/apps/backend/src/users/users.service.ts index 65794e358..4d8fb3e51 100644 --- a/apps/backend/src/users/users.service.ts +++ b/apps/backend/src/users/users.service.ts @@ -319,29 +319,39 @@ export class UsersService { ); } - return this.dataSource.transaction(async (transactionManager) => { - const userRepo = transactionManager.getRepository(User); + const queryRunner = this.dataSource.createQueryRunner(); + await queryRunner.connect(); + await queryRunner.startTransaction(); + try { user.role = Role.ADMIN; - const savedUser = await userRepo.save(user); + await queryRunner.manager.save(user); - await transactionManager.query( + await queryRunner.query( `DELETE FROM volunteer_assignments WHERE volunteer_id = $1`, [userId], ); if (user.userCognitoSub) { - try { - await this.authService.addUserToGroup(user.email, 'admin'); - await this.authService.removeUserFromGroup(user.email, 'volunteer'); - } catch (error) { - throw new InternalServerErrorException( - 'Failed to update Cognito groups. Please try again.', - ); - } + await this.authService.addUserToGroup(user.email, 'admin'); + await this.authService.removeUserFromGroup(user.email, 'volunteer'); } - return savedUser; - }); + await queryRunner.commitTransaction(); + return user; + } catch (error) { + await queryRunner.rollbackTransaction(); + if ( + error instanceof NotFoundException || + error instanceof BadRequestException + ) { + throw error; + } + throw new InternalServerErrorException( + 'Failed to promote volunteer to admin. Please try again.', + ); + } finally { + await queryRunner.release(); + } } } From 378d9e55d314ea33eb52394455dfb88a03cdc996 Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Sun, 7 Jun 2026 17:09:23 -0400 Subject: [PATCH 4/8] simplfying promote volunteer endpoint and making promote to admin ui the same as figma designs --- apps/backend/src/auth/auth.service.ts | 3 - .../src/users/dtos/update-user-role.dto.ts | 8 -- .../src/users/users.controller.spec.ts | 46 ++--------- apps/backend/src/users/users.controller.ts | 14 +--- apps/backend/src/users/users.service.spec.ts | 51 ++++++++---- apps/backend/src/users/users.service.ts | 35 ++------- apps/frontend/src/api/apiClient.ts | 6 +- .../src/containers/volunteerManagement.tsx | 77 ++++++++++--------- 8 files changed, 91 insertions(+), 149 deletions(-) delete mode 100644 apps/backend/src/users/dtos/update-user-role.dto.ts diff --git a/apps/backend/src/auth/auth.service.ts b/apps/backend/src/auth/auth.service.ts index 762edceaa..1c03876f3 100644 --- a/apps/backend/src/auth/auth.service.ts +++ b/apps/backend/src/auth/auth.service.ts @@ -6,9 +6,6 @@ import { import { CognitoIdentityProviderClient, AdminCreateUserCommand, -} from '@aws-sdk/client-cognito-identity-provider'; - -import { AdminAddUserToGroupCommand, AdminRemoveUserFromGroupCommand, } from '@aws-sdk/client-cognito-identity-provider'; diff --git a/apps/backend/src/users/dtos/update-user-role.dto.ts b/apps/backend/src/users/dtos/update-user-role.dto.ts deleted file mode 100644 index f9cebe9f0..000000000 --- a/apps/backend/src/users/dtos/update-user-role.dto.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { IsEnum, IsNotEmpty } from 'class-validator'; -import { Role } from '../types'; - -export class UpdateUserRoleDto { - @IsEnum(Role) - @IsNotEmpty() - role!: Role; -} diff --git a/apps/backend/src/users/users.controller.spec.ts b/apps/backend/src/users/users.controller.spec.ts index 47ea754a1..63ca001d5 100644 --- a/apps/backend/src/users/users.controller.spec.ts +++ b/apps/backend/src/users/users.controller.spec.ts @@ -6,7 +6,6 @@ import { userSchemaDto } from './dtos/userSchema.dto'; import { Test, TestingModule } from '@nestjs/testing'; import { mock } from 'jest-mock-extended'; import { UpdateUserInfoDto } from './dtos/update-user-info.dto'; -import { UpdateUserRoleDto } from './dtos/update-user-role.dto'; import { BadRequestException, NotFoundException } from '@nestjs/common'; import { AuthenticatedRequest } from '../auth/authenticated-request'; @@ -214,68 +213,33 @@ describe('UsersController', () => { }); }); - describe('PATCH /:id/role', () => { + describe('PATCH /:id/promote-volunteer', () => { it('should promote volunteer to admin successfully', async () => { - const promotedUser: Partial = { - ...mockUser1, - role: Role.ADMIN, - }; + mockUserService.promoteVolunteerToAdmin.mockResolvedValueOnce(undefined); - const dto: UpdateUserRoleDto = { role: Role.ADMIN }; - - mockUserService.promoteVolunteerToAdmin.mockResolvedValueOnce( - promotedUser as User, - ); + await controller.promoteToAdmin(1); - const result = await controller.promoteToAdmin(1, dto); - - expect(result).toEqual(promotedUser); - expect(result.role).toBe(Role.ADMIN); expect(mockUserService.promoteVolunteerToAdmin).toHaveBeenCalledWith(1); }); - it('should throw BadRequestException when role is not admin', async () => { - const dto: UpdateUserRoleDto = { role: Role.VOLUNTEER }; - - await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow( - new BadRequestException('Only promotion to admin is supported'), - ); - - expect(mockUserService.promoteVolunteerToAdmin).not.toHaveBeenCalled(); - }); - - it('should throw BadRequestException when role is pantry', async () => { - const dto: UpdateUserRoleDto = { role: Role.PANTRY }; - - await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow( - new BadRequestException('Only promotion to admin is supported'), - ); - - expect(mockUserService.promoteVolunteerToAdmin).not.toHaveBeenCalled(); - }); - it('should throw NotFoundException from service when user not found', async () => { - const dto: UpdateUserRoleDto = { role: Role.ADMIN }; - mockUserService.promoteVolunteerToAdmin.mockRejectedValueOnce( new NotFoundException('User 999 not found'), ); - await expect(controller.promoteToAdmin(999, dto)).rejects.toThrow( + await expect(controller.promoteToAdmin(999)).rejects.toThrow( new NotFoundException('User 999 not found'), ); }); it('should throw BadRequestException from service when user is not a volunteer', async () => { - const dto: UpdateUserRoleDto = { role: Role.ADMIN }; - mockUserService.promoteVolunteerToAdmin.mockRejectedValueOnce( new BadRequestException( 'User 1 is not a volunteer. Current role: admin', ), ); - await expect(controller.promoteToAdmin(1, dto)).rejects.toThrow( + await expect(controller.promoteToAdmin(1)).rejects.toThrow( BadRequestException, ); }); diff --git a/apps/backend/src/users/users.controller.ts b/apps/backend/src/users/users.controller.ts index 713694b7a..f5e37ce10 100644 --- a/apps/backend/src/users/users.controller.ts +++ b/apps/backend/src/users/users.controller.ts @@ -1,5 +1,4 @@ import { - BadRequestException, Controller, Delete, Get, @@ -15,7 +14,6 @@ import { UsersService } from './users.service'; import { User } from './users.entity'; import { userSchemaDto } from './dtos/userSchema.dto'; import { UpdateUserInfoDto } from './dtos/update-user-info.dto'; -import { UpdateUserRoleDto } from './dtos/update-user-role.dto'; import { PendingApplication, Role } from './types'; import { AuthenticatedRequest } from '../auth/authenticated-request'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; @@ -55,16 +53,10 @@ export class UsersController { return this.usersService.update(id, dto); } - @Patch('/:id/role') + @Patch('/:id/promote-volunteer') @Roles(Role.ADMIN) - async promoteToAdmin( - @Param('id', ParseIntPipe) id: number, - @Body() dto: UpdateUserRoleDto, - ): Promise { - if (dto.role !== Role.ADMIN) { - throw new BadRequestException('Only promotion to admin is supported'); - } - return this.usersService.promoteVolunteerToAdmin(id); + async promoteToAdmin(@Param('id', ParseIntPipe) id: number): Promise { + await this.usersService.promoteVolunteerToAdmin(id); } // Keeping these two as functionality seems useful diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index 1f7625d47..e3798b577 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -737,16 +737,19 @@ describe('UsersService', () => { describe('promoteVolunteerToAdmin', () => { it('should promote volunteer to admin successfully', async () => { - const volunteers = await testDataSource.getRepository(User).find({ + const userRepo = testDataSource.getRepository(User); + const volunteers = await userRepo.find({ where: { role: Role.VOLUNTEER }, }); expect(volunteers.length).toBeGreaterThan(0); const volunteer = volunteers[0]; - const result = await service.promoteVolunteerToAdmin(volunteer.id); + await service.promoteVolunteerToAdmin(volunteer.id); - expect(result.role).toBe(Role.ADMIN); - expect(result.id).toBe(volunteer.id); + const updatedUser = await userRepo.findOne({ + where: { id: volunteer.id }, + }); + expect(updatedUser!.role).toBe(Role.ADMIN); }); it('should clear volunteer pantry assignments after promotion', async () => { @@ -765,24 +768,40 @@ describe('UsersService', () => { expect(assignments).toHaveLength(0); }); - it('should call Cognito addUserToGroup and removeUserFromGroup', async () => { + it('should call Cognito addUserToGroup and removeUserFromGroup when user has Cognito account', async () => { + const userRepo = testDataSource.getRepository(User); + const volunteer = await userRepo.findOne({ + where: { role: Role.VOLUNTEER }, + }); + expect(volunteer).toBeDefined(); + + // Set userCognitoSub to simulate a user with a Cognito account + volunteer!.userCognitoSub = 'test-cognito-sub'; + await userRepo.save(volunteer!); + + await service.promoteVolunteerToAdmin(volunteer!.id); + + expect(mockAuthService.addUserToGroup).toHaveBeenCalledWith( + volunteer!.email, + 'admin', + ); + expect(mockAuthService.removeUserFromGroup).toHaveBeenCalledWith( + volunteer!.email, + 'volunteer', + ); + }); + + it('should not call Cognito methods when user has no Cognito account', async () => { const volunteer = await testDataSource.getRepository(User).findOne({ where: { role: Role.VOLUNTEER }, }); expect(volunteer).toBeDefined(); + expect(volunteer!.userCognitoSub).toBeNull(); await service.promoteVolunteerToAdmin(volunteer!.id); - if (volunteer!.userCognitoSub) { - expect(mockAuthService.addUserToGroup).toHaveBeenCalledWith( - volunteer!.email, - 'admin', - ); - expect(mockAuthService.removeUserFromGroup).toHaveBeenCalledWith( - volunteer!.email, - 'volunteer', - ); - } + expect(mockAuthService.addUserToGroup).not.toHaveBeenCalled(); + expect(mockAuthService.removeUserFromGroup).not.toHaveBeenCalled(); }); it('should throw NotFoundException when user does not exist', async () => { @@ -836,7 +855,7 @@ describe('UsersService', () => { await userRepo.save(volunteer!); mockAuthService.addUserToGroup.mockRejectedValueOnce( - new Error('Cognito error'), + new InternalServerErrorException('Cognito error'), ); await expect( diff --git a/apps/backend/src/users/users.service.ts b/apps/backend/src/users/users.service.ts index 4d8fb3e51..6dd16a747 100644 --- a/apps/backend/src/users/users.service.ts +++ b/apps/backend/src/users/users.service.ts @@ -301,7 +301,7 @@ export class UsersService { } } - async promoteVolunteerToAdmin(userId: number): Promise { + async promoteVolunteerToAdmin(userId: number): Promise { validateId(userId, 'User'); const user = await this.repo.findOne({ @@ -319,39 +319,18 @@ export class UsersService { ); } - const queryRunner = this.dataSource.createQueryRunner(); - await queryRunner.connect(); - await queryRunner.startTransaction(); + await this.dataSource.transaction(async (transactionManager) => { + const userRepo = transactionManager.getRepository(User); - try { user.role = Role.ADMIN; - await queryRunner.manager.save(user); - - await queryRunner.query( - `DELETE FROM volunteer_assignments WHERE volunteer_id = $1`, - [userId], - ); + user.pantries = []; + await userRepo.save(user); + // Only update Cognito groups if user has a Cognito account if (user.userCognitoSub) { await this.authService.addUserToGroup(user.email, 'admin'); await this.authService.removeUserFromGroup(user.email, 'volunteer'); } - - await queryRunner.commitTransaction(); - return user; - } catch (error) { - await queryRunner.rollbackTransaction(); - if ( - error instanceof NotFoundException || - error instanceof BadRequestException - ) { - throw error; - } - throw new InternalServerErrorException( - 'Failed to promote volunteer to admin. Please try again.', - ); - } finally { - await queryRunner.release(); - } + }); } } diff --git a/apps/frontend/src/api/apiClient.ts b/apps/frontend/src/api/apiClient.ts index 6d68018c2..d7229791c 100644 --- a/apps/frontend/src/api/apiClient.ts +++ b/apps/frontend/src/api/apiClient.ts @@ -282,10 +282,8 @@ export class ApiClient { .then((response) => response.data); } - public async promoteVolunteerToAdmin(userId: number): Promise { - return this.axiosInstance - .patch(`/api/users/${userId}/role`, { role: 'admin' }) - .then((response) => response.data); + public async promoteVolunteerToAdmin(userId: number): Promise { + await this.axiosInstance.patch(`/api/users/${userId}/promote-volunteer`); } public async getFoodRequest(requestId: number): Promise { diff --git a/apps/frontend/src/containers/volunteerManagement.tsx b/apps/frontend/src/containers/volunteerManagement.tsx index 5bc70920e..5f20cd497 100644 --- a/apps/frontend/src/containers/volunteerManagement.tsx +++ b/apps/frontend/src/containers/volunteerManagement.tsx @@ -177,6 +177,7 @@ const VolunteerManagement: React.FC = () => { > Actions + @@ -204,44 +205,44 @@ const VolunteerManagement: React.FC = () => { {volunteer.email} - - - View Assigned Pantries - - - - - - - - - - - { - setSelectedVolunteer(volunteer); - setIsPromoteModalOpen(true); - }} - > - Promote to Admin - - - - - - + + View Assigned Pantries + + + + + + + + + + + + + { + setSelectedVolunteer(volunteer); + setIsPromoteModalOpen(true); + }} + > + Promote to Admin + + + + + ))} From d4536892707d2f14d053b605cfd0731b3dcbfda1 Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Sun, 7 Jun 2026 17:22:53 -0400 Subject: [PATCH 5/8] fix failing tests: added DataSource provider and filter for null userCognitoSub --- apps/backend/src/foodRequests/request.service.spec.ts | 5 +++++ apps/backend/src/users/users.service.spec.ts | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/backend/src/foodRequests/request.service.spec.ts b/apps/backend/src/foodRequests/request.service.spec.ts index 0aa75f951..2f5cad1e9 100644 --- a/apps/backend/src/foodRequests/request.service.spec.ts +++ b/apps/backend/src/foodRequests/request.service.spec.ts @@ -1,5 +1,6 @@ import { Test } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { DataSource } from 'typeorm'; import { FoodRequest } from './request.entity'; import { RequestsService } from './request.service'; import { Pantry } from '../pantries/pantries.entity'; @@ -66,6 +67,10 @@ describe('RequestsService', () => { provide: EmailsService, useValue: mockEmailsService, }, + { + provide: DataSource, + useValue: testDataSource, + }, ], }).compile(); diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index e3798b577..25531990d 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -793,10 +793,9 @@ describe('UsersService', () => { it('should not call Cognito methods when user has no Cognito account', async () => { const volunteer = await testDataSource.getRepository(User).findOne({ - where: { role: Role.VOLUNTEER }, + where: { role: Role.VOLUNTEER, userCognitoSub: null }, }); expect(volunteer).toBeDefined(); - expect(volunteer!.userCognitoSub).toBeNull(); await service.promoteVolunteerToAdmin(volunteer!.id); From 388487c12fb9d51bdab5c31aac11ad1cdb0c4e64 Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Mon, 8 Jun 2026 14:03:24 -0400 Subject: [PATCH 6/8] fix test null for TypeORM --- apps/backend/src/users/users.service.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index 25531990d..17cf2691f 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -27,7 +27,7 @@ import { Pantry } from '../pantries/pantries.entity'; import { FoodManufacturer } from '../foodManufacturers/manufacturers.entity'; import { DonationItem } from '../donationItems/donationItems.entity'; import { Allocation } from '../allocations/allocations.entity'; -import { DataSource } from 'typeorm'; +import { DataSource, IsNull } from 'typeorm'; import { FoodManufacturersService } from '../foodManufacturers/manufacturers.service'; import { DonationItemsService } from '../donationItems/donationItems.service'; import { AllocationsService } from '../allocations/allocations.service'; @@ -793,7 +793,7 @@ describe('UsersService', () => { it('should not call Cognito methods when user has no Cognito account', async () => { const volunteer = await testDataSource.getRepository(User).findOne({ - where: { role: Role.VOLUNTEER, userCognitoSub: null }, + where: { role: Role.VOLUNTEER, userCognitoSub: IsNull() }, }); expect(volunteer).toBeDefined(); From 5fcfab46bafeca68b620746181cd923f16a5be11 Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Mon, 8 Jun 2026 14:35:57 -0400 Subject: [PATCH 7/8] fix tests for userCognitoSub --- apps/backend/src/users/users.service.spec.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index 17cf2691f..01867d5b3 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -27,7 +27,7 @@ import { Pantry } from '../pantries/pantries.entity'; import { FoodManufacturer } from '../foodManufacturers/manufacturers.entity'; import { DonationItem } from '../donationItems/donationItems.entity'; import { Allocation } from '../allocations/allocations.entity'; -import { DataSource, IsNull } from 'typeorm'; +import { DataSource } from 'typeorm'; import { FoodManufacturersService } from '../foodManufacturers/manufacturers.service'; import { DonationItemsService } from '../donationItems/donationItems.service'; import { AllocationsService } from '../allocations/allocations.service'; @@ -792,11 +792,14 @@ describe('UsersService', () => { }); it('should not call Cognito methods when user has no Cognito account', async () => { - const volunteer = await testDataSource.getRepository(User).findOne({ - where: { role: Role.VOLUNTEER, userCognitoSub: IsNull() }, - }); + const userRepo = testDataSource.getRepository(User); + const volunteer = await userRepo.findOneBy({ role: Role.VOLUNTEER }); expect(volunteer).toBeDefined(); + // Ensure userCognitoSub is null + volunteer!.userCognitoSub = null; + await userRepo.save(volunteer!); + await service.promoteVolunteerToAdmin(volunteer!.id); expect(mockAuthService.addUserToGroup).not.toHaveBeenCalled(); From 2d30e74b1307a5ad030d7b34047f26210301bacc Mon Sep 17 00:00:00 2001 From: jxuistrying Date: Mon, 8 Jun 2026 14:40:07 -0400 Subject: [PATCH 8/8] remove userCognitoSub check --- apps/backend/src/users/users.service.spec.ts | 28 ++------------------ apps/backend/src/users/users.service.ts | 7 ++--- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/apps/backend/src/users/users.service.spec.ts b/apps/backend/src/users/users.service.spec.ts index 01867d5b3..e6137f99b 100644 --- a/apps/backend/src/users/users.service.spec.ts +++ b/apps/backend/src/users/users.service.spec.ts @@ -768,17 +768,12 @@ describe('UsersService', () => { expect(assignments).toHaveLength(0); }); - it('should call Cognito addUserToGroup and removeUserFromGroup when user has Cognito account', async () => { - const userRepo = testDataSource.getRepository(User); - const volunteer = await userRepo.findOne({ + it('should call Cognito addUserToGroup and removeUserFromGroup', async () => { + const volunteer = await testDataSource.getRepository(User).findOne({ where: { role: Role.VOLUNTEER }, }); expect(volunteer).toBeDefined(); - // Set userCognitoSub to simulate a user with a Cognito account - volunteer!.userCognitoSub = 'test-cognito-sub'; - await userRepo.save(volunteer!); - await service.promoteVolunteerToAdmin(volunteer!.id); expect(mockAuthService.addUserToGroup).toHaveBeenCalledWith( @@ -791,21 +786,6 @@ describe('UsersService', () => { ); }); - it('should not call Cognito methods when user has no Cognito account', async () => { - const userRepo = testDataSource.getRepository(User); - const volunteer = await userRepo.findOneBy({ role: Role.VOLUNTEER }); - expect(volunteer).toBeDefined(); - - // Ensure userCognitoSub is null - volunteer!.userCognitoSub = null; - await userRepo.save(volunteer!); - - await service.promoteVolunteerToAdmin(volunteer!.id); - - expect(mockAuthService.addUserToGroup).not.toHaveBeenCalled(); - expect(mockAuthService.removeUserFromGroup).not.toHaveBeenCalled(); - }); - it('should throw NotFoundException when user does not exist', async () => { await expect(service.promoteVolunteerToAdmin(99999)).rejects.toThrow( NotFoundException, @@ -852,10 +832,6 @@ describe('UsersService', () => { }); expect(volunteer).toBeDefined(); - // Set userCognitoSub so the Cognito code path is triggered - volunteer!.userCognitoSub = 'test-cognito-sub'; - await userRepo.save(volunteer!); - mockAuthService.addUserToGroup.mockRejectedValueOnce( new InternalServerErrorException('Cognito error'), ); diff --git a/apps/backend/src/users/users.service.ts b/apps/backend/src/users/users.service.ts index 6dd16a747..3998a134a 100644 --- a/apps/backend/src/users/users.service.ts +++ b/apps/backend/src/users/users.service.ts @@ -326,11 +326,8 @@ export class UsersService { user.pantries = []; await userRepo.save(user); - // Only update Cognito groups if user has a Cognito account - if (user.userCognitoSub) { - await this.authService.addUserToGroup(user.email, 'admin'); - await this.authService.removeUserFromGroup(user.email, 'volunteer'); - } + await this.authService.addUserToGroup(user.email, 'admin'); + await this.authService.removeUserFromGroup(user.email, 'volunteer'); }); } }