From 5d2fd3e30ae90204c6af4e87a54650a525681170 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 11:21:47 -0400 Subject: [PATCH 1/9] Add MatchesService.findByVolunteerId Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/matches/matches.service.spec.ts | 39 ++++++++++++++++++- apps/backend/src/matches/matches.service.ts | 4 ++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/apps/backend/src/matches/matches.service.spec.ts b/apps/backend/src/matches/matches.service.spec.ts index b2d520821..093c550e5 100644 --- a/apps/backend/src/matches/matches.service.spec.ts +++ b/apps/backend/src/matches/matches.service.spec.ts @@ -2,17 +2,21 @@ import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; import { MatchesService } from './matches.service'; import { Match } from './matches.entity'; +import { MatchStatus } from './matches.types'; describe('MatchesService', () => { let service: MatchesService; + let repo: { find: jest.Mock }; beforeEach(async () => { + repo = { find: jest.fn() }; + const module: TestingModule = await Test.createTestingModule({ providers: [ MatchesService, { provide: getRepositoryToken(Match), - useValue: {}, + useValue: repo, }, ], }).compile(); @@ -23,4 +27,37 @@ describe('MatchesService', () => { it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('findByVolunteerId', () => { + it('returns the matches for the volunteer', async () => { + const matches = [ + { + matchId: 1, + volunteerId: 7, + chameleonAnimalId: 42, + status: MatchStatus.PENDING, + deniedReason: null, + }, + { + matchId: 2, + volunteerId: 7, + chameleonAnimalId: 43, + status: MatchStatus.DENIED, + deniedReason: 'Resident dog is not cat-friendly', + }, + ] as Match[]; + repo.find.mockResolvedValue(matches); + + const result = await service.findByVolunteerId(7); + + expect(result).toBe(matches); + expect(repo.find).toHaveBeenCalledWith({ where: { volunteerId: 7 } }); + }); + + it('returns an empty array when the volunteer has no matches', async () => { + repo.find.mockResolvedValue([]); + + await expect(service.findByVolunteerId(7)).resolves.toEqual([]); + }); + }); }); diff --git a/apps/backend/src/matches/matches.service.ts b/apps/backend/src/matches/matches.service.ts index e91ccaf65..da322e372 100644 --- a/apps/backend/src/matches/matches.service.ts +++ b/apps/backend/src/matches/matches.service.ts @@ -9,4 +9,8 @@ export class MatchesService { @InjectRepository(Match) private repo: Repository, ) {} + + findByVolunteerId(volunteerId: number): Promise { + return this.repo.find({ where: { volunteerId } }); + } } From 2749793c7ec53451c44ac686d1dd9c02c41ec10f Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 11:22:14 -0400 Subject: [PATCH 2/9] Add VolunteersService.existsById Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/volunteers/volunteers.service.spec.ts | 24 ++++++++++++++++++- .../src/volunteers/volunteers.service.ts | 19 +++------------ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/apps/backend/src/volunteers/volunteers.service.spec.ts b/apps/backend/src/volunteers/volunteers.service.spec.ts index fda546818..20330c8ad 100644 --- a/apps/backend/src/volunteers/volunteers.service.spec.ts +++ b/apps/backend/src/volunteers/volunteers.service.spec.ts @@ -5,14 +5,17 @@ import { FosterVolunteer } from './volunteers.entity'; describe('VolunteersService', () => { let service: VolunteersService; + let repo: { existsBy: jest.Mock }; beforeEach(async () => { + repo = { existsBy: jest.fn() }; + const module: TestingModule = await Test.createTestingModule({ providers: [ VolunteersService, { provide: getRepositoryToken(FosterVolunteer), - useValue: {}, + useValue: repo, }, ], }).compile(); @@ -23,4 +26,23 @@ describe('VolunteersService', () => { it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('existsById', () => { + it('returns true when a volunteer with the id exists', async () => { + repo.existsBy.mockResolvedValue(true); + + const result = await service.existsById(7); + + expect(result).toBe(true); + expect(repo.existsBy).toHaveBeenCalledWith({ volunteerId: 7 }); + }); + + it('returns false when no volunteer with the id exists', async () => { + repo.existsBy.mockResolvedValue(false); + + const result = await service.existsById(7); + + expect(result).toBe(false); + }); + }); }); diff --git a/apps/backend/src/volunteers/volunteers.service.ts b/apps/backend/src/volunteers/volunteers.service.ts index 1c6f17a61..a44b9f7e0 100644 --- a/apps/backend/src/volunteers/volunteers.service.ts +++ b/apps/backend/src/volunteers/volunteers.service.ts @@ -10,20 +10,7 @@ export class VolunteersService { private repo: Repository, ) {} - // Example service functions - // find(email: string) { - // return this.repo.find({ where: { email } }); - // } - - // async update(id: number, attrs: Partial) { - // const user = await this.findOne(id); - - // if (!user) { - // throw new NotFoundException('User not found'); - // } - - // Object.assign(user, attrs); - - // return this.repo.save(user); - // } + existsById(id: number): Promise { + return this.repo.existsBy({ volunteerId: id }); + } } From d1913d0feb488f9819d384acf2a76e19d3b3b5cc Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 11:22:57 -0400 Subject: [PATCH 3/9] Add GET /volunteers/:volunteerId/matches Co-Authored-By: Claude Opus 5.5 (1M context) --- .../volunteers/volunteers.controller.spec.ts | 64 ++++++++++++++++++- .../src/volunteers/volunteers.controller.ts | 28 ++++++-- 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/apps/backend/src/volunteers/volunteers.controller.spec.ts b/apps/backend/src/volunteers/volunteers.controller.spec.ts index a3cdb3daa..c8cf10aba 100644 --- a/apps/backend/src/volunteers/volunteers.controller.spec.ts +++ b/apps/backend/src/volunteers/volunteers.controller.spec.ts @@ -1,17 +1,30 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { BadRequestException, NotFoundException } from '@nestjs/common'; import { VolunteersController } from './volunteers.controller'; import { VolunteersService } from './volunteers.service'; +import { MatchesService } from '../matches/matches.service'; +import { Match } from '../matches/matches.entity'; +import { MatchStatus } from '../matches/matches.types'; describe('VolunteersController', () => { let controller: VolunteersController; + let volunteersService: { existsById: jest.Mock }; + let matchesService: { findByVolunteerId: jest.Mock }; beforeEach(async () => { + volunteersService = { existsById: jest.fn() }; + matchesService = { findByVolunteerId: jest.fn() }; + const module: TestingModule = await Test.createTestingModule({ controllers: [VolunteersController], providers: [ { provide: VolunteersService, - useValue: {}, + useValue: volunteersService, + }, + { + provide: MatchesService, + useValue: matchesService, }, ], }).compile(); @@ -22,4 +35,53 @@ describe('VolunteersController', () => { it('should be defined', () => { expect(controller).toBeDefined(); }); + + describe('getVolunteerMatches', () => { + it("returns the volunteer's matches", async () => { + const matches = [ + { + matchId: 1, + volunteerId: 7, + chameleonAnimalId: 42, + status: MatchStatus.DENIED, + deniedReason: 'Schedule conflict', + }, + ] as Match[]; + volunteersService.existsById.mockResolvedValue(true); + matchesService.findByVolunteerId.mockResolvedValue(matches); + + const result = await controller.getVolunteerMatches('7'); + + expect(result).toBe(matches); + expect(volunteersService.existsById).toHaveBeenCalledWith(7); + expect(matchesService.findByVolunteerId).toHaveBeenCalledWith(7); + }); + + it('returns an empty array when the volunteer exists but has no matches', async () => { + volunteersService.existsById.mockResolvedValue(true); + matchesService.findByVolunteerId.mockResolvedValue([]); + + await expect(controller.getVolunteerMatches('7')).resolves.toEqual([]); + }); + + it('throws NotFoundException when the volunteer does not exist', async () => { + volunteersService.existsById.mockResolvedValue(false); + + await expect(controller.getVolunteerMatches('999')).rejects.toThrow( + new NotFoundException('Volunteer with ID 999 not found'), + ); + expect(matchesService.findByVolunteerId).not.toHaveBeenCalled(); + }); + + it.each(['abc', '0', '-3', ''])( + 'throws BadRequestException for invalid id %p', + async (invalidId) => { + await expect(controller.getVolunteerMatches(invalidId)).rejects.toThrow( + BadRequestException, + ); + expect(volunteersService.existsById).not.toHaveBeenCalled(); + expect(matchesService.findByVolunteerId).not.toHaveBeenCalled(); + }, + ); + }); }); diff --git a/apps/backend/src/volunteers/volunteers.controller.ts b/apps/backend/src/volunteers/volunteers.controller.ts index d2c246fc8..e7c1b48dc 100644 --- a/apps/backend/src/volunteers/volunteers.controller.ts +++ b/apps/backend/src/volunteers/volunteers.controller.ts @@ -1,15 +1,29 @@ -import { Controller } from '@nestjs/common'; +import { Controller, Get, NotFoundException, Param } from '@nestjs/common'; import { VolunteersService } from './volunteers.service'; +import { MatchesService } from '../matches/matches.service'; +import { Match } from '../matches/matches.entity'; +import { validateId } from '../utils/validation.utils'; // @ApiTags('Volunteers') // @ApiBearerAuth() @Controller('volunteers') export class VolunteersController { - constructor(private volunteersService: VolunteersService) {} + constructor( + private volunteersService: VolunteersService, + private matchesService: MatchesService, + ) {} - // Example endpoint - // @Get('/:userId') - // async getUser(@Param('userId', ParseIntPipe) userId: number): Promise { - // return this.usersService.findOne(userId); - // } + @Get('/:volunteerId/matches') + async getVolunteerMatches( + @Param('volunteerId') volunteerId: string, + ): Promise { + const id = Number(volunteerId); + validateId(id, 'volunteer'); + + if (!(await this.volunteersService.existsById(id))) { + throw new NotFoundException(`Volunteer with ID ${id} not found`); + } + + return this.matchesService.findByVolunteerId(id); + } } From 5dc6ac05163bca59fc33ccad2f00eb2e23a116e3 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 11:24:13 -0400 Subject: [PATCH 4/9] Wire VolunteersModule so /volunteers/:id/matches is served Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/backend/src/app.module.ts | 2 + .../src/volunteers/volunteers.module.spec.ts | 80 +++++++++++++++++++ .../src/volunteers/volunteers.module.ts | 11 ++- 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 apps/backend/src/volunteers/volunteers.module.spec.ts diff --git a/apps/backend/src/app.module.ts b/apps/backend/src/app.module.ts index faede2748..de33e1d0b 100644 --- a/apps/backend/src/app.module.ts +++ b/apps/backend/src/app.module.ts @@ -3,6 +3,7 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { ConfigModule, ConfigService } from '@nestjs/config'; import typeorm from './config/typeorm'; import { CognitoModule } from './aws/cognito/cognito.module'; +import { VolunteersModule } from './volunteers/volunteers.module'; @Module({ imports: [ @@ -16,6 +17,7 @@ import { CognitoModule } from './aws/cognito/cognito.module'; configService.getOrThrow('typeorm'), }), CognitoModule, + VolunteersModule, ], }) export class AppModule {} diff --git a/apps/backend/src/volunteers/volunteers.module.spec.ts b/apps/backend/src/volunteers/volunteers.module.spec.ts new file mode 100644 index 000000000..2282b98e3 --- /dev/null +++ b/apps/backend/src/volunteers/volunteers.module.spec.ts @@ -0,0 +1,80 @@ +import { INestApplication } from '@nestjs/common'; +import { Test } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { VolunteersModule } from './volunteers.module'; +import { FosterVolunteer } from './volunteers.entity'; +import { Match } from '../matches/matches.entity'; +import { FosterCoordinator } from '../coordinators/coordinators.entity'; +import { MatchStatus } from '../matches/matches.types'; + +describe('VolunteersModule (HTTP)', () => { + let app: INestApplication; + let baseUrl: string; + const volunteerRepo = { existsBy: jest.fn() }; + const matchRepo = { find: jest.fn() }; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [VolunteersModule], + }) + .overrideProvider(getRepositoryToken(FosterVolunteer)) + .useValue(volunteerRepo) + .overrideProvider(getRepositoryToken(Match)) + .useValue(matchRepo) + .overrideProvider(getRepositoryToken(FosterCoordinator)) + .useValue({}) + .compile(); + + app = moduleRef.createNestApplication(); + await app.listen(0); + baseUrl = await app.getUrl(); + }); + + afterAll(async () => { + await app.close(); + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + const getMatches = (id: string) => + fetch(`${baseUrl}/volunteers/${id}/matches`); + + it('returns 200 with the matches', async () => { + const match = { + matchId: 1, + volunteerId: 7, + chameleonAnimalId: 42, + status: MatchStatus.PENDING, + deniedReason: null, + }; + volunteerRepo.existsBy.mockResolvedValue(true); + matchRepo.find.mockResolvedValue([match]); + + const res = await getMatches('7'); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual([match]); + }); + + it('returns 200 with [] when the volunteer has no matches', async () => { + volunteerRepo.existsBy.mockResolvedValue(true); + matchRepo.find.mockResolvedValue([]); + + const res = await getMatches('7'); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual([]); + }); + + it('returns 404 for an unknown volunteer', async () => { + volunteerRepo.existsBy.mockResolvedValue(false); + + expect((await getMatches('999')).status).toBe(404); + }); + + it('returns 400 for a non-numeric id', async () => { + expect((await getMatches('abc')).status).toBe(400); + }); +}); diff --git a/apps/backend/src/volunteers/volunteers.module.ts b/apps/backend/src/volunteers/volunteers.module.ts index d3fef67b4..25e68c9e7 100644 --- a/apps/backend/src/volunteers/volunteers.module.ts +++ b/apps/backend/src/volunteers/volunteers.module.ts @@ -3,9 +3,18 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { FosterVolunteer } from './volunteers.entity'; import { VolunteersController } from './volunteers.controller'; import { VolunteersService } from './volunteers.service'; +import { CoordinatorsModule } from '../coordinators/coordinators.module'; +import { MatchesModule } from '../matches/matches.module'; @Module({ - imports: [TypeOrmModule.forFeature([FosterVolunteer])], + // FosterVolunteer relates to FosterCoordinator, so CoordinatorsModule has to + // be registered too or autoLoadEntities leaves TypeORM unable to build that + // relation's metadata. + imports: [ + TypeOrmModule.forFeature([FosterVolunteer]), + CoordinatorsModule, + MatchesModule, + ], controllers: [VolunteersController], providers: [VolunteersService], exports: [VolunteersService], From b484ddb7081228e999e0a222f1a65c46e3ca7695 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 13:14:08 -0400 Subject: [PATCH 5/9] Respect @Column names in PluralNamingStrategy Co-Authored-By: Claude Opus 5.5 (1M context) --- .../strategies/plural-naming.strategy.spec.ts | 17 +++++++++++++++++ .../src/strategies/plural-naming.strategy.ts | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 apps/backend/src/strategies/plural-naming.strategy.spec.ts diff --git a/apps/backend/src/strategies/plural-naming.strategy.spec.ts b/apps/backend/src/strategies/plural-naming.strategy.spec.ts new file mode 100644 index 000000000..bb6673169 --- /dev/null +++ b/apps/backend/src/strategies/plural-naming.strategy.spec.ts @@ -0,0 +1,17 @@ +import { PluralNamingStrategy } from './plural-naming.strategy'; + +describe('PluralNamingStrategy', () => { + const strategy = new PluralNamingStrategy(); + + describe('columnName', () => { + it('uses the name given in @Column', () => { + expect(strategy.columnName('volunteerId', 'volunteer_id')).toBe( + 'volunteer_id', + ); + }); + + it('falls back to the property name', () => { + expect(strategy.columnName('phone', '')).toBe('phone'); + }); + }); +}); diff --git a/apps/backend/src/strategies/plural-naming.strategy.ts b/apps/backend/src/strategies/plural-naming.strategy.ts index 1a7c0a156..c1acbf17e 100644 --- a/apps/backend/src/strategies/plural-naming.strategy.ts +++ b/apps/backend/src/strategies/plural-naming.strategy.ts @@ -8,8 +8,8 @@ export class PluralNamingStrategy return userSpecifiedName || targetName.toLowerCase() + 's'; // Pluralize the table name } - columnName(propertyName: string): string { - return propertyName; + columnName(propertyName: string, customName: string | undefined): string { + return customName || propertyName; } relationName(propertyName: string): string { From a86160c1b26e03a4313996657c404a5f4388ebc0 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 13:53:20 -0400 Subject: [PATCH 6/9] Remove explanatory comments Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/backend/src/volunteers/volunteers.module.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/backend/src/volunteers/volunteers.module.ts b/apps/backend/src/volunteers/volunteers.module.ts index 25e68c9e7..4697b4cdd 100644 --- a/apps/backend/src/volunteers/volunteers.module.ts +++ b/apps/backend/src/volunteers/volunteers.module.ts @@ -7,9 +7,6 @@ import { CoordinatorsModule } from '../coordinators/coordinators.module'; import { MatchesModule } from '../matches/matches.module'; @Module({ - // FosterVolunteer relates to FosterCoordinator, so CoordinatorsModule has to - // be registered too or autoLoadEntities leaves TypeORM unable to build that - // relation's metadata. imports: [ TypeOrmModule.forFeature([FosterVolunteer]), CoordinatorsModule, From 99c6b3563d8fca586178d7c622a48262b842abfd Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 13:53:26 -0400 Subject: [PATCH 7/9] Restore scaffold example comments in volunteers controller and service Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/volunteers/volunteers.controller.ts | 6 ++++++ .../src/volunteers/volunteers.service.ts | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/apps/backend/src/volunteers/volunteers.controller.ts b/apps/backend/src/volunteers/volunteers.controller.ts index e7c1b48dc..6f7018bfd 100644 --- a/apps/backend/src/volunteers/volunteers.controller.ts +++ b/apps/backend/src/volunteers/volunteers.controller.ts @@ -13,6 +13,12 @@ export class VolunteersController { private matchesService: MatchesService, ) {} + // Example endpoint + // @Get('/:userId') + // async getUser(@Param('userId', ParseIntPipe) userId: number): Promise { + // return this.usersService.findOne(userId); + // } + @Get('/:volunteerId/matches') async getVolunteerMatches( @Param('volunteerId') volunteerId: string, diff --git a/apps/backend/src/volunteers/volunteers.service.ts b/apps/backend/src/volunteers/volunteers.service.ts index a44b9f7e0..ead6333f6 100644 --- a/apps/backend/src/volunteers/volunteers.service.ts +++ b/apps/backend/src/volunteers/volunteers.service.ts @@ -10,6 +10,23 @@ export class VolunteersService { private repo: Repository, ) {} + // Example service functions + // find(email: string) { + // return this.repo.find({ where: { email } }); + // } + + // async update(id: number, attrs: Partial) { + // const user = await this.findOne(id); + + // if (!user) { + // throw new NotFoundException('User not found'); + // } + + // Object.assign(user, attrs); + + // return this.repo.save(user); + // } + existsById(id: number): Promise { return this.repo.existsBy({ volunteerId: id }); } From d9d941273bc9a7db9549a46e10ea609ce108b3c5 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Sun, 27 Sep 2026 14:00:21 -0400 Subject: [PATCH 8/9] Remove volunteers module spec Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/volunteers/volunteers.module.spec.ts | 80 ------------------- 1 file changed, 80 deletions(-) delete mode 100644 apps/backend/src/volunteers/volunteers.module.spec.ts diff --git a/apps/backend/src/volunteers/volunteers.module.spec.ts b/apps/backend/src/volunteers/volunteers.module.spec.ts deleted file mode 100644 index 2282b98e3..000000000 --- a/apps/backend/src/volunteers/volunteers.module.spec.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { INestApplication } from '@nestjs/common'; -import { Test } from '@nestjs/testing'; -import { getRepositoryToken } from '@nestjs/typeorm'; -import { VolunteersModule } from './volunteers.module'; -import { FosterVolunteer } from './volunteers.entity'; -import { Match } from '../matches/matches.entity'; -import { FosterCoordinator } from '../coordinators/coordinators.entity'; -import { MatchStatus } from '../matches/matches.types'; - -describe('VolunteersModule (HTTP)', () => { - let app: INestApplication; - let baseUrl: string; - const volunteerRepo = { existsBy: jest.fn() }; - const matchRepo = { find: jest.fn() }; - - beforeAll(async () => { - const moduleRef = await Test.createTestingModule({ - imports: [VolunteersModule], - }) - .overrideProvider(getRepositoryToken(FosterVolunteer)) - .useValue(volunteerRepo) - .overrideProvider(getRepositoryToken(Match)) - .useValue(matchRepo) - .overrideProvider(getRepositoryToken(FosterCoordinator)) - .useValue({}) - .compile(); - - app = moduleRef.createNestApplication(); - await app.listen(0); - baseUrl = await app.getUrl(); - }); - - afterAll(async () => { - await app.close(); - }); - - beforeEach(() => { - jest.resetAllMocks(); - }); - - const getMatches = (id: string) => - fetch(`${baseUrl}/volunteers/${id}/matches`); - - it('returns 200 with the matches', async () => { - const match = { - matchId: 1, - volunteerId: 7, - chameleonAnimalId: 42, - status: MatchStatus.PENDING, - deniedReason: null, - }; - volunteerRepo.existsBy.mockResolvedValue(true); - matchRepo.find.mockResolvedValue([match]); - - const res = await getMatches('7'); - - expect(res.status).toBe(200); - expect(await res.json()).toEqual([match]); - }); - - it('returns 200 with [] when the volunteer has no matches', async () => { - volunteerRepo.existsBy.mockResolvedValue(true); - matchRepo.find.mockResolvedValue([]); - - const res = await getMatches('7'); - - expect(res.status).toBe(200); - expect(await res.json()).toEqual([]); - }); - - it('returns 404 for an unknown volunteer', async () => { - volunteerRepo.existsBy.mockResolvedValue(false); - - expect((await getMatches('999')).status).toBe(404); - }); - - it('returns 400 for a non-numeric id', async () => { - expect((await getMatches('abc')).status).toBe(400); - }); -}); From c788c170709d8f2982ffce06c4d99d3f60f656dd Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Tue, 29 Sep 2026 17:53:13 -0400 Subject: [PATCH 9/9] Respond to review comments Co-Authored-By: Claude Opus 5.5 --- .../strategies/plural-naming.strategy.spec.ts | 17 -------- .../src/strategies/plural-naming.strategy.ts | 8 ---- .../volunteers/volunteers.controller.spec.ts | 33 ++++++---------- .../src/volunteers/volunteers.controller.ts | 39 ++++++++++++------- .../src/volunteers/volunteers.service.spec.ts | 29 +++++++------- .../src/volunteers/volunteers.service.ts | 25 ++++-------- 6 files changed, 59 insertions(+), 92 deletions(-) delete mode 100644 apps/backend/src/strategies/plural-naming.strategy.spec.ts diff --git a/apps/backend/src/strategies/plural-naming.strategy.spec.ts b/apps/backend/src/strategies/plural-naming.strategy.spec.ts deleted file mode 100644 index bb6673169..000000000 --- a/apps/backend/src/strategies/plural-naming.strategy.spec.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { PluralNamingStrategy } from './plural-naming.strategy'; - -describe('PluralNamingStrategy', () => { - const strategy = new PluralNamingStrategy(); - - describe('columnName', () => { - it('uses the name given in @Column', () => { - expect(strategy.columnName('volunteerId', 'volunteer_id')).toBe( - 'volunteer_id', - ); - }); - - it('falls back to the property name', () => { - expect(strategy.columnName('phone', '')).toBe('phone'); - }); - }); -}); diff --git a/apps/backend/src/strategies/plural-naming.strategy.ts b/apps/backend/src/strategies/plural-naming.strategy.ts index c1acbf17e..03599d836 100644 --- a/apps/backend/src/strategies/plural-naming.strategy.ts +++ b/apps/backend/src/strategies/plural-naming.strategy.ts @@ -7,12 +7,4 @@ export class PluralNamingStrategy tableName(targetName: string, userSpecifiedName: string | undefined): string { return userSpecifiedName || targetName.toLowerCase() + 's'; // Pluralize the table name } - - columnName(propertyName: string, customName: string | undefined): string { - return customName || propertyName; - } - - relationName(propertyName: string): string { - return propertyName; - } } diff --git a/apps/backend/src/volunteers/volunteers.controller.spec.ts b/apps/backend/src/volunteers/volunteers.controller.spec.ts index c8cf10aba..f7ece72f3 100644 --- a/apps/backend/src/volunteers/volunteers.controller.spec.ts +++ b/apps/backend/src/volunteers/volunteers.controller.spec.ts @@ -1,5 +1,5 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { BadRequestException, NotFoundException } from '@nestjs/common'; +import { NotFoundException } from '@nestjs/common'; import { VolunteersController } from './volunteers.controller'; import { VolunteersService } from './volunteers.service'; import { MatchesService } from '../matches/matches.service'; @@ -8,11 +8,11 @@ import { MatchStatus } from '../matches/matches.types'; describe('VolunteersController', () => { let controller: VolunteersController; - let volunteersService: { existsById: jest.Mock }; + let volunteersService: { findByIdOrFail: jest.Mock }; let matchesService: { findByVolunteerId: jest.Mock }; beforeEach(async () => { - volunteersService = { existsById: jest.fn() }; + volunteersService = { findByIdOrFail: jest.fn() }; matchesService = { findByVolunteerId: jest.fn() }; const module: TestingModule = await Test.createTestingModule({ @@ -47,41 +47,32 @@ describe('VolunteersController', () => { deniedReason: 'Schedule conflict', }, ] as Match[]; - volunteersService.existsById.mockResolvedValue(true); + volunteersService.findByIdOrFail.mockResolvedValue({}); matchesService.findByVolunteerId.mockResolvedValue(matches); - const result = await controller.getVolunteerMatches('7'); + const result = await controller.getVolunteerMatches(7); expect(result).toBe(matches); - expect(volunteersService.existsById).toHaveBeenCalledWith(7); + expect(volunteersService.findByIdOrFail).toHaveBeenCalledWith(7); expect(matchesService.findByVolunteerId).toHaveBeenCalledWith(7); }); it('returns an empty array when the volunteer exists but has no matches', async () => { - volunteersService.existsById.mockResolvedValue(true); + volunteersService.findByIdOrFail.mockResolvedValue({}); matchesService.findByVolunteerId.mockResolvedValue([]); - await expect(controller.getVolunteerMatches('7')).resolves.toEqual([]); + await expect(controller.getVolunteerMatches(7)).resolves.toEqual([]); }); it('throws NotFoundException when the volunteer does not exist', async () => { - volunteersService.existsById.mockResolvedValue(false); + volunteersService.findByIdOrFail.mockRejectedValue( + new NotFoundException('Volunteer with ID 999 not found'), + ); - await expect(controller.getVolunteerMatches('999')).rejects.toThrow( + await expect(controller.getVolunteerMatches(999)).rejects.toThrow( new NotFoundException('Volunteer with ID 999 not found'), ); expect(matchesService.findByVolunteerId).not.toHaveBeenCalled(); }); - - it.each(['abc', '0', '-3', ''])( - 'throws BadRequestException for invalid id %p', - async (invalidId) => { - await expect(controller.getVolunteerMatches(invalidId)).rejects.toThrow( - BadRequestException, - ); - expect(volunteersService.existsById).not.toHaveBeenCalled(); - expect(matchesService.findByVolunteerId).not.toHaveBeenCalled(); - }, - ); }); }); diff --git a/apps/backend/src/volunteers/volunteers.controller.ts b/apps/backend/src/volunteers/volunteers.controller.ts index 6f7018bfd..a28c8aaa1 100644 --- a/apps/backend/src/volunteers/volunteers.controller.ts +++ b/apps/backend/src/volunteers/volunteers.controller.ts @@ -1,10 +1,17 @@ -import { Controller, Get, NotFoundException, Param } from '@nestjs/common'; +import { + Controller, + Get, + HttpStatus, + Param, + ParseIntPipe, +} from '@nestjs/common'; +import { ApiOperation, ApiParam, ApiResponse, ApiTags } from '@nestjs/swagger'; import { VolunteersService } from './volunteers.service'; import { MatchesService } from '../matches/matches.service'; import { Match } from '../matches/matches.entity'; import { validateId } from '../utils/validation.utils'; -// @ApiTags('Volunteers') +@ApiTags('Volunteers') // @ApiBearerAuth() @Controller('volunteers') export class VolunteersController { @@ -13,23 +20,25 @@ export class VolunteersController { private matchesService: MatchesService, ) {} - // Example endpoint - // @Get('/:userId') - // async getUser(@Param('userId', ParseIntPipe) userId: number): Promise { - // return this.usersService.findOne(userId); - // } - @Get('/:volunteerId/matches') + @ApiOperation({ summary: 'Get all Matches for a Volunteer' }) + @ApiParam({ + name: 'volunteerId', + type: Number, + description: 'ID of the Volunteer', + }) + @ApiResponse({ + status: HttpStatus.OK, + description: "The Volunteer's Matches", + type: [Match], + }) async getVolunteerMatches( - @Param('volunteerId') volunteerId: string, + @Param('volunteerId', ParseIntPipe) volunteerId: number, ): Promise { - const id = Number(volunteerId); - validateId(id, 'volunteer'); + validateId(volunteerId, 'Volunteer'); - if (!(await this.volunteersService.existsById(id))) { - throw new NotFoundException(`Volunteer with ID ${id} not found`); - } + await this.volunteersService.findByIdOrFail(volunteerId); - return this.matchesService.findByVolunteerId(id); + return this.matchesService.findByVolunteerId(volunteerId); } } diff --git a/apps/backend/src/volunteers/volunteers.service.spec.ts b/apps/backend/src/volunteers/volunteers.service.spec.ts index 20330c8ad..7ff6ef097 100644 --- a/apps/backend/src/volunteers/volunteers.service.spec.ts +++ b/apps/backend/src/volunteers/volunteers.service.spec.ts @@ -1,14 +1,15 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { NotFoundException } from '@nestjs/common'; import { getRepositoryToken } from '@nestjs/typeorm'; import { VolunteersService } from './volunteers.service'; import { FosterVolunteer } from './volunteers.entity'; describe('VolunteersService', () => { let service: VolunteersService; - let repo: { existsBy: jest.Mock }; + let repo: { findOneBy: jest.Mock }; beforeEach(async () => { - repo = { existsBy: jest.fn() }; + repo = { findOneBy: jest.fn() }; const module: TestingModule = await Test.createTestingModule({ providers: [ @@ -27,22 +28,24 @@ describe('VolunteersService', () => { expect(service).toBeDefined(); }); - describe('existsById', () => { - it('returns true when a volunteer with the id exists', async () => { - repo.existsBy.mockResolvedValue(true); + describe('findByIdOrFail', () => { + it('returns the volunteer when one with the id exists', async () => { + const volunteer = { volunteerId: 7 } as FosterVolunteer; + repo.findOneBy.mockResolvedValue(volunteer); - const result = await service.existsById(7); + const result = await service.findByIdOrFail(7); - expect(result).toBe(true); - expect(repo.existsBy).toHaveBeenCalledWith({ volunteerId: 7 }); + expect(result).toBe(volunteer); + expect(repo.findOneBy).toHaveBeenCalledWith({ volunteerId: 7 }); }); - it('returns false when no volunteer with the id exists', async () => { - repo.existsBy.mockResolvedValue(false); + it('throws NotFoundException when no volunteer with the id exists', async () => { + repo.findOneBy.mockResolvedValue(null); - const result = await service.existsById(7); - - expect(result).toBe(false); + await expect(service.findByIdOrFail(7)).rejects.toThrow( + new NotFoundException('Volunteer with ID 7 not found'), + ); + expect(repo.findOneBy).toHaveBeenCalledWith({ volunteerId: 7 }); }); }); }); diff --git a/apps/backend/src/volunteers/volunteers.service.ts b/apps/backend/src/volunteers/volunteers.service.ts index ead6333f6..6da786853 100644 --- a/apps/backend/src/volunteers/volunteers.service.ts +++ b/apps/backend/src/volunteers/volunteers.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, NotFoundException } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { FosterVolunteer } from './volunteers.entity'; @@ -10,24 +10,13 @@ export class VolunteersService { private repo: Repository, ) {} - // Example service functions - // find(email: string) { - // return this.repo.find({ where: { email } }); - // } + async findByIdOrFail(id: number): Promise { + const volunteer = await this.repo.findOneBy({ volunteerId: id }); - // async update(id: number, attrs: Partial) { - // const user = await this.findOne(id); + if (!volunteer) { + throw new NotFoundException(`Volunteer with ID ${id} not found`); + } - // if (!user) { - // throw new NotFoundException('User not found'); - // } - - // Object.assign(user, attrs); - - // return this.repo.save(user); - // } - - existsById(id: number): Promise { - return this.repo.existsBy({ volunteerId: id }); + return volunteer; } }