#4076: created getGuestDefinition endpoint#4148
#4076: created getGuestDefinition endpoint#4148jasperpinkus wants to merge 11 commits intodevelopfrom
Conversation
staysgt
left a comment
There was a problem hiding this comment.
Looks really good so far, left some comments
|
|
||
| const def = await prisma.guest_Definition.findUnique({ where: { definitionId } }); | ||
| static async getGuestDefinition( | ||
| userCreatedId: string, |
There was a problem hiding this comment.
You will not need to take in the user created id, that does not matter when requesting a guest definition, so you can delete that parameter
| organizationId: string, | ||
| definitionId: string | ||
| ): Promise<Guest_Definition> { | ||
| const guest = await prisma.guest_Definition.findFirst({ |
There was a problem hiding this comment.
This can instead be findUnique, since there will only be one definition with the given definitionId
| if (!def) throw new NotFoundException('Guest Definition', definitionId); | ||
| if (def.dateDeleted) throw new DeletedException('Guest Definition', definitionId); | ||
| if (!guest) { | ||
| throw new NotFoundException('User', userCreatedId); |
There was a problem hiding this comment.
this should instead be NotFoundException('Guest Definition', definitionId)
| try { | ||
| const allDefinitons = await RecruitmentServices.getAllGuestDefinitions(req.organization); | ||
| res.status(200).json(allDefinitons); | ||
| const { userCreatedId, organizationId, definitionId } = req.params as { |
There was a problem hiding this comment.
We will only be extracting the definitionId from the parameters. You can get the organizationId directly from the request with req.organization.organizationId
staysgt
left a comment
There was a problem hiding this comment.
Two really small things! Looking really good
|
|
||
| static async getGuestDefinition(req: Request, res: Response, next: NextFunction) { | ||
| try { | ||
| console.log({ organizationId: req.organization.organizationId, definitionId: req.params.definitionId as string }); |
There was a problem hiding this comment.
Can you remove the console log
| RecruitmentController.createGuestDefinition | ||
| ); | ||
|
|
||
| recruitmentRouter.get('/guestDefinition/:definitionId', RecruitmentController.getGuestDefinition); |
There was a problem hiding this comment.
Make guestDefinition all lowercase
| * @returns a definition | ||
| * @throws if the definition is not found in the db | ||
| */ | ||
| static async getGuestDefinition(organizationId: string, definitionId: string) { |
There was a problem hiding this comment.
call this getSingleGuestDefinition so its use is clear
| try { | ||
| const definition = await RecruitmentServices.getGuestDefinition( | ||
| req.organization.organizationId, | ||
| req.params.definitionId as string |
There was a problem hiding this comment.
follow the conventions for accessing a req.params
look at getSingleEvent in calendar.controllers.ts
| static async getGuestDefinition(req: Request, res: Response, next: NextFunction) { | ||
| try { | ||
| const definition = await RecruitmentServices.getGuestDefinition( | ||
| req.organization.organizationId, |
There was a problem hiding this comment.
send the whole organization not just the id
Changes
Added the getGuestDefenition endpoint, adding functions to the service, controller, and route in the recruitment files.
Test Cases
Screenshots
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes # (issue #)
closes #4076