From 2af9d9736955c3e43dc46f932255355ef6c5ae92 Mon Sep 17 00:00:00 2001 From: jonkaplan Date: Sat, 29 Aug 2026 17:04:40 -0400 Subject: [PATCH 1/4] Revert "Yield a friendly error if user tries to submit a one time allocation that is higher than the allowable total giving amount (#128)" This reverts commit 5d1582f3621829d00521f989f2c6a00f13ef5691. --- Gemfile.lock | 1 - .../controllers/one_time_amount_controller.js | 33 ------------------- app/models/allocation/one_time.rb | 26 ++------------- .../scenarios/_allocation_modal.html.erb | 5 ++- .../allocations_controller_test.rb | 8 ----- test/models/allocation/one_time_test.rb | 30 ----------------- 6 files changed, 4 insertions(+), 99 deletions(-) delete mode 100644 app/javascript/controllers/one_time_amount_controller.js diff --git a/Gemfile.lock b/Gemfile.lock index ecea679..92fe435 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -469,7 +469,6 @@ PLATFORMS arm-linux-musl arm64-darwin-23 arm64-darwin-24 - arm64-darwin-25 x86_64-linux x86_64-linux-gnu x86_64-linux-musl diff --git a/app/javascript/controllers/one_time_amount_controller.js b/app/javascript/controllers/one_time_amount_controller.js deleted file mode 100644 index a635c20..0000000 --- a/app/javascript/controllers/one_time_amount_controller.js +++ /dev/null @@ -1,33 +0,0 @@ -import { Controller } from "@hotwired/stimulus" - -const currency = new Intl.NumberFormat("en-US", { - style: "currency", - currency: "USD", - maximumFractionDigits: 0, -}) - -// Keeps a one-time giving amount within the scenario's remaining giving budget. -// Sets a custom-validity message so the browser blocks submission (via native -// constraint validation, regardless of number-input support) and shows a -// friendly error without closing the modal. -export default class extends Controller { - static values = { max: Number } - - connect() { - this.element.addEventListener("input", this.validate) - this.validate() - } - - disconnect() { - this.element.removeEventListener("input", this.validate) - } - - validate = () => { - const overMaxValue = this.element.value !== "" && Number(this.element.value) > this.maxValue - this.element.setCustomValidity(overMaxValue ? this.message : "") - } - - get message() { - return `Enter an amount of ${currency.format(this.maxValue)} or less — that's your remaining one-time giving budget.` - } -} \ No newline at end of file diff --git a/app/models/allocation/one_time.rb b/app/models/allocation/one_time.rb index d81eee1..2df868a 100644 --- a/app/models/allocation/one_time.rb +++ b/app/models/allocation/one_time.rb @@ -19,36 +19,14 @@ def share_percentage (amount.to_i / total.to_f * 100).round end - # The most this allocation can be set to while staying within the scenario's - # 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. - def max_amount(scenario = self.scenario) - remaining = budget_remaining(scenario) - return if remaining.nil? - - [ remaining, amount.to_i ].max - end - private def within_total_giving_amount return if amount.blank? || scenario&.total_giving_amount.blank? - remaining = budget_remaining(scenario) - return if remaining.nil? - - if amount > remaining - others = scenario.total_giving_amount - remaining + others = scenario.one_time_allocations.where.not(id: id).sum(:amount) + if others + amount > scenario.total_giving_amount errors.add(:amount, "would bring one-time giving to #{others + amount}, over the total giving amount of #{scenario.total_giving_amount.to_i}") end end - - def budget_remaining(scenario = self.scenario) - return nil if scenario.blank? || scenario.total_giving_amount.blank? - - others = scenario.one_time_allocations.where.not(id: id).sum(:amount) - scenario.total_giving_amount - others - end end diff --git a/app/views/scenarios/_allocation_modal.html.erb b/app/views/scenarios/_allocation_modal.html.erb index 2d6aca2..6cbda23 100644 --- a/app/views/scenarios/_allocation_modal.html.erb +++ b/app/views/scenarios/_allocation_modal.html.erb @@ -28,14 +28,13 @@ class: [ "allocation-slider block mt-2", ("allocation-slider--capped" if remaining_percentage < 100) ] %> <% else %> - <% max_amount = allocation.max_amount(scenario) if klass == Allocation::OneTime %>
<%= label_tag "allocation_amount_#{suffix}", "Amount", class: "block text-sm text-ink-soft" %>
$ - <%= number_field_tag "allocation[amount]", allocation.amount, id: "allocation_amount_#{suffix}", min: 0, max: max_amount, step: "1", required: true, placeholder: "0", + <%= number_field_tag "allocation[amount]", allocation.amount, id: "allocation_amount_#{suffix}", min: 0, step: "1", required: true, placeholder: "0", inputmode: "numeric", - data: max_amount.present? ? { controller: "integer-input one-time-amount", action: "input->integer-input#transform", one_time_amount_max_value: max_amount } : { controller: "integer-input", action: "input->integer-input#transform" }, + data: { controller: "integer-input", action: "input->integer-input#transform" }, class: "block w-full rounded-md border border-line bg-surface pl-7 pr-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10" %>
diff --git a/test/controllers/allocations_controller_test.rb b/test/controllers/allocations_controller_test.rb index 9ebd078..7900eae 100644 --- a/test/controllers/allocations_controller_test.rb +++ b/test/controllers/allocations_controller_test.rb @@ -56,14 +56,6 @@ class AllocationsControllerTest < ActionDispatch::IntegrationTest assert_match Allocation::GreatestCommunityNeed::DESCRIPTION, response.body end - test "caps the one-time amount field at the remaining giving budget" do - # scenario total is 10000 and education_grant fixture already allocates 5000. - get scenario_url(@scenario) - assert_response :success - assert_match %r{id="allocation_amount_one_time"[^>]*max="5000"}, response.body - assert_match %r{data-one-time-amount-max-value="5000"}, response.body - end - test "rejects a duplicate Greatest Community Need allocation" do # one_arlington already has the greatest_need fixture allocation. assert_no_difference -> { @scenario.allocations.count } do diff --git a/test/models/allocation/one_time_test.rb b/test/models/allocation/one_time_test.rb index 89f9212..8b6e232 100644 --- a/test/models/allocation/one_time_test.rb +++ b/test/models/allocation/one_time_test.rb @@ -9,34 +9,4 @@ class Allocation::OneTimeTest < ActiveSupport::TestCase allocation = Allocation::OneTime.new(amount: 500, scenario: scenarios(:two_boston)) assert_equal 0, allocation.share_percentage end - - test "max_amount is the remaining budget for a new allocation" do - scenario = scenarios(:one_arlington) - allocation = Allocation::OneTime.new - - assert_equal 5000, allocation.max_amount(scenario) - end - - test "max_amount excludes the allocation's own amount when editing" do - scenario = scenarios(:one_arlington) - allocation = allocations(:education_grant) - - assert_equal 10000, allocation.max_amount(scenario) - end - - test "max_amount never drops below the allocation's own amount" do - scenario = scenarios(:one_arlington) - scenario.update!(total_giving_amount: 4000) - allocation = allocations(:education_grant) - - assert_equal 5000, allocation.max_amount(scenario) - end - - test "max_amount is nil when no total giving amount is set" do - scenario = scenarios(:two_boston) - scenario.update!(total_giving_amount: nil) - allocation = Allocation::OneTime.new - - assert_nil allocation.max_amount(scenario) - end end From 6ccca6d90f25a2bfa934b6e5ccd38000b37d2044 Mon Sep 17 00:00:00 2001 From: jonkaplan Date: Sun, 30 Aug 2026 09:34:57 -0400 Subject: [PATCH 2/4] Move allocation form to partial --- app/views/scenarios/_allocation_form.html.erb | 57 +++++++++++++++++++ .../scenarios/_allocation_modal.html.erb | 57 +------------------ 2 files changed, 58 insertions(+), 56 deletions(-) create mode 100644 app/views/scenarios/_allocation_form.html.erb diff --git a/app/views/scenarios/_allocation_form.html.erb b/app/views/scenarios/_allocation_form.html.erb new file mode 100644 index 0000000..88ba170 --- /dev/null +++ b/app/views/scenarios/_allocation_form.html.erb @@ -0,0 +1,57 @@ +<%# locals: (allocation:, scenario:, color: nil) %> +<% klass = allocation.class %> +<% editing = allocation.persisted? %> +<% suffix = editing ? dom_id(allocation) : klass.model_name.element %> +<% remaining_percentage = klass == Allocation::Ongoing ? remaining_ongoing_percentage(scenario, allocation) : nil %> +<% percentage = allocation.percentage || [ 20, remaining_percentage.to_i ].min %> +<% color ||= ScenariosHelper::CHART_COLORS.first %> +<%= form_with url: (editing ? scenario_allocation_path(scenario, allocation) : scenario_allocations_path(scenario)), method: (editing ? :patch : :post), class: "p-8" do |form| %> + <%= hidden_field_tag "allocation[type]", klass.name %> +

