[SSF-171] update volunteers endpoint

apps/backend/src/pantries/dtos/update-pantry-volunteers-dto.ts

@@ -0,0 +1,16 @@
+import { IsArray, IsInt, IsOptional } from 'class-validator';
+import { Type } from 'class-transformer';
+
+export class UpdatePantryVolunteersDto {
+  @IsOptional()
+  @IsArray()
+  @IsInt({ each: true })
+  @Type(() => Number)
+  addVolunteerIds?: number[];
+
+  @IsOptional()
+  @IsArray()
+  @IsInt({ each: true })
+  @Type(() => Number)
+  removeVolunteerIds?: number[];
+}

apps/backend/src/pantries/pantries.controller.spec.ts

@@ -22,6 +22,7 @@ import { ApplicationStatus } from '../shared/types';
 import { User } from '../users/users.entity';
 import { AuthenticatedRequest } from '../auth/authenticated-request';
 import { UpdatePantryApplicationDto } from './dtos/update-pantry-application.dto';
+import { BadRequestException } from '@nestjs/common';
 
 const mockPantriesService = mock<PantriesService>();
 const mockOrdersService = mock<OrdersService>();
@@ -357,17 +358,29 @@ describe('PantriesController', () => {
   });
 
   describe('updatePantryVolunteers', () => {
-    it('should overwrite the set of volunteers assigned to a pantry', async () => {
+    it('should call pantriesService.updatePantryVolunteers with add and remove lists', async () => {
       const pantryId = 1;
-      const volunteerIds = [10, 11, 12];
+      const addVolunteerIds = [10, 11, 12];
+      const removeVolunteerIds = [1, 2];
 
       mockPantriesService.updatePantryVolunteers.mockResolvedValue(undefined);
 
-      await controller.updatePantryVolunteers(pantryId, volunteerIds);
+      await controller.updatePantryVolunteers(pantryId, {
+        addVolunteerIds,
+        removeVolunteerIds,
+      });
 
       expect(mockPantriesService.updatePantryVolunteers).toHaveBeenCalledWith(
         pantryId,
-        volunteerIds,
+        { addVolunteerIds, removeVolunteerIds },
+      );
+    });
+
+    it('should throw BadRequestException if neither ID lists are given', async () => {
+      await expect(controller.updatePantryVolunteers(1, {})).rejects.toThrow(
+        new BadRequestException(
+          'At least one of addVolunteerIds or removeVolunteerIds must be provided',
+        ),
       );
     });
   });

apps/backend/src/pantries/pantries.controller.ts

@@ -1,11 +1,11 @@
 import {
+  BadRequestException,
   Body,
   Controller,
   Get,
   Param,
   ParseIntPipe,
   Patch,
-  Put,
   Post,
   Query,
   Req,
@@ -34,6 +34,7 @@ import { CheckOwnership, pipeNullable } from '../auth/ownership.decorator';
 import { Public } from '../auth/public.decorator';
 import { AuthenticatedRequest } from '../auth/authenticated-request';
 import { UpdatePantryApplicationDto } from './dtos/update-pantry-application.dto';
+import { UpdatePantryVolunteersDto } from './dtos/update-pantry-volunteers-dto';
 
 @Controller('pantries')
 export class PantriesController {
@@ -382,11 +383,21 @@ export class PantriesController {
   }
 
   @Roles(Role.ADMIN)
-  @Put('/:pantryId/volunteers')
+  @Patch('/:pantryId/volunteers')
   async updatePantryVolunteers(
     @Param('pantryId', ParseIntPipe) pantryId: number,
-    @Body('volunteerIds') volunteerIds: number[],
+    @Body(new ValidationPipe())
+    body: UpdatePantryVolunteersDto,
   ): Promise<void> {
-    return this.pantriesService.updatePantryVolunteers(pantryId, volunteerIds);
+    const { addVolunteerIds, removeVolunteerIds } = body;
+    if (
+      (!addVolunteerIds || addVolunteerIds.length === 0) &&
+      (!removeVolunteerIds || removeVolunteerIds.length === 0)
+    ) {
+      throw new BadRequestException(
+        'At least one of addVolunteerIds or removeVolunteerIds must be provided',
+      );
+    }
+    return this.pantriesService.updatePantryVolunteers(pantryId, body);
   }
 }

apps/backend/src/pantries/pantries.service.spec.ts

@@ -805,7 +805,8 @@ describe('PantriesService', () => {
       const saved = await testDataSource.getRepository(Pantry).findOne({
         where: { pantryName: 'No Volunteer Pantry' },
       });
-      await testDataSource.getRepository(Pantry).update(saved!.pantryId, {
+      if (!saved) throw new Error('Pantry not found after creation');
+      await testDataSource.getRepository(Pantry).update(saved.pantryId, {
         status: ApplicationStatus.APPROVED,
       });
 
@@ -827,42 +828,166 @@ describe('PantriesService', () => {
   });
 
   describe('updatePantryVolunteers', () => {
-    const getVolunteerId = async (email: string) =>
-      (
-        await testDataSource.query(
-          `SELECT user_id FROM users WHERE email = $1 LIMIT 1`,
-          [email],
-        )
-      )[0].user_id;
+    it('adds volunteers to a pantry', async () => {
+      await service.updatePantryVolunteers(1, {
+        addVolunteerIds: [7],
+        removeVolunteerIds: [],
+      });
+      const pantry = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      expect(pantry?.volunteers?.map((v) => v.id)).toContain(7);
+    });
+
+    it('removes volunteers from a pantry', async () => {
+      await service.updatePantryVolunteers(1, {
+        addVolunteerIds: [],
+        removeVolunteerIds: [6],
+      });
+      const pantry = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      expect(pantry?.volunteers?.map((v) => v.id)).not.toContain(6);
+    });
+
+    it('adds and removes volunteers in a single request', async () => {
+      await service.updatePantryVolunteers(1, {
+        addVolunteerIds: [8],
+        removeVolunteerIds: [6, 9],
+      });
+      const pantry = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      const ids = pantry?.volunteers?.map((v) => v.id);
+      expect(ids).toContain(8);
+      expect(ids).not.toContain(6);
+      expect(ids).not.toContain(9);
+    });
 
-    it('replaces volunteer set', async () => {
-      const williamId = Number(await getVolunteerId('william.m@volunteer.org'));
-      await service.updatePantryVolunteers(1, [williamId]);
+    it('silently ignores adding an already-assigned volunteer', async () => {
+      await service.updatePantryVolunteers(1, {
+        addVolunteerIds: [6],
+        removeVolunteerIds: [],
+      });
       const pantry = await testDataSource
         .getRepository(Pantry)
         .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
-      expect(pantry?.volunteers).toHaveLength(1);
-      expect(pantry?.volunteers?.[0].id).toBe(williamId);
+      const ids = pantry?.volunteers?.map((v) => v.id);
+      expect(ids?.filter((id) => id === 6)).toHaveLength(1);
+    });
+
+    it('silently ignores removing a volunteer not assigned to the pantry', async () => {
+      const pantryBefore = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      await service.updatePantryVolunteers(1, {
+        addVolunteerIds: [],
+        removeVolunteerIds: [8],
+      });
+      const pantryAfter = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      expect(pantryBefore?.volunteers).toEqual(pantryAfter?.volunteers);
+    });
+
+    it('throws BadRequestException for duplicate IDs in addVolunteerIds', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [7, 7],
+          removeVolunteerIds: [],
+        }),
+      ).rejects.toThrow(
+        new BadRequestException('addVolunteerIds contains duplicate values'),
+      );
+    });
+
+    it('throws BadRequestException for duplicate IDs in removeVolunteerIds', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [],
+          removeVolunteerIds: [6, 6],
+        }),
+      ).rejects.toThrow(
+        new BadRequestException('removeVolunteerIds contains duplicate values'),
+      );
+    });
+
+    it('throws BadRequestException when same ID appears in both add and remove lists', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [6],
+          removeVolunteerIds: [6],
+        }),
+      ).rejects.toThrow(
+        new BadRequestException(
+          'The following ID(s) appear in both the add and remove lists: 6',
+        ),
+      );
     });
 
     it('throws NotFoundException when pantry not found', async () => {
-      const williamId = Number(await getVolunteerId('william.m@volunteer.org'));
       await expect(
-        service.updatePantryVolunteers(9999, [williamId]),
+        service.updatePantryVolunteers(9999, {
+          addVolunteerIds: [6],
+          removeVolunteerIds: [],
+        }),
       ).rejects.toThrow(NotFoundException);
     });
 
