Fix scenario page error rendering

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

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 <dialog>, 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

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()
   }

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?
@@ -19,36 +24,21 @@ 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?
+    return if errors[:amount].any?
 
-    remaining = budget_remaining(scenario)
-    return if remaining.nil?
+    total = scenario.total_giving_amount.to_i
+    others = scenario.one_time_allocations.where.not(id: id).sum(:amount)
+    return if others + amount <= total
 
-    if amount > remaining
-      others = scenario.total_giving_amount - remaining
-      errors.add(:amount, "would bring one-time giving to #{others + amount}, over the total giving amount of #{scenario.total_giving_amount.to_i}")
-    end
+    remaining = [ total - others, 0 ].max
+    errors.add(:amount, "You have #{money(remaining)} left to allocate.")
   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
+  def money(value)
+    ActiveSupport::NumberHelper.number_to_currency(value, precision: 0)
   end
 end

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

app/views/scenarios/_allocation_form.html.erb

@@ -0,0 +1,61 @@
+<%# 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), 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 %>
+  <h2 class="font-serif font-medium text-2xl text-ink"><%= editing ? "Edit allocation" : "Create allocation" %></h2>
+
+  <%= render "scenarios/category_picker", allocation: allocation, scenario: scenario %>
+
+  <% if klass == Allocation::Ongoing %>
+    <div class="mt-6" data-controller="allocation-slider" data-allocation-slider-limit-value="<%= remaining_percentage %>">
+      <div class="flex items-center justify-between">
+        <span class="text-sm text-ink-soft">Giving percentage %</span>
+        <span id="allocation_headroom_<%= suffix %>" class="text-sm text-ink-faint"><%= remaining_percentage %>% remaining</span>
+      </div>
+      <p class="mt-1 text-lg font-medium text-ink" data-allocation-slider-target="percent"><%= percentage %>%</p>
+      <%# 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) ] %>
+      <%= render "scenarios/field_errors", messages: allocation.errors[:percentage] %>
+    </div>
+  <% else %>
+    <div class="mt-6">
+      <%= label_tag "allocation_amount_#{suffix}", "Amount", class: "block text-sm text-ink-soft" %>
+      <div class="relative mt-1">
+        <span class="absolute left-3 top-1/2 -translate-y-1/2 text-ink-faint">$</span>
+        <%= 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 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"}" %>
+      </div>
+      <%= render "scenarios/field_errors", messages: allocation.errors[:amount] %>
+    </div>
+  <% end %>
+
+  <%= render "scenarios/additional_preferences", allocation: allocation, scenario: scenario, suffix: suffix %>
+
+  <div class="mt-6">
+    <%= 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" %>
+  </div>
+
+  <div class="mt-8 flex justify-end gap-3">
+    <button type="button" data-action="dialog#close"
+            class="rounded-md border border-line bg-surface px-5 py-2.5 font-medium text-ink-soft hover:bg-canvas cursor-pointer transition">
+      Cancel
+    </button>
+    <%= 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" %>
+  </div>
+<% end %>

app/views/scenarios/_allocation_modal.html.erb

@@ -1,60 +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 %>
 <dialog data-dialog-target="dialog" data-action="click->dialog#backdropClose" class="m-auto w-full max-w-lg rounded-2xl p-0 shadow-lg backdrop:bg-[rgba(20,18,14,0.48)]">
-  <%= 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 %>
-    <h2 class="font-serif font-medium text-2xl text-ink"><%= editing ? "Edit allocation" : "Create allocation" %></h2>
-
-    <%= render "scenarios/category_picker", allocation: allocation, scenario: scenario %>
-
-    <% if klass == Allocation::Ongoing %>
-      <div class="mt-6" data-controller="allocation-slider" data-allocation-slider-limit-value="<%= remaining_percentage %>">
-        <div class="flex items-center justify-between">
-          <span class="text-sm text-ink-soft">Giving percentage %</span>
-          <span id="allocation_headroom_<%= suffix %>" class="text-sm text-ink-faint"><%= remaining_percentage %>% remaining</span>
-        </div>
-        <p class="mt-1 text-lg font-medium text-ink" data-allocation-slider-target="percent"><%= percentage %>%</p>
-        <%# 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) ] %>
-      </div>
-    <% else %>
-      <% max_amount = allocation.max_amount(scenario) if klass == Allocation::OneTime %>
-      <div class="mt-6">
-        <%= label_tag "allocation_amount_#{suffix}", "Amount", class: "block text-sm text-ink-soft" %>
-        <div class="relative mt-1">
-          <span class="absolute left-3 top-1/2 -translate-y-1/2 text-ink-faint">$</span>
-          <%= number_field_tag "allocation[amount]", allocation.amount, id: "allocation_amount_#{suffix}", min: 0, max: max_amount, 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" },
-                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" %>
-        </div>
-      </div>
-    <% end %>
-
-    <%= render "scenarios/additional_preferences", allocation: allocation, scenario: scenario, suffix: suffix %>
-
-    <div class="mt-6">
-      <%= 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" %>
-    </div>
-
-    <div class="mt-8 flex justify-end gap-3">
-      <button type="button" data-action="dialog#close"
-              class="rounded-md border border-line bg-surface px-5 py-2.5 font-medium text-ink-soft hover:bg-canvas cursor-pointer transition">
-        Cancel
-      </button>
-      <%= 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" %>
-    </div>
-  <% end %>
+  <%= render "scenarios/allocation_form", allocation: allocation, scenario: scenario, color: color %>
 </dialog>