SSF-225 FM Deleting & Editing Donation Item Backend#188
Conversation
There was a problem hiding this comment.
changes requested for codebase consistency!
let's have oz/value for donation items stay optional in the backend while the frontend requires them to match the create donation items & confirm item details contracts.
edit: i just saw your new pr up which updates those backend fields to be required & since it is better to match be + fe contracts and it's already implemented...let's keep it required in the be then, thanks for taking care of that :)
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
very very clean pr! few small things and one question worth discussing, but aside from that lgtm!
| @Min(1) | ||
| quantity!: number; | ||
|
|
||
| @IsNumber( |
There was a problem hiding this comment.
@Yurika-Kan can you explain the rationale behind making these required? the case im considering:
if i make a new donation with 2 items where i dont fill out the ozPerItem and estimatedValue, the next time i go to update the donation, i cant just make an edit to one of them, i have to update all fields for all values. i think they should be allowed to go in and just update the ozPerItem for one item if thats all they have.
lmk if im missing something though!
There was a problem hiding this comment.
To my understanding we cannot now make new donation items without ozPerItem and estimatedValue?
There was a problem hiding this comment.
They wanted to make all those fields now required upon creation so this was to keep consistency with that request
There was a problem hiding this comment.
ie oz, donation val, food rescue
There was a problem hiding this comment.
okay, then in that case, while it wasnt directly in the ticket, i think we should make an update to the donationItems entity, and write a new migration to fix it in the database and fix the frontend create donations so everything is consistent again
There was a problem hiding this comment.
I asked Yurika specifically about the entity/db and she said it wasn't necessary at the time @Yurika-Kan thoughts?
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
2 things, but imma approve since its pretty small.
| @Min(1) | ||
| quantity!: number; | ||
|
|
||
| @IsNumber( |
There was a problem hiding this comment.
okay, then in that case, while it wasnt directly in the ticket, i think we should make an update to the donationItems entity, and write a new migration to fix it in the database and fix the frontend create donations so everything is consistent again
Yurika-Kan
left a comment
Yurika-Kan
left a comment
There was a problem hiding this comment.
LGTMMM!! just address one smol request & itll be good to go~+!
|
|
||
| const donation = await donationTransactionRepo.findOne({ | ||
| where: { donationId }, | ||
| relations: ['donationItems'], |
There was a problem hiding this comment.
donation.donationItems seems to not be ever read here! if you want to drop this relation
3 participants
ℹ️ Issue
Closes https://vidushimisra.atlassian.net/browse/SSF-225
📝 Description
Added backend support for patch route donations/:donationId/item to allow adding, editing and deleting donation items for a donation.
✔️ Verification
Postman verified and service/controller tests
🏕️ (Optional) Future Work / Notes
N/A