-
Notifications
You must be signed in to change notification settings - Fork 0
[SSF-64] Volunteer Backend Updates #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
apps/backend/src/migrations/1760033134668-AddVolunteerPantryUniqueConstraint.ts
Show resolved
Hide resolved
| @PrimaryColumn({ name: 'pantry_id' }) | ||
| pantryId: number; | ||
|
|
||
| @ManyToOne(() => User, { nullable: false, onDelete: 'CASCADE' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for @sam-schu : I did not know about this onDelete: 'CASCADE' feature. For future work, should look into other entities to see where we may want to add this, as its quite useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me neither!
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few changes that I think would be helpful! Other than that lgtm!
apps/backend/src/volunteerAssignments/volunteerAssignments.entity.ts
Outdated
Show resolved
Hide resolved
b5d2a30 to
619fbf3
Compare
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few small changes
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few final changes!
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 small changes to make, but other than that lgtm!! 🍡 🔢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial thoughts:
- Could we simplify some of the users service logic by adding into the user entity a many-to-many relationship with pantries that would allow you to directly access all the pantries assigned to a volunteer through the user repo? Going through the user repo should be cleaner than having to manually access the junction table
- With any remaining logic that requires directly accessing the assignments or pantry entities (hopefully not that much), could we make service functions in the corresponding services for that logic so we only directly access the user repo in the users service?
apps/backend/src/migrations/1760033134668-AddVolunteerPantryUniqueConstraint.ts
Show resolved
Hide resolved
| @PrimaryColumn({ name: 'pantry_id' }) | ||
| pantryId: number; | ||
|
|
||
| @ManyToOne(() => User, { nullable: false, onDelete: 'CASCADE' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me neither!
49f97bb to
192c9a5
Compare
113d065 to
1205abc
Compare
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few small changes 🥜
b13da6b to
8600652
Compare
8600652 to
76c4a47
Compare
dburkhart07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!! 🎲
sam-schu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two nits, looks great! Awesome we're getting rid of a whole module :)
ℹ️ Issue
Closes SSF-64
📝 Description
GET /users/volunteersto return a volunteer's assigned pantry IDsGET /users/{id}/pantriesendpoint to get pantry info about volunteer's assigned pantriesPATCH /users/{id}/pantriesendpoint to assign list of pantries to a volunteer✔️ Verification
🏕️ (Optional) Future Work / Notes
Need to write tests.