Conversation
Lets foster coordinators recommend a Chameleon animal to a volunteer before a formal match exists. New recommendations default to isActive: true and the endpoint returns 200 with the created row. - CreateRecommendationDTO validates volunteerId and chameleonAnimalId as positive integers; the controller re-checks with validateId so the guard holds when the handler is called directly - 404 when the volunteer does not exist, via VolunteersService.existsById - validateId now rejects non-integers, which the "valid integer" rule needs - Registers RecommendationsModule on the app module Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code review caught that registering RecommendationsModule made the app fail to boot. It pulls in VolunteersModule, the first forFeature module reachable from AppModule, so autoLoadEntities handed TypeORM FosterVolunteer without FosterCoordinator - which its assignedCoordinator relation targets - and metadata building threw before app.listen. No test caught it because none stand up the module graph. - VolunteersModule now imports CoordinatorsModule, so it is self-contained wherever it is registered rather than relying on the app module - recommendations.module.spec asserts the entity graph reachable from the module is closed under relations; it fails without the fix above - Service upserts on the composite key instead of save(), so two coordinators recommending the same animal at once cannot race into a primary key violation surfacing as a 500 - Pin the 200 status the ticket requires, so dropping @httpcode fails Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dburkhart07
left a comment
There was a problem hiding this comment.
some initial things but looks really good so far. learned a thing or to about repo commands from this one 🐢
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| summary: 'Recommend a Chameleon Animal to an active Volunteer', | ||
| }) | ||
| @ApiResponse({ | ||
| status: HttpStatus.CREATED, |
There was a problem hiding this comment.
Should this endpoint be returning 200 OK instead of Nest’s default 201 Created? Since the ticket calls for POST /api/recommendations to return 200 with the saved recommendation
There was a problem hiding this comment.
@dburkhart07 thoughts? after saying to drop the @httpcode(HttpStatus.OK) override
There was a problem hiding this comment.
good callouts! i think the ticket was a mistake, we should be returning a 201 here for a created, since thats what all POST endpoints should have.
- Document RecommendationsService.create and VolunteersService.findActiveOrFail with summary, context, @PARAM, @returns and @throws - Assert findOneBy is called in the not-found and inactive findActiveOrFail tests - Drop controller tests for malformed IDs, which the DTO's class-validator decorators already cover - Replace the redundant "defaults to active" service test with one for reactivating an inactive recommendation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ations-endpoint # Conflicts: # apps/backend/src/app.module.ts # apps/backend/src/volunteers/volunteers.module.ts # apps/backend/src/volunteers/volunteers.service.spec.ts # apps/backend/src/volunteers/volunteers.service.ts
dburkhart07
left a comment
There was a problem hiding this comment.
all small things so ill approve! ty justin 🐯
| @@ -0,0 +1,11 @@ | |||
| import { IsInt, IsPositive } from 'class-validator'; | |||
There was a problem hiding this comment.
can we put this file in a dto directory within recommendations?
| summary: 'Recommend a Chameleon Animal to an active Volunteer', | ||
| }) | ||
| @ApiResponse({ | ||
| status: HttpStatus.CREATED, |
There was a problem hiding this comment.
good callouts! i think the ticket was a mistake, we should be returning a 201 here for a created, since thats what all POST endpoints should have.
| expect(recommendationsService.create).toHaveBeenCalledWith(body); | ||
| }); | ||
|
|
||
| it.each([ |
There was a problem hiding this comment.
i think we can just put this into one test and include both in the description (like 'does not create a recommendation when the volunteer does not exist or is not active'), since these are using the exact same mock
ℹ️ Issue
Closes MSPCA-11
📝 Description
Adds
POST /api/recommendations, which lets a foster coordinator recommend a Chameleon animal to a volunteer before a formal match exists. The body is{ volunteerId, chameleonAnimalId }, and the endpoint returns 200 with the saved recommendation (isActive: true). It returns 400 when either ID is missing or isn't a positive integer, and 404 when the volunteer doesn't exist.Changes:
CreateRecommendationDTO,RecommendationsService.create, and the controller route. The route uses@HttpCode(200)because the ticket asks for 200 and Nest's default for POST is 201.VolunteersService.existsByIdfor the 404 check.RecommendationsModulenow importsVolunteersModule, and it's registered inAppModule.createupserts on the(volunteer_id, chameleon_animal_id)primary key. Recommending the same animal twice reactivates the existing row instead of failing with a PK violation, even when two coordinators do it at the same time.validateIdnow usesNumber.isInteger, so1.5andundefinedare rejected.VolunteersModulenow importsCoordinatorsModule. Without it, registeringFosterVolunteerthroughautoLoadEntitiesmade the app fail on boot with "Entity metadata for FosterVolunteer#assignedCoordinator was not found".✔️ Verification
yarn test: all backend suites pass (20 suites, 160 tests). This covers new service and controller tests for success, the 404 case, each 400 case, and the 200 status code.yarn lint:check,yarn format:check, andtsc --noEmiton the backend are clean.isActive: true. A volunteer ID that doesn't exist should return 404, and a missing or non-integer ID should return 400.🏕️ (Optional) Future Work / Notes