Skip to content

Yield a friendly error if user tries to submit a one time allocation that is higher than the allowable total giving amount - #128

Merged
freestylebit merged 3 commits into
mainfrom
pc-one-time
Aug 29, 2026
Merged

Yield a friendly error if user tries to submit a one time allocation that is higher than the allowable total giving amount#128
freestylebit merged 3 commits into
mainfrom
pc-one-time

Conversation

@freestylebit

Copy link
Copy Markdown
Collaborator

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

@freestylebit freestylebit added the rubyforgood Work done during the Ruby For Good 2026 Event 😎 label Aug 28, 2026
@jonny5

jonny5 commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

@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

Comment thread app/javascript/controllers/one_time_amount_controller.js Outdated
Comment thread app/models/allocation/one_time.rb Outdated
class: "allocation-slider block mt-2" %>
</div>
<% else %>
<% one_time_max = allocation.one_time_amount_max(scenario) if klass == Allocation::OneTime %>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, is there a benefit to using klass == Allocation::OneTime over allocation.one_time?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread app/models/allocation/one_time.rb Outdated
Comment thread app/models/allocation/one_time.rb Outdated
# 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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are asserting this context in the tests, so we may be good to remove these comments.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@dalmaboros dalmaboros left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😎

@freestylebit
freestylebit merged commit 5d1582f into main Aug 29, 2026
5 checks passed
@freestylebit
freestylebit deleted the pc-one-time branch August 29, 2026 20:08
jonny5 added a commit that referenced this pull request Aug 29, 2026
…ocation that is higher than the allowable total giving amount (#128)"

This reverts commit 5d1582f.
jonny5 added a commit that referenced this pull request Aug 30, 2026
…ocation that is higher than the allowable total giving amount (#128)"

This reverts commit 5d1582f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rubyforgood Work done during the Ruby For Good 2026 Event 😎

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants