From 9ab559d5d800feccec5cb00aafa002a3e11f80c1 Mon Sep 17 00:00:00 2001 From: Mike Dalton Date: Tue, 23 Jun 2026 23:05:52 -0400 Subject: [PATCH] Let org admins view and edit any scenario in their organization Admins and owners can now open any scenario in their org from the admin scenarios dashboard and use the existing edit UI (name, total giving amount, allocations) to view, edit, and delete it. Access is resolved through the org membership: a new ScenarioScoping concern scopes lookups to org-wide for admins/owners and own-only for everyone else (including super admins, who have no membership). The admin dashboard rows link to each scenario, and the show page shows an "on behalf of" banner plus a back-link to the dashboard when an admin is viewing someone else's plan. Co-Authored-By: Claude Opus 4.8 --- app/controllers/allocations_controller.rb | 4 +- app/controllers/concerns/scenario_scoping.rb | 21 ++++++++++ app/controllers/scenarios/names_controller.rb | 4 +- .../total_giving_amounts_controller.rb | 4 +- app/controllers/scenarios_controller.rb | 17 ++++---- app/models/current.rb | 4 ++ app/models/organization_membership.rb | 12 ++++++ app/views/admin/scenarios/index.html.erb | 12 ++++-- app/views/scenarios/show.html.erb | 15 ++++++- .../admin/scenarios_controller_test.rb | 9 ++++ .../allocations_controller_test.rb | 28 +++++++++++++ .../scenarios/names_controller_test.rb | 16 ++++++++ .../total_giving_amounts_controller_test.rb | 16 ++++++++ test/controllers/scenarios_controller_test.rb | 41 +++++++++++++++++++ test/models/organization_membership_test.rb | 20 +++++++++ 15 files changed, 206 insertions(+), 17 deletions(-) create mode 100644 app/controllers/concerns/scenario_scoping.rb diff --git a/app/controllers/allocations_controller.rb b/app/controllers/allocations_controller.rb index b708dca..d7ec421 100644 --- a/app/controllers/allocations_controller.rb +++ b/app/controllers/allocations_controller.rb @@ -1,4 +1,6 @@ class AllocationsController < ApplicationController + include ScenarioScoping + before_action :set_scenario def create @@ -27,7 +29,7 @@ def destroy private def set_scenario - @scenario = Current.user.scenarios.where(organization: Current.organization).find(params[:scenario_id]) + @scenario = accessible_scenarios.find(params[:scenario_id]) end def allocation_params diff --git a/app/controllers/concerns/scenario_scoping.rb b/app/controllers/concerns/scenario_scoping.rb new file mode 100644 index 0000000..6640c07 --- /dev/null +++ b/app/controllers/concerns/scenario_scoping.rb @@ -0,0 +1,21 @@ +module ScenarioScoping + extend ActiveSupport::Concern + + included do + helper_method :viewing_on_behalf? + end + + private + + def accessible_scenarios + Current.organization_membership&.accessible_scenarios || owned_scenarios + end + + def owned_scenarios + Current.user.scenarios.where(organization: Current.organization) + end + + def viewing_on_behalf?(scenario) + scenario.present? && scenario.user_id != Current.user.id + end +end diff --git a/app/controllers/scenarios/names_controller.rb b/app/controllers/scenarios/names_controller.rb index b7c8037..01aaba4 100644 --- a/app/controllers/scenarios/names_controller.rb +++ b/app/controllers/scenarios/names_controller.rb @@ -1,4 +1,6 @@ class Scenarios::NamesController < ApplicationController + include ScenarioScoping + before_action :set_scenario def show @@ -18,7 +20,7 @@ def update private def set_scenario - @scenario = Current.user.scenarios.where(organization: Current.organization).find(params[:scenario_id]) + @scenario = accessible_scenarios.find(params[:scenario_id]) end def name_params diff --git a/app/controllers/scenarios/total_giving_amounts_controller.rb b/app/controllers/scenarios/total_giving_amounts_controller.rb index 631e8f6..54910a9 100644 --- a/app/controllers/scenarios/total_giving_amounts_controller.rb +++ b/app/controllers/scenarios/total_giving_amounts_controller.rb @@ -1,4 +1,6 @@ class Scenarios::TotalGivingAmountsController < ApplicationController + include ScenarioScoping + before_action :set_scenario def show @@ -18,7 +20,7 @@ def update private def set_scenario - @scenario = Current.user.scenarios.where(organization: Current.organization).find(params[:scenario_id]) + @scenario = accessible_scenarios.find(params[:scenario_id]) end def total_giving_amount_params diff --git a/app/controllers/scenarios_controller.rb b/app/controllers/scenarios_controller.rb index 31e186a..f3ff772 100644 --- a/app/controllers/scenarios_controller.rb +++ b/app/controllers/scenarios_controller.rb @@ -1,19 +1,21 @@ class ScenariosController < ApplicationController + include ScenarioScoping + before_action :set_scenario, only: %i[ show update destroy ] def index - @scenarios = scenarios.order(created_at: :desc) + @scenarios = owned_scenarios.order(created_at: :desc) end def show end def new - @scenario = scenarios.new + @scenario = owned_scenarios.new end def create - @scenario = scenarios.new(scenario_params) + @scenario = owned_scenarios.new(scenario_params) if @scenario.save redirect_to scenario_path(@scenario) else @@ -30,18 +32,15 @@ def update end def destroy + on_behalf = viewing_on_behalf?(@scenario) @scenario.destroy - redirect_to scenarios_path + redirect_to on_behalf ? admin_scenarios_path : scenarios_path end private - def scenarios - Current.user.scenarios.where(organization: Current.organization) - end - def set_scenario - @scenario = scenarios.find(params[:id]) + @scenario = accessible_scenarios.find(params[:id]) end def scenario_params diff --git a/app/models/current.rb b/app/models/current.rb index 68edde0..9f18593 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -2,4 +2,8 @@ class Current < ActiveSupport::CurrentAttributes attribute :session attribute :organization delegate :user, to: :session, allow_nil: true + + def organization_membership + user&.membership_in(organization) + end end diff --git a/app/models/organization_membership.rb b/app/models/organization_membership.rb index 5c641b3..559ae85 100644 --- a/app/models/organization_membership.rb +++ b/app/models/organization_membership.rb @@ -6,4 +6,16 @@ class OrganizationMembership < ApplicationRecord validates :user_id, uniqueness: { scope: :organization_id } validates :role, presence: true + + def accessible_scenarios + if admin? || owner? + organization.scenarios + else + owned_scenarios + end + end + + def owned_scenarios + user.scenarios.where(organization: organization) + end end diff --git a/app/views/admin/scenarios/index.html.erb b/app/views/admin/scenarios/index.html.erb index 95ffa19..3c4ed4f 100644 --- a/app/views/admin/scenarios/index.html.erb +++ b/app/views/admin/scenarios/index.html.erb @@ -32,10 +32,16 @@ <% @scenarios.each do |scenario| %> - <%= scenario.user.display_name %> - <%= scenario.name %> + + <%= link_to scenario.user.display_name, scenario_path(scenario), data: { turbo_frame: "_top" }, class: "block hover:underline" %> + + + <%= link_to scenario.name, scenario_path(scenario), data: { turbo_frame: "_top" }, class: "block hover:underline" %> + - <%= scenario.total_giving_amount.present? ? number_to_currency(scenario.total_giving_amount, precision: 0) : content_tag(:span, "—", class: "text-ink-faint") %> + <%= link_to scenario_path(scenario), data: { turbo_frame: "_top" }, class: "block" do %> + <%= scenario.total_giving_amount.present? ? number_to_currency(scenario.total_giving_amount, precision: 0) : content_tag(:span, "—", class: "text-ink-faint") %> + <% end %> <% end %> diff --git a/app/views/scenarios/show.html.erb b/app/views/scenarios/show.html.erb index 1c191ee..7c70cec 100644 --- a/app/views/scenarios/show.html.erb +++ b/app/views/scenarios/show.html.erb @@ -1,6 +1,17 @@
- <%= link_to "← Back to explore options", scenarios_path, - class: "inline-block rounded-md border border-line bg-surface px-3 py-1.5 text-sm text-ink-soft hover:bg-canvas transition" %> + <% if viewing_on_behalf?(@scenario) %> + <%= link_to "← Back to all scenarios", admin_scenarios_path, + class: "inline-block rounded-md border border-line bg-surface px-3 py-1.5 text-sm text-ink-soft hover:bg-canvas transition" %> + <% else %> + <%= link_to "← Back to explore options", scenarios_path, + class: "inline-block rounded-md border border-line bg-surface px-3 py-1.5 text-sm text-ink-soft hover:bg-canvas transition" %> + <% end %> + + <% if viewing_on_behalf?(@scenario) %> +
+ Viewing <%= @scenario.user.display_name %>'s scenario as an admin. Changes you make are saved to their account. +
+ <% end %> <%= render "scenarios/names/name", scenario: @scenario %> diff --git a/test/controllers/admin/scenarios_controller_test.rb b/test/controllers/admin/scenarios_controller_test.rb index 47d9f9d..a19b5fb 100644 --- a/test/controllers/admin/scenarios_controller_test.rb +++ b/test/controllers/admin/scenarios_controller_test.rb @@ -34,6 +34,15 @@ class Admin::ScenariosControllerTest < ActionDispatch::IntegrationTest assert_select "td", text: @admin.display_name end + test "rows link to the scenario and break out of the table turbo frame" do + sign_in_as(@owner) + get admin_scenarios_path + + # data-turbo-frame="_top" is required so the link navigates the whole page + # instead of trying (and failing) to render into the scenarios_table frame. + assert_select "a[href=?][data-turbo-frame=_top]", scenario_path(scenarios(:one_arlington)) + end + test "does not show scenarios from another organization" do sign_in_as(@owner) get admin_scenarios_path diff --git a/test/controllers/allocations_controller_test.rb b/test/controllers/allocations_controller_test.rb index a037dad..754c8ff 100644 --- a/test/controllers/allocations_controller_test.rb +++ b/test/controllers/allocations_controller_test.rb @@ -103,4 +103,32 @@ class AllocationsControllerTest < ActionDispatch::IntegrationTest end assert_response :not_found end + + test "admin can add an allocation to another user's scenario in the same org" do + sign_in_as users(:admin) + assert_difference -> { @scenario.allocations.count }, 1 do + post scenario_allocations_url(@scenario), params: { + allocation: { type: "Allocation::Ongoing", option: "Population: Youth", percentage: 25 } + } + end + assert_redirected_to scenario_path(@scenario) + end + + test "admin can destroy an allocation on another user's scenario in the same org" do + sign_in_as users(:admin) + assert_difference -> { @scenario.allocations.count }, -1 do + delete scenario_allocation_url(@scenario, allocations(:greatest_need)) + end + assert_redirected_to scenario_path(@scenario) + end + + test "plain member cannot add an allocation to another user's scenario in the same org" do + sign_in_as users(:passwordless) + assert_no_difference -> { Allocation.count } do + post scenario_allocations_url(@scenario), params: { + allocation: { type: "Allocation::Ongoing", option: "X", percentage: 10 } + } + end + assert_response :not_found + end end diff --git a/test/controllers/scenarios/names_controller_test.rb b/test/controllers/scenarios/names_controller_test.rb index 937e24b..162fe53 100644 --- a/test/controllers/scenarios/names_controller_test.rb +++ b/test/controllers/scenarios/names_controller_test.rb @@ -35,4 +35,20 @@ class Scenarios::NamesControllerTest < ActionDispatch::IntegrationTest get edit_scenario_name_url(scenarios(:two_boston)) assert_response :not_found end + + test "admin can edit another user's scenario name in the same org" do + sign_in_as users(:admin) + get edit_scenario_name_url(@scenario) + assert_response :success + + patch scenario_name_url(@scenario), params: { scenario: { name: "Admin renamed" } } + assert_response :success + assert_equal "Admin renamed", @scenario.reload.name + end + + test "plain member cannot edit another user's scenario name in the same org" do + sign_in_as users(:passwordless) + get edit_scenario_name_url(@scenario) + assert_response :not_found + end end diff --git a/test/controllers/scenarios/total_giving_amounts_controller_test.rb b/test/controllers/scenarios/total_giving_amounts_controller_test.rb index c74978b..0281475 100644 --- a/test/controllers/scenarios/total_giving_amounts_controller_test.rb +++ b/test/controllers/scenarios/total_giving_amounts_controller_test.rb @@ -29,4 +29,20 @@ class Scenarios::TotalGivingAmountsControllerTest < ActionDispatch::IntegrationT get edit_scenario_total_giving_amount_url(scenarios(:two_boston)) assert_response :not_found end + + test "admin can edit another user's scenario total in the same org" do + sign_in_as users(:admin) + get edit_scenario_total_giving_amount_url(@scenario) + assert_response :success + + patch scenario_total_giving_amount_url(@scenario), params: { scenario: { total_giving_amount: 7500 } } + assert_response :success + assert_equal 7500, @scenario.reload.total_giving_amount + end + + test "plain member cannot edit another user's scenario total in the same org" do + sign_in_as users(:passwordless) + get edit_scenario_total_giving_amount_url(@scenario) + assert_response :not_found + end end diff --git a/test/controllers/scenarios_controller_test.rb b/test/controllers/scenarios_controller_test.rb index 33a63f0..8b2131b 100644 --- a/test/controllers/scenarios_controller_test.rb +++ b/test/controllers/scenarios_controller_test.rb @@ -63,4 +63,45 @@ class ScenariosControllerTest < ActionDispatch::IntegrationTest get scenario_url(scenarios(:two_boston)) assert_response :not_found end + + test "admin can view another user's scenario in the same org" do + sign_in_as users(:admin) + get scenario_url(scenarios(:one_arlington)) + assert_response :success + assert_select "a[href=?]", admin_scenarios_path + assert_match "as an admin", response.body + end + + test "admin can update another user's scenario" do + sign_in_as users(:admin) + patch scenario_url(scenarios(:one_arlington)), params: { scenario: { total_giving_amount: 999 } } + assert_equal 999, scenarios(:one_arlington).reload.total_giving_amount + end + + test "admin destroy of another user's scenario returns to the admin dashboard" do + sign_in_as users(:admin) + assert_difference -> { Scenario.count }, -1 do + delete scenario_url(scenarios(:one_arlington)) + end + assert_redirected_to admin_scenarios_path + end + + test "admin still cannot reach a scenario in another org" do + sign_in_as users(:admin) + get scenario_url(scenarios(:two_boston)) + assert_response :not_found + end + + test "plain member cannot reach another user's scenario in the same org" do + sign_in_as users(:passwordless) + get scenario_url(scenarios(:one_arlington)) + assert_response :not_found + end + + test "super admin cannot change another user's scenario" do + sign_in_as users(:super_admin) + patch scenario_url(scenarios(:one_arlington)), params: { scenario: { total_giving_amount: 1 } } + assert_response :not_found + assert_equal 10000, scenarios(:one_arlington).reload.total_giving_amount + end end diff --git a/test/models/organization_membership_test.rb b/test/models/organization_membership_test.rb index 7d4b0eb..1f2843d 100644 --- a/test/models/organization_membership_test.rb +++ b/test/models/organization_membership_test.rb @@ -37,4 +37,24 @@ class OrganizationMembershipTest < ActiveSupport::TestCase assert_not membership.valid? assert_includes membership.errors[:role], "can't be blank" end + + test "owned_scenarios are the member's own scenarios in the organization" do + membership = organization_memberships(:one_arlington) + assert_includes membership.owned_scenarios, scenarios(:one_arlington) + assert_not_includes membership.owned_scenarios, scenarios(:admin_arlington) + end + + test "admins and owners can access any scenario in the organization" do + %i[ admin_arlington one_arlington ].each do |fixture| + membership = organization_memberships(fixture) + assert_includes membership.accessible_scenarios, scenarios(:one_arlington) + assert_includes membership.accessible_scenarios, scenarios(:admin_arlington) + end + end + + test "plain members can only access their own scenarios" do + membership = organization_memberships(:passwordless_arlington) + assert_equal membership.owned_scenarios.to_a, membership.accessible_scenarios.to_a + assert_not_includes membership.accessible_scenarios, scenarios(:one_arlington) + end end