From e9d73eb687e35c7a610094a24ec2d7993d93eeb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 10 Apr 2018 17:53:57 +0200 Subject: [PATCH] Fix `max_per_heading` filter in Admin budget investments list Also changed the name of the param to `min_total_supports`, which is more descriptive on what it actually does. Backport of 75f20d5 and 07f0806 from AyuntamientoMadrid/consul fork --- .../admin_budget_investments_helper.rb | 2 +- app/models/budget/investment.rb | 15 +---- .../budget_investments/_investments.html.erb | 4 +- .../budget_investments/_search_form.html.erb | 2 +- config/locales/en/admin.yml | 2 +- config/locales/es/admin.yml | 2 +- .../features/admin/budget_investments_spec.rb | 61 +++++++------------ 7 files changed, 29 insertions(+), 59 deletions(-) diff --git a/app/helpers/admin_budget_investments_helper.rb b/app/helpers/admin_budget_investments_helper.rb index 742ff5bac..492225e35 100644 --- a/app/helpers/admin_budget_investments_helper.rb +++ b/app/helpers/admin_budget_investments_helper.rb @@ -1,7 +1,7 @@ module AdminBudgetInvestmentsHelper def advanced_menu_visibility - (params[:advanced_filters].empty? && params["max_per_heading"].blank?) ? 'hide' : '' + (params[:advanced_filters].empty? && params["min_total_supports"].blank?) ? 'hide' : '' end def init_advanced_menu diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index ee3292fcd..1e2d3887c 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -109,7 +109,8 @@ class Budget budget = Budget.find_by(slug: params[:budget_id]) || Budget.find_by(id: params[:budget_id]) results = Investment.by_budget(budget) - results = limit_results(budget, params, results) if params[:max_per_heading].present? + results = results.where("cached_votes_up + physical_votes >= ?", + params[:min_total_supports]) if params[:min_total_supports].present? results = results.where(group_id: params[:group_id]) if params[:group_id].present? results = results.by_tag(params[:tag_name]) if params[:tag_name].present? results = results.by_heading(params[:heading_id]) if params[:heading_id].present? @@ -132,18 +133,6 @@ class Budget results.where("budget_investments.id IN (?)", ids) end - def self.limit_results(budget, params, results) - max_per_heading = params[:max_per_heading].to_i - return results if max_per_heading <= 0 - - ids = [] - budget.headings.pluck(:id).each do |hid| - ids += Investment.where(heading_id: hid).order(confidence_score: :desc).limit(max_per_heading).pluck(:id) - end - - results.where("budget_investments.id IN (?)", ids) - end - def self.search_by_title_or_id(title_or_id, results) if title_or_id =~ /^[0-9]+$/ results.where(id: title_or_id) diff --git a/app/views/admin/budget_investments/_investments.html.erb b/app/views/admin/budget_investments/_investments.html.erb index 17b0315b1..db10e2210 100644 --- a/app/views/admin/budget_investments/_investments.html.erb +++ b/app/views/admin/budget_investments/_investments.html.erb @@ -112,7 +112,7 @@ investment, filter: params[:filter], sort_by: params[:sort_by], - max_per_heading: params[:max_per_heading], + min_total_supports: params[:min_total_supports], advanced_filters: params[:advanced_filters], page: params[:page]), method: :patch, @@ -125,7 +125,7 @@ investment, filter: params[:filter], sort_by: params[:sort_by], - max_per_heading: params[:max_per_heading], + min_total_supports: params[:min_total_supports], advanced_filters: params[:advanced_filters], page: params[:page]), method: :patch, diff --git a/app/views/admin/budget_investments/_search_form.html.erb b/app/views/admin/budget_investments/_search_form.html.erb index 0f3090c60..dc3269443 100644 --- a/app/views/admin/budget_investments/_search_form.html.erb +++ b/app/views/admin/budget_investments/_search_form.html.erb @@ -18,7 +18,7 @@ <% end %>
- <%= text_field_tag :max_per_heading, params["max_per_heading"], placeholder: t("admin.budget_investments.index.filters.max_per_heading") %> + <%= text_field_tag :min_total_supports, params["min_total_supports"], placeholder: t("admin.budget_investments.index.filters.min_total_supports") %>
diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 48ee46db6..aab916b53 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -167,7 +167,7 @@ en: selected: Selected undecided: Undecided unfeasible: Unfeasible - max_per_heading: Max. supports per heading + min_total_supports: Minimum supports winners: Winners one_filter_html: "Current applied filters: %{filter}" two_filters_html: "Current applied filters: %{filter}, %{advanced_filters}" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 8929f9e3b..d01c38e79 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -167,7 +167,7 @@ es: selected: Seleccionados undecided: Sin decidir unfeasible: Inviables - max_per_heading: Corte por partida + min_total_supports: Apoyos mínimos winners: Ganadores one_filter_html: "Filtros en uso: %{filter}" two_filters_html: "Filtros en uso: %{filter}, %{advanced_filters}" diff --git a/spec/features/admin/budget_investments_spec.rb b/spec/features/admin/budget_investments_spec.rb index de34f5298..874615475 100644 --- a/spec/features/admin/budget_investments_spec.rb +++ b/spec/features/admin/budget_investments_spec.rb @@ -371,59 +371,40 @@ feature 'Admin budget investments' do expect(page).to have_content 'The budget has to stay on phase "Balloting projects", "Reviewing Ballots" or "Finished budget" in order to calculate winners projects' end - scenario "Limiting by max number of investments per heading", :js do + scenario "Filtering by minimum number of votes", :js do group_1 = create(:budget_group, budget: budget) group_2 = create(:budget_group, budget: budget) parks = create(:budget_heading, group: group_1) roads = create(:budget_heading, group: group_2) streets = create(:budget_heading, group: group_2) - [2, 4, 90, 100, 200, 300].each do |n| - create(:budget_investment, heading: parks, cached_votes_up: n, title: "Park with #{n} supports") - end - - [21, 31, 51, 81, 91, 101].each do |n| - create(:budget_investment, heading: roads, cached_votes_up: n, title: "Road with #{n} supports") - end - - [3, 10, 30, 33, 44, 55].each do |n| - create(:budget_investment, heading: streets, cached_votes_up: n, title: "Street with #{n} supports") - end + create(:budget_investment, heading: parks, cached_votes_up: 40, title: "Park with 40 supports") + create(:budget_investment, heading: parks, cached_votes_up: 99, title: "Park with 99 supports") + create(:budget_investment, heading: roads, cached_votes_up: 100, title: "Road with 100 supports") + create(:budget_investment, heading: roads, cached_votes_up: 199, title: "Road with 199 supports") + create(:budget_investment, heading: streets, cached_votes_up: 200, title: "Street with 200 supports") + create(:budget_investment, heading: streets, cached_votes_up: 300, title: "Street with 300 supports") visit admin_budget_budget_investments_path(budget) - [2, 4, 90, 100, 200, 300].each do |n| - expect(page).to have_link("Park with #{n} supports") - end - - [21, 31, 51, 81, 91, 101].each do |n| - expect(page).to have_link("Road with #{n} supports") - end - - [3, 10, 30, 33, 44, 55].each do |n| - expect(page).to have_link("Street with #{n} supports") - end + expect(page).to have_link("Park with 40 supports") + expect(page).to have_link("Park with 99 supports") + expect(page).to have_link("Road with 100 supports") + expect(page).to have_link("Road with 199 supports") + expect(page).to have_link("Street with 200 supports") + expect(page).to have_link("Street with 300 supports") click_link 'Advanced filters' - fill_in "max_per_heading", with: 5 + fill_in "min_total_supports", with: 180 click_button 'Filter' - expect(page).to have_content('There are 15 investments') - expect(page).not_to have_link("Park with 2 supports") - expect(page).not_to have_link("Road with 21 supports") - expect(page).not_to have_link("Street with 3 supports") - - [4, 90, 100, 200, 300].each do |n| - expect(page).to have_link("Park with #{n} supports") - end - - [31, 51, 81, 91, 101].each do |n| - expect(page).to have_link("Road with #{n} supports") - end - - [10, 30, 33, 44, 55].each do |n| - expect(page).to have_link("Street with #{n} supports") - end + expect(page).to have_content('There are 3 investments') + expect(page).to have_link("Road with 199 supports") + expect(page).to have_link("Street with 200 supports") + expect(page).to have_link("Street with 300 supports") + expect(page).not_to have_link("Park with 40 supports") + expect(page).not_to have_link("Park with 99 supports") + expect(page).not_to have_link("Road with 100 supports") end scenario "Combination of checkbox with text search", :js do