From 11d47f5de1ded03b8af0898fb68d607c1ce508dd Mon Sep 17 00:00:00 2001 From: ByteAtATime Date: Mon, 9 Nov 2026 17:23:31 -0800 Subject: [PATCH 1/2] [Reimbursements] Don't let card grant recipients pick a reviewer --- app/models/reimbursement/report.rb | 2 +- .../events/settings/_reimbursements.html.erb | 1 + .../reimbursement/reports_controller_spec.rb | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/models/reimbursement/report.rb b/app/models/reimbursement/report.rb index b5750f724b..221447c858 100644 --- a/app/models/reimbursement/report.rb +++ b/app/models/reimbursement/report.rb @@ -326,7 +326,7 @@ def team_review_required? end def reviewer_selectable? - !event.reviewer_selection_disabled? + card_grant.blank? && !event.reviewer_selection_disabled? end def assigned_reviewer_required? diff --git a/app/views/events/settings/_reimbursements.html.erb b/app/views/events/settings/_reimbursements.html.erb index 6d4edc998a..d3bfdd7c2c 100644 --- a/app/views/events/settings/_reimbursements.html.erb +++ b/app/views/events/settings/_reimbursements.html.erb @@ -40,6 +40,7 @@
<%= form.label :reimbursement_reviewer_selection, "Let submitters pick a reviewer" %> <%= form.select :reimbursement_reviewer_selection, [["Disabled", "disabled"], ["Enabled, optional", "optional"], ["Enabled, required", "required"]], {}, { disabled: } %> +

This doesn’t apply to reports converted from card grants. Those submitters can’t pick a reviewer, but one can still be assigned from the report’s settings menu.

