[BUG] Insufficient allotment errors not being handled on purchases and donations. -- stage 1

[cielf]

Summary

Insufficient allotment errors are not being handled properly on purchases and donations, resulting in 500 errors

Why?

Our banks don't know what's going on when they get a 500 error, naturally. Confusion, frustration. Support tickets sometimes result, but sometimes they just give up.

Details

In the last month, in our bugsnag, we have seen unhandled InsufficientAllotment errors on purchases.update, purchases.destroy, donations.update. It's not a stretch to imagine the issue occurs on donations.destroy also

These errors need to be caught with an appropriate error displayed to the bank user.

The core issue is that code that calls StorageLocation#decrease_inventory (where this error ends up being raised) are not doing the validation that could let us feed a sensible error into the Rails-standard human-readable error flow.

In at least one case, the calling code simply swallows the raised error and returns false. In the case here with purchases and donations, the error isn't being caught.

There is a followon issue (# 3393 ) which is classified as technical debt. However, if you want to knock them both off, please do!

Criteria for completion for issue 1

Note: here we're saying that you might just have to swallow your pride but not the error. Rescue the error and return 4xx level error per Rails convention.

• [ ] Confirm that if a distribution update would cause the inventory to go below zero, the user is given a friendly error message, and the distribution is not updated.
• [ ] When a user tries to delete a purchase that would cause the inventory to go below zero, they are given a friendly error message and the purchase is not deleted
• [ ] When a user tries to update a purchase that would cause the inventory to go below zero, they are given a friendly error message and the purchase is not updated
• [ ] When a user tries to delete a donation that would cause the inventory to go below zero, they are given a friendly error message(and the purchase is not deleted
• [ ] When a user tries to update a donation that would cause the inventory to go below zero, they are given a friendly error message and the purchase is not updated
• [ ] Tests to support the above functionality

[walterbenson]

I can pick this up

[dorner]

You got it!

[github-actions[bot]]

This issue has been inactive for 248 hours (10.33 days) and will be automatically unassigned after 112 more hours (4.67 days).

[walterbenson]

Oops. Codes done just got bogged down in testing, I'll push on it soon and get it done before next week.

On Wed, Mar 15, 2023, 7:13 PM github-actions[bot] @.***> wrote:

This issue has been inactive for 248 hours (10.33 days) and will be automatically unassigned after 112 more hours (4.67 days).

— Reply to this email directly, view it on GitHub https://github.com/rubyforgood/human-essentials/issues/3392#issuecomment-1471014411, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANE2WU6YGTJMZG5HQFSBRCLW4JLKZANCNFSM6AAAAAAVBDJYU4 . You are receiving this because you were assigned.Message ID: @.***>

[cielf]

Hey @walterbenson -- How's this coming along?

[walterbenson]

Sorry @cielf i was not able to finish this. I had a bad case of covid and now I'm traveling for the next week. The code was pretty simple but the testing needs doing. If someone else picks this up that's great otherwise I'll get back to this when i return.

[cielf]

@walterbenson S'alright -- feel better!

[sophiabrent]

I can finish this one up!

[cielf]

Cool! Go for it, @sophiabrent

[github-actions[bot]]

This issue has been inactive for 241 hours (10.04 days) and will be automatically unassigned after 119 more hours (4.96 days).

[sophiabrent]

Sorry I got caught up with schoolwork, I'll be working on this and my other issue in the next week.

[cielf]

Alright -- let us know if you aren't going to be able to work on it -- it's giving a 500 error in production, so we'd really like to get it addressed.

[sophiabrent]

Quick clarification - what does it mean for a purchase/donation to cause inventory to go below zero? I'm just playing around on the browser and trying to duplicate the error, but I'm not sure I totally understand what the functionality is meant to do.

[sophiabrent]

Another follow up question: in my most recent commit to 3392, I just set it to flash an error if the action is unsuccessful, specifically for donations destroy and update. I try to check if these errors appear in the tests, but they are coming back nil. If someone could offer some guidance, I would appreciate it!

[cielf]

An example is that someone has entered a donation or purchase, and then has distributed some of the items, then they are coming back and making a correction to the donation or purchase. If they've already allocated the goods to be distributed, and, say, remove the item from the purchase or donation, that's a problem -- it may be a real problem that they've promised things they don't have!

[cielf]

So to manually recreate the problem, I would add a purchase, of say, 100 adult briefs, then do a distribution that would take the number of adult briefs below 100, then try to do a correction on the purchase, removing the 100 adult briefs. Does that help?

[sophiabrent]

Ah, I see - that was very helpful, thank you! @cielf

[github-actions[bot]]

This issue has been inactive for 244 hours (10.17 days) and will be automatically unassigned after 116 more hours (4.83 days).

[github-actions[bot]]

This issue has been inactive for 364 hours (15.17 days) and is past the limit of 360 hours (15.00 days) so is being unassigned.

[cielf]

Hey @sophiabrent -- Are you still working on this?

[cielf]

The insufficient allotment issue will be handled as part of the "event sourcing" work that @dorner is heading up. Leaving the issue here for now in case that goes dramatically bad, as it is not yet solved.

[cielf]

WE've had that work out with early adopters for about 6 weeks, with no complaints, and this was internally tested beforehand. I'm comfortable with closing this one.