<%= editing ? "Edit allocation" : "Create allocation" %>

+ + <%= render "scenarios/category_picker", allocation: allocation, scenario: scenario %> + + <% if klass == Allocation::Ongoing %> +
+
+ Giving percentage % + <%= remaining_percentage %>% remaining +
+

<%= percentage %>%

+ <%# max stays at 100 (see allocation_slider_controller.js); the headroom the + slider actually stops at is announced through the description instead. %> + <%= range_field_tag "allocation[percentage]", percentage, min: 0, max: 100, step: 1, + style: "--slider-color: #{color}; --slider-value: #{percentage.to_i}; --slider-limit: #{remaining_percentage}", + aria: { describedby: "allocation_headroom_#{suffix}" }, + data: { allocation_slider_target: "input", action: "input->allocation-slider#update" }, + class: [ "allocation-slider block mt-2", ("allocation-slider--capped" if remaining_percentage < 100) ] %> +
+ <% else %> +
+ <%= label_tag "allocation_amount_#{suffix}", "Amount", class: "block text-sm text-ink-soft" %> +
+ $ + <%= number_field_tag "allocation[amount]", allocation.amount, id: "allocation_amount_#{suffix}", min: 0, step: "1", required: true, placeholder: "0", + inputmode: "numeric", + data: { controller: "integer-input", action: "input->integer-input#transform" }, + class: "block w-full rounded-md border border-line bg-surface pl-7 pr-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10" %> +
+
+ <% end %> + + <%= render "scenarios/additional_preferences", allocation: allocation, scenario: scenario, suffix: suffix %> + +
+ <%= label_tag "allocation_note_#{suffix}", "Note (optional)", class: "block text-sm text-ink-soft" %> + <%= text_area_tag "allocation[note]", allocation.note, id: "allocation_note_#{suffix}", rows: 3, placeholder: "Extra preferences, restrictions, e.g. need-based scholarships only, exclude specific orgs…", + class: "mt-1 block w-full rounded-md border border-line bg-surface px-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10" %> +
+ +
+ + <%= form.submit (editing ? "Save" : "Create"), class: "rounded-md px-5 py-2.5 bg-accent hover:bg-[#444] text-white font-medium cursor-pointer transition" %> +
+<% end %> diff --git a/app/views/scenarios/_allocation_modal.html.erb b/app/views/scenarios/_allocation_modal.html.erb index 6cbda23..3a04b09 100644 --- a/app/views/scenarios/_allocation_modal.html.erb +++ b/app/views/scenarios/_allocation_modal.html.erb @@ -1,59 +1,4 @@ <%# locals: (allocation:, scenario:, color: nil) %> -<% klass = allocation.class %> -<% editing = allocation.persisted? %> -<% suffix = editing ? dom_id(allocation) : klass.model_name.element %> -<% remaining_percentage = klass == Allocation::Ongoing ? remaining_ongoing_percentage(scenario, allocation) : nil %> -<% percentage = allocation.percentage || [ 20, remaining_percentage.to_i ].min %> -<% color ||= ScenariosHelper::CHART_COLORS.first %> - <%= form_with url: (editing ? scenario_allocation_path(scenario, allocation) : scenario_allocations_path(scenario)), method: (editing ? :patch : :post), class: "p-8" do |form| %> - <%= hidden_field_tag "allocation[type]", klass.name %> -

