Conversation
sam-schu
left a comment
There was a problem hiding this comment.
sorry for the delay!
The service function logic does not fully handle the third bullet in the ticket (the case where both upcoming reminders could be for the same donation). For example, testing with a once-weekly recurrence starting 4/15 and a once-monthly recurrence starting 5/11 returned those two dates, rather than 4/15 and 4/22. (It's only weekly donations that recur on multiple days of the week that will have multiple dates in their next_donation_dates array, so we need to handle more than just that case)
sam-schu
left a comment
There was a problem hiding this comment.
It looks like the logic updates only change how weekly donations are handled, but we need to handle all recurrence types - e.g., a monthly donation could recur twice before a yearly donation, or a yearly donation could recur twice before an every-3-years donation
swarkewalia
left a comment
There was a problem hiding this comment.
great work amy! just one bug i noticed - while all the service tests pass, the service logic produces duplicate reminders sometimes. i tested this by adding these console logs and noticed this result (in manufacturers.service.ts) and the output from it:
The last two dates are duplicated, and this happens because dates from nextDonationDates are mapped in the first pass and then calculateNextDonationDate regenerates some of those same dates in the second pass. the final output happens to be correct because slice(0, 2) cuts the array before any duplicate reaches the first two positions, but that's not guaranteed
|
missed some final comments after approval of #149 that i added to this pr @sam-schu @swarkewalia |
sam-schu
left a comment
There was a problem hiding this comment.
sorry the logic for this has ended up being so complex 😭
ℹ️ Issue
Closes SSF-183
📝 Description
/manufacturers/:foodManufacturerId/next-two-donationsgetUpcomingDonationReminders✔️ Verification
tested endpoint on postman and verified tests pass
🏕️ (Optional) Future Work / Notes
not sure if it's just me but
/manufacturers/:foodManufacturerId/donationsis causing an error/not working...