[SSF 227] - Volunteer archive and edit orders backend#193
Conversation
Yurika-Kan
left a comment
Yurika-Kan
left a comment
There was a problem hiding this comment.
looking pretty good~ crucial questions about donation status setting tho!
| donationItemId?: number; | ||
|
|
||
| @IsInt() | ||
| @Min(1) |
There was a problem hiding this comment.
in the case that we want to omit an item from being part of the update, we would want this value to be 0~ this contradicts the service logic that treats 0 as a legal drop
There was a problem hiding this comment.
the way i intend for this to be used in the frontend is this:
- passing in an allocationId means you want to edit an existing allocations quantity to a new number (just decrease it, not delete it)
- passing in a donationItemId means you want to add a new allocation to the order
both cases above require the allocated quantity to be at minimum 1.
all allocations not included in here (which will happen if they are set to 0), will then be free'd.
| ); | ||
| } | ||
|
|
||
| @Patch('/update-status/:orderId') |
There was a problem hiding this comment.
note: deprecate endpoint!
| }; | ||
| } | ||
|
|
||
| async updateStatus(orderId: number, newStatus: OrderStatus) { |
There was a problem hiding this comment.
note: deprecate service func of /update-status!
| expect(orderBefore.status).toBe(OrderStatus.PENDING); | ||
| expect(cerealBefore.reservedQuantity).toBe(75); | ||
| expect(almondBefore.reservedQuantity).toBe(20); | ||
| expect(cerealDonationBefore.status).toBe(DonationStatus.FULFILLED); |
There was a problem hiding this comment.
highlighting that this dummy data Donation #4 (FoodCorp, total_items=75): status = 'fulfilled' with a pending order seems to be a faulty/dirty test data since a donation with a pending order should be matched and never fulfilled.
not huge deal, a fix (if we really want to be clean) is to localize the status by setting the donation to be matched before the test
Yurika-Kan
left a comment
Yurika-Kan
left a comment
There was a problem hiding this comment.
taking care of new order status~
side effects of adding new enum CLOSED:
- in [request.service.ts:309-315], this method updateRequestStatus decides request status (done when all associated orders = DELIVERED). however now with CLOSED (order) introduced, there can be a case where all orders are DELIVERED but one is CLOSED, resulting in that associated request never reaching closed status. we want to close a food request if all of its associated orders are either DELIVERED or CLOSED. can we update that logic here?
`
const allDelivered = orders.every(
(order) => order.status === OrderStatus.DELIVERED,
);
request.status = allDelivered
? FoodRequestStatus.CLOSED
: FoodRequestStatus.ACTIVE;`
2 participants
ℹ️ Issue
Closes #227
📝 Description
NOTE: For editing allocations, the way I designed it was that the dto would take in an allocationId or donationItemId. Should the allocationId be provided, it would be an edited allocation that already exists in the order. Should it be a donationItemId, it is a new allocation to be created for that order. Finally, all existing allocations that are do not get their ids provided will be assumed to be deleted. I made sure to check that all the donationItemIds involved in the editing are part of the donations from the food manufacturers, and all donations affected as assessed to see if they'ev either beomce matched or available as a result to gaining or losing allocations.
✔️ Verification
🏕️ (Optional) Future Work / Notes
We will have to account for this setup in our frontend implementation so that no allocations with a 0 quantity for the order get sent to the backend.