Yield a friendly error if user tries to submit a one time allocation that is higher than the allowable total giving amount - #128
Conversation
|
@freestylebit There is already a validator on this field, it just isn't being surfaced properly. See https://github.com/rubyforgood/community_foundation/blame/main/app/models/allocation/one_time.rb/#L24 It looks like your validation is more thorough, but I'd make sure it plays nicely with what is already there |
…that is higher than the allowable total giving amount
a03a5b4 to
e572df8
Compare
| class: "allocation-slider block mt-2" %> | ||
| </div> | ||
| <% else %> | ||
| <% one_time_max = allocation.one_time_amount_max(scenario) if klass == Allocation::OneTime %> |
There was a problem hiding this comment.
Just curious, is there a benefit to using klass == Allocation::OneTime over allocation.one_time?
There was a problem hiding this comment.
I think the intention here is to check the type of object to make sure this method exists. I think the agent was noticing that there are other calls like this in this template, though i'm sort of not sure if I agree with how we're guarding from method not found errors.
| # pre-existing over-allocated data stays editable. Used by the view helper so | ||
| # the slider cap and the server validator stay in sync. | ||
| def one_time_amount_max(scenario = self.scenario) | ||
| remaining = one_time_budget_remaining(scenario) |
There was a problem hiding this comment.
I like remaining, very readable :)
| # total giving budget. Returns nil when no budget is set (the server imposes | ||
| # no cap there either). Never drops below the allocation's own amount so | ||
| # pre-existing over-allocated data stays editable. Used by the view helper so | ||
| # the slider cap and the server validator stay in sync. |
There was a problem hiding this comment.
I think we are asserting this context in the tests, so we may be good to remove these comments.
There was a problem hiding this comment.
Something I noticed when working with AI agents is that they do pay attention to these breadcrumbs, so lately I've been kind of leaving them in there...provided that they update these when they change this logic. But I can remove this for now.
I sometimes try and submit a one time allocation for a scenario, and the form would close without doing anything; pretty sure nothing changed. I think the UI should be verbose enough to let the user know that they can't submit an amount higher than what is allocated to the scenario.
Screen.Recording.2026-08-28.at.5.32.54.PM.mov