<%= editing ? "Edit allocation" : "Create allocation" %>

- - <%= render "scenarios/category_picker", allocation: allocation, scenario: scenario %> - - <% if klass == Allocation::Ongoing %> -
-
- Giving percentage % - <%= remaining_percentage %>% remaining -
-

<%= percentage %>%

- <%# max stays at 100 (see allocation_slider_controller.js); the headroom the - slider actually stops at is announced through the description instead. %> - <%= range_field_tag "allocation[percentage]", percentage, min: 0, max: 100, step: 1, - style: "--slider-color: #{color}; --slider-value: #{percentage.to_i}; --slider-limit: #{remaining_percentage}", - aria: { describedby: "allocation_headroom_#{suffix}" }, - data: { allocation_slider_target: "input", action: "input->allocation-slider#update" }, - class: [ "allocation-slider block mt-2", ("allocation-slider--capped" if remaining_percentage < 100) ] %> -
- <% else %> -
- <%= label_tag "allocation_amount_#{suffix}", "Amount", class: "block text-sm text-ink-soft" %> -
- $ - <%= number_field_tag "allocation[amount]", allocation.amount, id: "allocation_amount_#{suffix}", min: 0, step: "1", required: true, placeholder: "0", - inputmode: "numeric", - data: { controller: "integer-input", action: "input->integer-input#transform" }, - class: "block w-full rounded-md border border-line bg-surface pl-7 pr-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10" %> -
-
- <% end %> - - <%= render "scenarios/additional_preferences", allocation: allocation, scenario: scenario, suffix: suffix %> - -
- <%= label_tag "allocation_note_#{suffix}", "Note (optional)", class: "block text-sm text-ink-soft" %> - <%= text_area_tag "allocation[note]", allocation.note, id: "allocation_note_#{suffix}", rows: 3, placeholder: "Extra preferences, restrictions, e.g. need-based scholarships only, exclude specific orgs…", - class: "mt-1 block w-full rounded-md border border-line bg-surface px-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10" %> -
- -
- - <%= form.submit (editing ? "Save" : "Create"), class: "rounded-md px-5 py-2.5 bg-accent hover:bg-[#444] text-white font-medium cursor-pointer transition" %> -
- <% end %> + <%= render "scenarios/allocation_form", allocation: allocation, scenario: scenario, color: color %> From e92f7344c5cdead85199a05b112ce331c116cdcd Mon Sep 17 00:00:00 2001 From: jonkaplan Date: Sun, 30 Aug 2026 09:34:57 -0400 Subject: [PATCH 3/4] Render allocation validation errors under their fields --- app/views/scenarios/_allocation_form.html.erb | 8 ++++++-- app/views/scenarios/_field_errors.html.erb | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 app/views/scenarios/_field_errors.html.erb diff --git a/app/views/scenarios/_allocation_form.html.erb b/app/views/scenarios/_allocation_form.html.erb index 88ba170..a989950 100644 --- a/app/views/scenarios/_allocation_form.html.erb +++ b/app/views/scenarios/_allocation_form.html.erb @@ -5,8 +5,10 @@ <% remaining_percentage = klass == Allocation::Ongoing ? remaining_ongoing_percentage(scenario, allocation) : nil %> <% percentage = allocation.percentage || [ 20, remaining_percentage.to_i ].min %> <% color ||= ScenariosHelper::CHART_COLORS.first %> -<%= form_with url: (editing ? scenario_allocation_path(scenario, allocation) : scenario_allocations_path(scenario)), method: (editing ? :patch : :post), class: "p-8" do |form| %> +<%= form_with url: (editing ? scenario_allocation_path(scenario, allocation) : scenario_allocations_path(scenario)), method: (editing ? :patch : :post), id: dom_id(allocation, :form), class: "p-8" do |form| %> <%= hidden_field_tag "allocation[type]", klass.name %> + <%# Carried through so a re-render after a validation error keeps the slider's chart color. %> + <%= hidden_field_tag "allocation_color", color %>

