Conversation
d21970f to
8fd4520
Compare
|
At first glance I'm seeing a decent number of things that don't fully match the design. Could you please take another look at the design and try to match things like spacing, alignment, colors, etc. as closely as possible? |
maxn990
left a comment
There was a problem hiding this comment.
couldn't find a great way given chakra constraints to match the right edges of the refrigeration status badges, if anyone has any ideas lmk!
also is this still blocked?
amywng
left a comment
There was a problem hiding this comment.
for your comment, do you mean the right align of the refrigerated badges? because i think you can just do text-align="right" for the column title and rows.
a couple other things:
- can we ask priya about what it should look like if the volunteer has no assigned pantries and add that?
- can we add a full
isLoadingstate with a spinner? i feel like the individual "Loading..." everywhere doesn't make sense - right now, you could potentially make a ton of api calls depending on how many pantries a volunteer has assigned to them. @sam-schu @Yurika-Kan i would suggest just returning all the pantry info in the
getAllVolunteerscall since we don't use the call anywhere else and i feel like this method is less efficient
sam-schu
left a comment
There was a problem hiding this comment.
Based on the ticket, this page should display a particular volunteer's assigned pantries, rather than all the pantries assigned to all volunteers. We have an endpoint that was previously created for this purpose, GET /api/volunteers/:id/pantries; this endpoint returns the full pantry details for each assigned pantry so we only need to make this single API call, resolving @amywng's suggestion. To determine which volunteer ID to use in the API call based on which volunteer is currently signed in, we may want to add a /my-id endpoint to the users controller to get the ID of the currently authenticated user (similar to the /my-id endpoint in the pantries controller but even simpler, as that one needs to translate from user ID to pantry ID, whereas a volunteer ID is just the volunteer user's user ID).
amywng
left a comment
There was a problem hiding this comment.
while you're at it can you rename user.entity.ts to users.entity.ts
| export class UsersController { | ||
| constructor(private usersService: UsersService) {} | ||
|
|
||
| @Get('/my-id') |
There was a problem hiding this comment.
i think we should set auth in some way here. @sam-schu should it be public and then we check if req.user.id is null or not and then throw an exception if it is? or should it be restricted to volunteers? or something else
There was a problem hiding this comment.
I'll do Volunteers for now to match pantries
There was a problem hiding this comment.
We shouldn't mark it as public because the user does have to be signed in, but we don't need to limit it to any specific role
amywng
left a comment
There was a problem hiding this comment.
small nits but approving! 🙏 ty max
ℹ️ Issue
Closes 62
📝 Description
Added page to view all the pantries they were assigned to, as well as their information from the pantry application.
Path /volunteer-assigned-pantries
Also tested adding filtering by being Refrigerator Friendly or not.