-    it('throws NotFoundException when volunteer id does not exist', async () => {
-      await expect(service.updatePantryVolunteers(1, [99999])).rejects.toThrow(
-        NotFoundException,
-      );
+    it('throws NotFoundException when volunteer ID does not exist', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [99999],
+          removeVolunteerIds: [],
+        }),
+      ).rejects.toThrow(new NotFoundException('Users not found: 99999'));
+    });
+
+    it('throws NotFoundException when some volunteer IDs do not exist', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [7, 99999],
+          removeVolunteerIds: [],
+        }),
+      ).rejects.toThrow(new NotFoundException('Users not found: 99999'));
     });
 
     it('throws BadRequestException when user is not a volunteer', async () => {
-      const adminId = Number(await getVolunteerId('john.smith@ssf.org'));
       await expect(
-        service.updatePantryVolunteers(1, [adminId]),
-      ).rejects.toThrow(BadRequestException);
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [1],
+          removeVolunteerIds: [],
+        }),
+      ).rejects.toThrow(
+        new BadRequestException('User(s) 1 are not volunteers'),
+      );
+    });
+
+    it('throws BadRequestException when some users are not volunteers in a mixed list', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, {
+          addVolunteerIds: [6, 1],
+          removeVolunteerIds: [],
+        }),
+      ).rejects.toThrow(
+        new BadRequestException('User(s) 1 are not volunteers'),
+      );
+    });
+
+    it('handles pantry with empty volunteers array', async () => {
+      const pantryId = 4;
+      const pantryBefore = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId }, relations: ['volunteers'] });
+      expect(pantryBefore?.volunteers).toHaveLength(0);
+      await service.updatePantryVolunteers(pantryId, {
+        addVolunteerIds: [7],
+        removeVolunteerIds: [],
+      });
+      const pantryAfter = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId }, relations: ['volunteers'] });
+      expect(pantryAfter?.volunteers?.map((v) => v.id)).toContain(7);
     });
   });
 });