[SSF-171] update volunteers endpoint

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

@@ -357,17 +357,23 @@ 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,
       );
     });
   });

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

@@ -5,7 +5,6 @@ import {
   Param,
   ParseIntPipe,
   Patch,
-  Put,
   Post,
   Query,
   Req,
@@ -382,11 +381,16 @@ export class PantriesController {
   }
 
   @Roles(Role.ADMIN)
-  @Put('/:pantryId/volunteers')
+  @Patch('/:pantryId/volunteers')
   async updatePantryVolunteers(
     @Param('pantryId', ParseIntPipe) pantryId: number,
-    @Body('volunteerIds') volunteerIds: number[],
+    @Body('addVolunteerIds') addVolunteerIds: number[],
+    @Body('removeVolunteerIds') removeVolunteerIds: number[],
   ): Promise<void> {
-    return this.pantriesService.updatePantryVolunteers(pantryId, volunteerIds);
+    return this.pantriesService.updatePantryVolunteers(
+      pantryId,
+      addVolunteerIds,
+      removeVolunteerIds,
+    );
   }
 }

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,98 @@ 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, [7], []);
+      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, [], [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, [8], [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('silently ignores adding an already-assigned volunteer', async () => {
+      await service.updatePantryVolunteers(1, [6], []);
+      const pantry = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      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, [], [8]);
+      const pantryAfter = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      expect(pantryBefore?.volunteers).toEqual(pantryAfter?.volunteers);
+    });
 
-    it('replaces volunteer set', async () => {
-      const williamId = Number(await getVolunteerId('william.m@volunteer.org'));
-      await service.updatePantryVolunteers(1, [williamId]);
+    it('handles duplicate IDs in addVolunteerIds without constraint violation', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, [7, 7], []),
+      ).resolves.not.toThrow();
       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 === 7)).toHaveLength(1);
+    });
+
+    it('handles duplicate IDs in removeVolunteerIds', async () => {
+      await expect(
+        service.updatePantryVolunteers(1, [], [6, 6]),
+      ).resolves.not.toThrow();
+      const pantry = await testDataSource
+        .getRepository(Pantry)
+        .findOne({ where: { pantryId: 1 }, relations: ['volunteers'] });
+      expect(pantry?.volunteers?.map((v) => v.id)).not.toContain(6);
+    });
+
+    it('throws BadRequestException when same ID appears in both add and remove lists', async () => {
+      await expect(service.updatePantryVolunteers(1, [6], [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, [6], []),
       ).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, [99999], []),
+      ).rejects.toThrow(new NotFoundException('User 99999 not found'));
     });
 
     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);
+      await expect(service.updatePantryVolunteers(1, [1], [])).rejects.toThrow(
+        new BadRequestException('User(s) 1 are not volunteers'),
+      );
     });
   });
 });

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

@@ -332,7 +332,7 @@ export class PantriesService {
         pantryMessage.subject,
         pantryMessage.bodyHTML,
       );
-    } catch (error) {
+    } catch {
       throw new InternalServerErrorException(
         'Failed to send pantry application submitted confirmation email to representative',
       );
@@ -345,7 +345,7 @@ export class PantriesService {
         adminMessage.subject,
         adminMessage.bodyHTML,
       );
-    } catch (error) {
+    } catch {
       throw new InternalServerErrorException(
         'Failed to send new pantry application notification email to SSF',
       );
@@ -419,7 +419,7 @@ export class PantriesService {
         message.subject,
         message.bodyHTML,
       );
-    } catch (error) {
+    } catch {
       throw new InternalServerErrorException(
         'Failed to send pantry account approved notification email to representative',
       );
@@ -465,10 +465,25 @@ export class PantriesService {
 
   async updatePantryVolunteers(
     pantryId: number,
-    volunteerIds: number[],
+    addVolunteerIds: number[],
+    removeVolunteerIds: number[],
   ): Promise<void> {
     validateId(pantryId, 'Pantry');
-    volunteerIds.forEach((id) => validateId(id, 'Volunteer'));
+
+    const overlap = addVolunteerIds.filter((id) =>
+      removeVolunteerIds.includes(id),
+    );
+    if (overlap.length > 0) {
+      throw new BadRequestException(
+        `The following ID(s) appear in both the add and remove lists: ${overlap.join(
+          ', ',
+        )}`,
+      );
+    }
+
+    const uniqueAddIds = [...new Set(addVolunteerIds)];
+    const allVolunteerIds = [...uniqueAddIds, ...removeVolunteerIds];
+    allVolunteerIds.forEach((id) => validateId(id, 'Volunteer'));
 
     const pantry = await this.repo.findOne({
       where: { pantryId },
@@ -480,24 +495,32 @@ export class PantriesService {
     }
 
     const users = await Promise.all(
-      volunteerIds.map((id) => this.usersService.findOne(id)),
+      allVolunteerIds.map((id) => this.usersService.findOne(id)),
     );
 
-    if (users.length !== volunteerIds.length) {
-      throw new NotFoundException('One or more users not found');
-    }
-
     const nonVolunteers = users.filter((user) => user.role !== Role.VOLUNTEER);
 
     if (nonVolunteers.length > 0) {
       throw new BadRequestException(
-        `Users ${nonVolunteers
+        `User(s) ${nonVolunteers
           .map((user) => user.id)
           .join(', ')} are not volunteers`,
       );
     }
 
-    pantry.volunteers = users;
+    const usersToAdd = users.filter((u) => uniqueAddIds.includes(u.id));
+
+    const currentVolunteers = pantry.volunteers ?? [];
+    const filteredVolunteers = currentVolunteers.filter(
+      (v) => !removeVolunteerIds.includes(v.id),
+    );
+
+    const existingVolunteerIds = new Set(filteredVolunteers.map((v) => v.id));
+    const volunteersToAdd = usersToAdd.filter(
+      (u) => !existingVolunteerIds.has(u.id),
+    );
+
+    pantry.volunteers = [...filteredVolunteers, ...volunteersToAdd];
     await this.repo.save(pantry);
   }
 

apps/backend/src/users/users.service.ts

@@ -81,7 +81,7 @@ export class UsersService {
           message.subject,
           message.bodyHTML,
         );
-      } catch (error) {
+      } catch {
         throw new InternalServerErrorException(
           'Failed to send account created notification email to volunteer',
         );