<%= editing ? "Edit allocation" : "Create allocation" %>

<%= render "scenarios/category_picker", allocation: allocation, scenario: scenario %> @@ -25,6 +27,7 @@ aria: { describedby: "allocation_headroom_#{suffix}" }, data: { allocation_slider_target: "input", action: "input->allocation-slider#update" }, class: [ "allocation-slider block mt-2", ("allocation-slider--capped" if remaining_percentage < 100) ] %> + <%= render "scenarios/field_errors", messages: allocation.errors[:percentage] %> <% else %>
@@ -34,8 +37,9 @@ <%= number_field_tag "allocation[amount]", allocation.amount, id: "allocation_amount_#{suffix}", min: 0, step: "1", required: true, placeholder: "0", inputmode: "numeric", data: { controller: "integer-input", action: "input->integer-input#transform" }, - class: "block w-full rounded-md border border-line bg-surface pl-7 pr-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10" %> + class: "block w-full rounded-md border bg-surface pl-7 pr-3 py-2 shadow-sm focus:border-accent focus:outline-none focus:ring-2 focus:ring-accent/10 #{allocation.errors[:amount].any? ? "border-danger" : "border-line"}" %>
+ <%= render "scenarios/field_errors", messages: allocation.errors[:amount] %> <% end %> diff --git a/app/views/scenarios/_field_errors.html.erb b/app/views/scenarios/_field_errors.html.erb new file mode 100644 index 0000000..0bfb212 --- /dev/null +++ b/app/views/scenarios/_field_errors.html.erb @@ -0,0 +1,4 @@ +<%# locals: (messages:) %> +<% if messages.any? %> +

