Conversation
Juwang110
left a comment
There was a problem hiding this comment.
Honestly lgtm, one thing though. @sam-schu @Yurika-Kan I was going to comment to add test cases for these new bad request exceptions but it seems like approve/deny routes for both manufacturers and pantries don't have service tests. Should we add them here or is it fine?
|
@Juwang110 We don't have manufacturers service tests at all yet, and the pantries service tests are still mocking the repository. We don't need to worry about those tests here, we will add/update them in a future ticket! |
sam-schu
left a comment
There was a problem hiding this comment.
ConflictExceptionmight be better here since the client request is perfectly fine on its own, it's just that it conflicts with the current state of the pantry/manufacturer- Could you clarify why it was a bug not having the
foodManufacturerRepresentativerelation?
updated the exception! also thought it was a bug since the relation would be loaded as undefined, so when approve/deny methods try to access info about the foodManufacturer's user (foodManufacturerRepresentative) they would throw a 'cannot read properties of undefined error'. |
…-4-Community/ssf into sk/SSF-151-restrict-actions
sam-schu
left a comment
There was a problem hiding this comment.
Oh I see, thanks for catching that!
- Just a note: normally I would not suggest updating the
findOneservice function to fix that bug - that function is used by theGET /api/manufacturers/:foodManufacturerIdendpoint, so updating that function could cause the endpoint to return an extra relation that isn't needed. But in this case, that endpoint doesn't look to be called in the frontend, and when we do use that endpoint to view manufacturer applications later, we will need the representative info, so updating it is fine in this case! - It looks like the bug is still happening because the
approvefunction is still using the repo'sfindOnefunction, not the service's - can we change that?
ℹ️ Issue
Closes
📝 Description
added backend validation to throw a BadRequestException when a user tries to approve or deny a non pending pantry or manufacturer. also found a bug with foodManufacturer missing relations so fixed that too!
✔️ Verification
tried to approve/deny a non pending pantry/manufacturer