diff --git a/spec/controllers/reimbursement/reports_controller_spec.rb b/spec/controllers/reimbursement/reports_controller_spec.rb index 213ae24bf8..37ac61f0ea 100644 --- a/spec/controllers/reimbursement/reports_controller_spec.rb +++ b/spec/controllers/reimbursement/reports_controller_spec.rb @@ -92,6 +92,23 @@ def header_badges(body) expect(selects.map { |select| select["required"] }).to all(eq("required")) end + it "never asks card grant submitters for a reviewer" do + event = create(:event, :card_grant_event, :with_positive_balance) + member = create(:user) + create(:organizer_position, user: member, event:, role: :member) + card_grant = create(:card_grant, event:, user: member) + report = create(:reimbursement_report, user: member, event:, card_grant:) + create_session(member, verified: true) + + event.update!(reimbursement_reviewer_selection: :optional) + get(:show, params: { id: report.id }) + expect(Nokogiri::HTML5(response.body).css('select[name="reviewer_id"]')).to be_empty + + event.update!(reimbursement_reviewer_selection: :required) + get(:show, params: { id: report.id }) + expect(Nokogiri::HTML5(response.body).css('select[name="reviewer_id"]')).to be_empty + end + it "shows the status badge when no reviewer is assigned" do creator = create(:user) event = create(:event) From e147f6433c3186d57aceaa83f6006ee4cbb0e839 Mon Sep 17 00:00:00 2001 From: ByteAtATime Date: Mon, 9 Nov 2026 17:37:15 -0800 Subject: [PATCH 2/2] [Reimbursements] Let card grant recipients assign a reviewer from report settings --- .../reimbursement/reports_controller.rb | 2 +- app/models/reimbursement/report.rb | 4 ++ .../reimbursement/reports/_edit_form.html.erb | 5 +++ app/views/reimbursement/reports/show.html.erb | 2 +- .../reimbursement/reports_controller_spec.rb | 42 ++++++++++++++++++- 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/app/controllers/reimbursement/reports_controller.rb b/app/controllers/reimbursement/reports_controller.rb index 75aea78817..3d473a535d 100644 --- a/app/controllers/reimbursement/reports_controller.rb +++ b/app/controllers/reimbursement/reports_controller.rb @@ -481,7 +481,7 @@ def update_reimbursement_report_params reimbursement_report_params = params.require(:reimbursement_report).permit(:report_name, :event_id, :maximum_amount, :reviewer_id).compact reimbursement_report_params.delete(:maximum_amount) unless admin_signed_in? || OrganizerPosition.role_at_least?(current_user, @event, :manager) reimbursement_report_params.delete(:maximum_amount) unless @report.draft? || @report.submitted? - reimbursement_report_params.delete(:reviewer_id) unless admin_signed_in? || OrganizerPosition.role_at_least?(current_user, @event, :manager) + reimbursement_report_params.delete(:reviewer_id) unless admin_signed_in? || OrganizerPosition.role_at_least?(current_user, @event, :manager) || (@report.reviewer_assignable_by_creator? && @report.user == current_user && (@report.draft? || @report.submitted?)) reimbursement_report_params end diff --git a/app/models/reimbursement/report.rb b/app/models/reimbursement/report.rb index 221447c858..c2f826ebd5 100644 --- a/app/models/reimbursement/report.rb +++ b/app/models/reimbursement/report.rb @@ -329,6 +329,10 @@ def reviewer_selectable? card_grant.blank? && !event.reviewer_selection_disabled? end + def reviewer_assignable_by_creator? + card_grant.present? && !event.reviewer_selection_disabled? + end + def assigned_reviewer_required? event.reviewer_selection_required? end diff --git a/app/views/reimbursement/reports/_edit_form.html.erb b/app/views/reimbursement/reports/_edit_form.html.erb index 163c7a9b10..55f54a8774 100644 --- a/app/views/reimbursement/reports/_edit_form.html.erb +++ b/app/views/reimbursement/reports/_edit_form.html.erb @@ -35,6 +35,11 @@ <%= form.label :reviewer_id, "Assigned reviewer", class: "mt2" %> <%= form.collection_select :reviewer_id, @report.event.managers, :id, :name, include_blank: "Anyone" %> + <% elsif @report.reviewer_assignable_by_creator? && current_user == @report.user && (@report.submitted? || @report.draft?) %> +
+ <%= form.label :reviewer_id, "Reviewer", class: "mt2" %> + <%= form.collection_select :reviewer_id, @report.eligible_reviewers, :id, :name, include_blank: "No preference" %> +
<% end %>
<%= form.submit "Update report", class: "left mt2" %> diff --git a/app/views/reimbursement/reports/show.html.erb b/app/views/reimbursement/reports/show.html.erb index fedbf504b8..b3b5cc8473 100644 --- a/app/views/reimbursement/reports/show.html.erb +++ b/app/views/reimbursement/reports/show.html.erb @@ -55,7 +55,7 @@ style: "height: 32px; width: 32px; background: #338eda20; color: #338eda", 'aria-label': "View the associated card grant." %> <% end %> - <% unless @report.closed? || !(organizer_signed_in?(as: :member) || policy(@report).change_event?) %> + <% unless @report.closed? || !(organizer_signed_in?(as: :member) || policy(@report).change_event? || (@report.reviewer_assignable_by_creator? && policy(@report).edit?)) %> <%= pop_icon_to "settings", "#", data: { behavior: "modal_trigger", modal: "edit" }, diff --git a/spec/controllers/reimbursement/reports_controller_spec.rb b/spec/controllers/reimbursement/reports_controller_spec.rb index 37ac61f0ea..0ce08cc591 100644 --- a/spec/controllers/reimbursement/reports_controller_spec.rb +++ b/spec/controllers/reimbursement/reports_controller_spec.rb @@ -102,7 +102,9 @@ def header_badges(body) event.update!(reimbursement_reviewer_selection: :optional) get(:show, params: { id: report.id }) - expect(Nokogiri::HTML5(response.body).css('select[name="reviewer_id"]')).to be_empty + body = Nokogiri::HTML5(response.body) + expect(body.css('select[name="reviewer_id"]')).to be_empty + expect(body.css('select[name="reimbursement_report[reviewer_id]"]')).not_to be_empty event.update!(reimbursement_reviewer_selection: :required) get(:show, params: { id: report.id }) @@ -416,6 +418,44 @@ def header_badges(body) expect(report.reload.reviewer_id).to be_nil end + it "lets a card grant recipient assign a reviewer from the settings menu" do + event = create(:event, :card_grant_event, :with_positive_balance) + recipient = create(:user) + reviewer = create(:user) + create(:organizer_position, user: reviewer, event:) + card_grant = create(:card_grant, event:, user: recipient) + report = create(:reimbursement_report, user: recipient, event:, card_grant:) + + event.update!(reimbursement_reviewer_selection: :optional) + create_session(recipient, verified: true) + + patch(:update, params: { + id: report.id, + reimbursement_report: { report_name: report.name, reviewer_id: reviewer.id } + }) + + expect(report.reload.reviewer_id).to eq(reviewer.id) + end + + it "rejects an ineligible reviewer from a card grant recipient" do + event = create(:event, :card_grant_event, :with_positive_balance) + recipient = create(:user) + outsider = create(:user) + card_grant = create(:card_grant, event:, user: recipient) + report = create(:reimbursement_report, user: recipient, event:, card_grant:) + + event.update!(reimbursement_reviewer_selection: :optional) + create_session(recipient, verified: true) + + patch(:update, params: { + id: report.id, + reimbursement_report: { report_name: report.name, reviewer_id: outsider.id } + }) + + expect(response).to have_http_status(:unprocessable_content) + expect(report.reload.reviewer_id).to be_nil + end + it "allows a manager to assign a reviewer on their own report" do manager = create(:user) event = create(:event)