<%= messages.to_sentence %>

+<% end %> From 2cacc53d8d5e01805c194cdec1df62d40e68e7d9 Mon Sep 17 00:00:00 2001 From: jonkaplan Date: Sun, 30 Aug 2026 09:34:57 -0400 Subject: [PATCH 4/4] Show allocation errors in the form instead of a flash The form lives in a top-layer , so a flash rendered behind the modal and the redirect dropped the user's input. --- app/controllers/allocations_controller.rb | 26 ++++++---- .../allocation_category_picker_controller.js | 37 +++++++++++++- app/models/allocation/one_time.rb | 22 +++++++-- app/models/allocation/ongoing.rb | 18 +++++-- app/views/scenarios/_category_picker.html.erb | 12 ++++- .../allocations_controller_test.rb | 48 ++++++++++++++----- test/models/allocation_test.rb | 31 ++++++++++++ 7 files changed, 161 insertions(+), 33 deletions(-) diff --git a/app/controllers/allocations_controller.rb b/app/controllers/allocations_controller.rb index 450f1af..aadbf6f 100644 --- a/app/controllers/allocations_controller.rb +++ b/app/controllers/allocations_controller.rb @@ -8,7 +8,7 @@ def create if allocation.persisted? redirect_to scenario_path(@scenario) else - redirect_to scenario_path(@scenario), alert: allocation.errors.full_messages.to_sentence + render_errors allocation end end @@ -17,22 +17,32 @@ def update if allocation.update(allocation_params) redirect_to scenario_path(@scenario) else - redirect_to scenario_path(@scenario), alert: allocation.errors.full_messages.to_sentence + render_errors allocation end end def destroy allocation = @scenario.allocations.find(params[:id]) - if allocation.greatest_community_need? - redirect_to scenario_path(@scenario), alert: "Greatest Community Need can't be removed." - else - allocation.destroy - redirect_to scenario_path(@scenario) - end + # No delete button is rendered for Greatest Community Need; this guards the route. + return head :forbidden if allocation.greatest_community_need? + + allocation.destroy + redirect_to scenario_path(@scenario) end private + # Errors go back into the form itself: each one comes back attached to the field + # that produced it. The form only exists inside a top-layer , so there is + # no other surface a message could render on. + def render_errors(allocation) + render turbo_stream: turbo_stream.replace( + helpers.dom_id(allocation, :form), + partial: "scenarios/allocation_form", + locals: { allocation: allocation, scenario: @scenario, color: params[:allocation_color].presence } + ), status: :unprocessable_entity + end + def set_scenario @scenario = accessible_scenarios.find(params[:scenario_id]) end diff --git a/app/javascript/controllers/allocation_category_picker_controller.js b/app/javascript/controllers/allocation_category_picker_controller.js index 4f53938..0e2c9f0 100644 --- a/app/javascript/controllers/allocation_category_picker_controller.js +++ b/app/javascript/controllers/allocation_category_picker_controller.js @@ -1,16 +1,22 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = ["panel", "search", "row", "tab", "tabPanel", "categoryId", "optionField", "label", "customInput"] - static classes = ["activeTab", "inactiveTab"] + static targets = ["panel", "search", "row", "tab", "tabPanel", "categoryId", "optionField", "label", "customInput", "trigger", "error"] + static classes = ["activeTab", "inactiveTab", "invalid", "valid"] connect() { this.activateInitialTab() document.addEventListener("click", this.onOutsideClick) + this.form = this.element.closest("form") + this.form?.addEventListener("submit", this.onSubmit) + this.dialog = this.element.closest("dialog") + this.dialog?.addEventListener("close", this.onDialogClose) } disconnect() { document.removeEventListener("click", this.onOutsideClick) + this.form?.removeEventListener("submit", this.onSubmit) + this.dialog?.removeEventListener("close", this.onDialogClose) } toggle() { @@ -27,6 +33,7 @@ export default class extends Controller { this.optionFieldTarget.value = "" if (this.hasCustomInputTarget) this.customInputTarget.value = "" this.labelTarget.textContent = row.dataset.name + this.clearError() this.close() } @@ -47,6 +54,7 @@ export default class extends Controller { this.optionFieldTarget.value = value this.categoryIdTarget.value = "" this.labelTarget.textContent = value + this.clearError() this.close() } @@ -66,6 +74,31 @@ export default class extends Controller { }) } + // Mirrors Allocation#category_or_option_present so an incomplete form stays + // open with the error attached to the field instead of closing on submit. + onSubmit = (event) => { + if (this.categoryIdTarget.value || this.optionFieldTarget.value) return this.clearError() + + event.preventDefault() + this.errorTarget.hidden = false + this.triggerTarget.classList.remove(...this.validClasses) + this.triggerTarget.classList.add(...this.invalidClasses) + this.triggerTarget.focus() + } + + // Cancel, Esc, and backdrop clicks all fire the dialog's close event: reopening + // should start clean rather than showing the previous attempt's error. + onDialogClose = () => { + this.clearError() + this.close() + } + + clearError() { + this.errorTarget.hidden = true + this.triggerTarget.classList.remove(...this.invalidClasses) + this.triggerTarget.classList.add(...this.validClasses) + } + onOutsideClick = (event) => { if (!this.element.contains(event.target)) this.close() } diff --git a/app/models/allocation/one_time.rb b/app/models/allocation/one_time.rb index 2df868a..eece9f9 100644 --- a/app/models/allocation/one_time.rb +++ b/app/models/allocation/one_time.rb @@ -1,7 +1,12 @@ class Allocation::OneTime < Allocation + # allow_nil hands the blank case to the presence validator alone, so a blank + # field yields one message instead of two. validates :amount, - presence: true, - numericality: { only_integer: true, greater_than: 0 } + presence: { message: "Enter an amount." }, + numericality: { + only_integer: true, greater_than: 0, allow_nil: true, + message: "Enter a whole dollar amount greater than $0." + } validate :within_total_giving_amount def ongoing? @@ -23,10 +28,17 @@ def share_percentage def within_total_giving_amount return if amount.blank? || scenario&.total_giving_amount.blank? + return if errors[:amount].any? + total = scenario.total_giving_amount.to_i others = scenario.one_time_allocations.where.not(id: id).sum(:amount) - if others + amount > scenario.total_giving_amount - errors.add(:amount, "would bring one-time giving to #{others + amount}, over the total giving amount of #{scenario.total_giving_amount.to_i}") - end + return if others + amount <= total + + remaining = [ total - others, 0 ].max + errors.add(:amount, "You have #{money(remaining)} left to allocate.") + end + + def money(value) + ActiveSupport::NumberHelper.number_to_currency(value, precision: 0) end end diff --git a/app/models/allocation/ongoing.rb b/app/models/allocation/ongoing.rb index 35335bd..d629e85 100644 --- a/app/models/allocation/ongoing.rb +++ b/app/models/allocation/ongoing.rb @@ -1,9 +1,15 @@ class Allocation::Ongoing < Allocation PERPETUITY_PAYOUT_RATE = 0.05 + # allow_nil hands the blank case to the presence validator alone, so a blank + # field yields one message instead of two. validates :percentage, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 100 } + presence: { message: "Choose a percentage." }, + numericality: { + only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 100, + allow_nil: true, + message: "Enter a percentage between 0 and 100." + } validate :within_ongoing_percentage_total def dollar_amount @@ -26,10 +32,12 @@ def one_time? def within_ongoing_percentage_total return if percentage.blank? || scenario.blank? + return if errors[:percentage].any? others = scenario.ongoing_allocations.where.not(id: id).sum(:percentage) - if others + percentage > 100 - errors.add(:percentage, "would bring ongoing giving to #{others + percentage}%, over 100%") - end + return if others + percentage <= 100 + + remaining = [ 100 - others, 0 ].max + errors.add(:percentage, "You have #{remaining}% left to allocate.") end end diff --git a/app/views/scenarios/_category_picker.html.erb b/app/views/scenarios/_category_picker.html.erb index 77e9be2..bc7de63 100644 --- a/app/views/scenarios/_category_picker.html.erb +++ b/app/views/scenarios/_category_picker.html.erb @@ -3,8 +3,12 @@ <% children_by_parent = categories.group_by(&:parent_id) %> <% selected = allocation.allocation_category %> <% initial_type = selected&.type || AllocationCategory::TAB_CLASSES.first %> +<%# :base is where the category-or-option validation lands, so it renders on the picker. %> +<% base_errors = allocation.errors[:base] %>
Category @@ -15,13 +19,19 @@ data: { allocation_category_picker_target: "optionField" } %> + <%= tag.p base_errors.to_sentence.presence || "Choose a category or enter a custom option", + data: { allocation_category_picker_target: "error" }, + hidden: base_errors.none?, + class: "mt-1 text-sm text-danger" %> +