MSPCA-7: Add activate/deactivate endpoints for volunteers and coordin… - #6
pujitakalinadhabhotla wants to merge 1 commit into
Conversation
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
few small things for each. very clean pr tho, ty pujita!!! 🦁
| // return this.usersService.findOne(userId); | ||
| // } | ||
|
|
||
| @Patch('/:id/deactivate') |
There was a problem hiding this comment.
I should've specified this in the ticket, but we are going to be documenting all of our endpoints so that they can be easily read within Swagger without having to go into our repo. To do this, we are gonna add Tags from NestJs. For all your endpoints, can you add an @apioperation, @ApiParam, and just one @apiresponse for the happy path, including the status code, description, and type it returns? We will do this for all API endpoints moving forward.
| // } | ||
|
|
||
| async deactivate(id: number): Promise<FosterVolunteer> { | ||
| const volunteer = await this.repo.findOne({ where: { volunteerId: id } }); |
There was a problem hiding this comment.
good call to put this here. i think what we will do is, in cases where we are going to fetch the volunteer in the service function anyways, we don't need an existence check in the controller. in cases we aren't we will want one
| ); | ||
| expect(repo.save).not.toHaveBeenCalled(); | ||
| }); | ||
| it('should not delete the volunteer record', async () => { |
There was a problem hiding this comment.
i think we can just add in line 65 into the first test
|
|
||
| describe('deactivate', () => { | ||
| it('should call service.deactivate with the parsed id', async () => { | ||
| const volunteer = { volunteerId: 1, active: false }; |
There was a problem hiding this comment.
can we type cast this to a FosterVolunteer
|
|
||
| describe('activate', () => { | ||
| it('should call service.activate with the parsed id', async () => { | ||
| const volunteer = { volunteerId: 1, active: true }; |
|
|
||
| describe('deactivate', () => { | ||
| it('should call service.deactivate with the parsed id', async () => { | ||
| const coordinator = { coordinatorId: 1, active: false }; |
|
|
||
| describe('activate', () => { | ||
| it('should call service.activate with the parsed id', async () => { | ||
| const coordinator = { coordinatorId: 1, active: true }; |
| ); | ||
| expect(repo.save).not.toHaveBeenCalled(); | ||
| }); | ||
| it('should not delete the coordinator record', async () => { |
There was a problem hiding this comment.
same thing here as the volunteer service
2 participants
ℹ️ Issue
Closes MSPCA-7
📝 Description
Adds PATCH /volunteers/:id/deactivate, PATCH /volunteers/:id/activate, PATCH /coordinators/:id/deactivate, and PATCH /coordinators/:id/activate, letting coordinators toggle a volunteer's or coordinator's active status by ID without deleting any of their data. An unknown ID returns a 404, and a non-numeric or non-positive ID returns a 400.
✔️ Verification
yarn test volunteers coordinators: all new and existing volunteer/coordinator suites pass.