From 5332ae609ec503166ea2b474cbc6e8cba3fd9c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 8 Sep 2020 19:11:38 +0200 Subject: [PATCH] Filter investments only by assigned staff In commit 74083df1 we added the possibility to assign administrators and valuators to budgets, so they would only manage the budgets they're assigned to. However, when filtering projects, we were still showing all administrators and valuators as options to filter investments. It makes more sense to only show the valuators and administrators assigned to the current budget. Note this change only affects the view, and so malicious users could technically send any other administrator or valuator ID. In this case, they would get empty results since those administrators/valuators wouldn't have any investments assigned, so taking this case into account is not necessary. --- .../admin_budget_investments_helper.rb | 12 +++---- .../budget_investments/_search_form.html.erb | 4 +-- .../admin_budget_investments_helper_spec.rb | 35 +++++++++++++++++++ spec/system/admin/budget_investments_spec.rb | 6 +++- 4 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 spec/helpers/admin_budget_investments_helper_spec.rb diff --git a/app/helpers/admin_budget_investments_helper.rb b/app/helpers/admin_budget_investments_helper.rb index 08c600243..f34fbcb65 100644 --- a/app/helpers/admin_budget_investments_helper.rb +++ b/app/helpers/admin_budget_investments_helper.rb @@ -13,16 +13,16 @@ module AdminBudgetInvestmentsHelper params[:advanced_filters] = [] unless params[:advanced_filters] end - def admin_select_options - Administrator.with_user.map { |v| [v.description_or_name, v.id] }.sort_by { |a| a[0] } + def admin_select_options(budget) + budget.administrators.with_user.map { |v| [v.description_or_name, v.id] }.sort_by { |a| a[0] } end - def valuator_or_group_select_options - valuator_group_select_options + valuator_select_options + def valuator_or_group_select_options(budget) + valuator_group_select_options + valuator_select_options(budget) end - def valuator_select_options - Valuator.order("description ASC").order("users.email ASC").includes(:user). + def valuator_select_options(budget) + budget.valuators.order("description ASC").order("users.email ASC").includes(:user). map { |v| [v.description_or_email, "valuator_#{v.id}"] } end diff --git a/app/views/admin/budget_investments/_search_form.html.erb b/app/views/admin/budget_investments/_search_form.html.erb index 890700340..8b78a8f73 100644 --- a/app/views/admin/budget_investments/_search_form.html.erb +++ b/app/views/admin/budget_investments/_search_form.html.erb @@ -34,12 +34,12 @@
<%= select_tag :administrator_id, - options_for_select(admin_select_options, params[:administrator_id]), + options_for_select(admin_select_options(@budget), params[:administrator_id]), { prompt: t("admin.budget_investments.index.administrator_filter_all") } %>
<%= select_tag :valuator_or_group_id, - options_for_select(valuator_or_group_select_options, params[:valuator_or_group_id]), + options_for_select(valuator_or_group_select_options(@budget), params[:valuator_or_group_id]), { prompt: t("admin.budget_investments.index.valuator_filter_all") } %>
diff --git a/spec/helpers/admin_budget_investments_helper_spec.rb b/spec/helpers/admin_budget_investments_helper_spec.rb new file mode 100644 index 000000000..2c8a0294d --- /dev/null +++ b/spec/helpers/admin_budget_investments_helper_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +describe AdminBudgetInvestmentsHelper do + describe "#admin_select_options" do + it "includes administrators assigned to the budget" do + admin = create(:administrator, user: create(:user, username: "Winston")) + budget = create(:budget, administrators: [admin]) + + expect(admin_select_options(budget)).to eq([["Winston", admin.id]]) + end + + it "does not include other administrators" do + create(:administrator, user: create(:user, username: "Winston")) + budget = create(:budget, administrators: []) + + expect(admin_select_options(budget)).to be_empty + end + end + + describe "#valuator_select_options" do + it "includes valuators assigned to the budget" do + valuator = create(:valuator, description: "Kodogo") + budget = create(:budget, valuators: [valuator]) + + expect(valuator_select_options(budget)).to eq([["Kodogo", "valuator_#{valuator.id}"]]) + end + + it "does not include other valuators" do + create(:valuator, description: "Kodogo") + budget = create(:budget, valuators: []) + + expect(valuator_select_options(budget)).to be_empty + end + end +end diff --git a/spec/system/admin/budget_investments_spec.rb b/spec/system/admin/budget_investments_spec.rb index 8b2f1f72b..ec8d4c9ec 100644 --- a/spec/system/admin/budget_investments_spec.rb +++ b/spec/system/admin/budget_investments_spec.rb @@ -160,7 +160,8 @@ describe "Admin budget investments" do user = create(:user, username: "Admin 1") user2 = create(:user, username: "Admin 2") administrator = create(:administrator, user: user) - create(:administrator, user: user2, description: "Alias") + administrator2 = create(:administrator, user: user2, description: "Alias") + budget.administrators = [administrator, administrator2] create(:budget_investment, title: "Realocate visitors", budget: budget, administrator: administrator) create(:budget_investment, title: "Destroy the city", budget: budget) @@ -201,6 +202,7 @@ describe "Admin budget investments" do scenario "Filtering by valuator", :js do user = create(:user) valuator = create(:valuator, user: user, description: "Valuator 1") + budget.valuators = [valuator] create(:budget_investment, title: "Realocate visitors", budget: budget, valuators: [valuator]) create(:budget_investment, title: "Destroy the city", budget: budget) @@ -645,6 +647,7 @@ describe "Admin budget investments" do scenario "Combination of checkbox with text search", :js do user = create(:user, username: "Admin 1") administrator = create(:administrator, user: user) + budget.administrators = [administrator] create(:budget_investment, budget: budget, title: "Educate the children", administrator: administrator) @@ -716,6 +719,7 @@ describe "Admin budget investments" do scenario "Combination of checkbox with text search and checkbox", :js do user = create(:user, username: "Admin 1") administrator = create(:administrator, user: user) + budget.administrators = [administrator] create(:budget_investment, :feasible, :finished, budget: budget, title: "Educate the children", administrator: administrator)