From 756a16f67a700996c562a9687df1fe6009dd7393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 16 Sep 2021 01:47:00 +0200 Subject: [PATCH] Remove investment filters in groups The interface was a bit confusing, since after clicking on "See unfeasible investments" (or similar), we were on a page where no investments were shown. Besides, since commit 7e3dd47d5, the group page is only linked from the "my ballot" page, through a link inviting the user to vote in that group, and it's only possible to vote selected investments (which is the default filter during the final voting phase). The only reason we had these links here was these links weren't present in the investments page. But they're present there since commit 04605d5d5, so we don't need them in the group page anymore. --- app/controllers/budgets/groups_controller.rb | 4 -- .../budgets/investments_controller.rb | 12 +++-- .../concerns/investment_filters.rb | 21 --------- app/helpers/budgets_helper.rb | 4 +- app/views/budgets/groups/show.html.erb | 47 +------------------ config/locales/en/budgets.yml | 4 -- config/locales/es/budgets.yml | 4 -- spec/system/budgets/budgets_spec.rb | 36 -------------- spec/system/budgets/groups_spec.rb | 19 -------- spec/system/budgets/investments_spec.rb | 10 +--- 10 files changed, 13 insertions(+), 148 deletions(-) delete mode 100644 app/controllers/concerns/investment_filters.rb diff --git a/app/controllers/budgets/groups_controller.rb b/app/controllers/budgets/groups_controller.rb index 5fc81d0fb..542be0fb6 100644 --- a/app/controllers/budgets/groups_controller.rb +++ b/app/controllers/budgets/groups_controller.rb @@ -1,6 +1,5 @@ module Budgets class GroupsController < ApplicationController - include InvestmentFilters include FeatureFlags feature_flag :budgets @@ -9,9 +8,6 @@ module Budgets authorize_resource :budget authorize_resource :group, class: "Budget::Group" - before_action :set_default_investment_filter, only: :show - has_filters investment_filters, only: [:show] - def show end diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index 43916bbb4..e3d82cb6f 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -8,7 +8,6 @@ module Budgets include DocumentAttributes include MapLocationAttributes include Translatable - include InvestmentFilters PER_PAGE = 10 @@ -33,8 +32,7 @@ module Budgets has_orders %w[most_voted newest oldest], only: :show has_orders ->(c) { c.instance_variable_get(:@budget).investments_orders }, only: :index - - has_filters investment_filters, only: [:index, :show, :suggest] + has_filters ->(c) { c.instance_variable_get(:@budget).investments_filters }, only: [:index, :show, :suggest] invisible_captcha only: [:create, :update], honeypot: :subtitle, scope: :budget_investment @@ -170,6 +168,14 @@ module Budgets end end + def set_default_investment_filter + if @budget&.finished? + params[:filter] ||= "winners" + elsif @budget&.publishing_prices_or_later? + params[:filter] ||= "selected" + end + end + def load_map @map_location = MapLocation.load_from_heading(@heading) end diff --git a/app/controllers/concerns/investment_filters.rb b/app/controllers/concerns/investment_filters.rb deleted file mode 100644 index 159ac575f..000000000 --- a/app/controllers/concerns/investment_filters.rb +++ /dev/null @@ -1,21 +0,0 @@ -module InvestmentFilters - extend ActiveSupport::Concern - - class_methods do - def investment_filters - ->(controller) { controller.investment_filters } - end - end - - def set_default_investment_filter - if @budget&.finished? - params[:filter] ||= "winners" - elsif @budget&.publishing_prices_or_later? - params[:filter] ||= "selected" - end - end - - def investment_filters - @budget.investments_filters - end -end diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index c472a0f35..0352570ed 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -16,9 +16,7 @@ module BudgetsHelper end def css_for_ballot_heading(heading) - return "" if current_ballot.blank? || @current_filter == "unfeasible" - - current_ballot.has_lines_in_heading?(heading) ? "is-active" : "" + current_ballot&.has_lines_in_heading?(heading) ? "is-active" : "" end def current_ballot diff --git a/app/views/budgets/groups/show.html.erb b/app/views/budgets/groups/show.html.erb index 75b082dd4..6997f6485 100644 --- a/app/views/budgets/groups/show.html.erb +++ b/app/views/budgets/groups/show.html.erb @@ -1,27 +1,9 @@ -<% content_for :canonical do %> - <%= render "shared/canonical", href: budget_group_url(filter: @current_filter) %> -<% end %> -
<%= back_link_to budget_path(@budget) %>

<%= t("budgets.groups.show.title") %>

- <% if @current_filter == "unfeasible" %> -
-
-

<%= t("budgets.groups.show.unfeasible_title") %>

-
-
- <% elsif @current_filter == "unselected" %> -
-
-

<%= t("budgets.groups.show.unselected_title") %>

-
-
- <% end %> -
@@ -30,9 +12,8 @@ <% slice.each do |heading| %> - <%= link_to heading.name, - budget_investments_path(heading_id: heading.id, - filter: @current_filter) %>
+ <%= link_to heading.name, budget_investments_path(heading_id: heading.id) %> +
<% end %>
@@ -44,28 +25,4 @@ <%= image_tag(image_path_for("map.jpg")) %>
- - <% if @budget.balloting_or_later? %> - <% unless @current_filter == "unfeasible" %> -
-
- - <%= link_to t("budgets.groups.show.unfeasible"), - budget_group_path(@budget, @group, filter: "unfeasible") %> - -
-
- <% end %> - - <% unless @current_filter == "unselected" %> -
-
- - <%= link_to t("budgets.groups.show.unselected"), - budget_group_path(@budget, @group, filter: "unselected") %> - -
-
- <% end %> - <% end %>
diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index c438b581d..d7f919f5c 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -43,10 +43,6 @@ en: groups: show: title: Select a heading - unfeasible_title: Unfeasible investments - unfeasible: See unfeasible investments - unselected_title: Investments not selected for balloting phase - unselected: See investments not selected for balloting phase phase: drafting: Draft (Not visible to the public) informing: Information diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 51513a371..322beb317 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -43,10 +43,6 @@ es: groups: show: title: Selecciona una partida - unfeasible_title: Proyectos de gasto inviables - unfeasible: Ver proyectos inviables - unselected_title: Proyectos no seleccionados para la votación final - unselected: Ver los proyectos no seleccionados para la votación final phase: drafting: Borrador (No visible para el público) informing: Información diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index 01ae866a8..bb6474b8d 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -328,42 +328,6 @@ describe "Budgets" do let!(:budget) { create(:budget, :selecting) } let!(:group) { create(:budget_group, budget: budget) } - describe "Links to unfeasible and selected" do - scenario "are not seen before balloting" do - visit budget_group_path(budget, group) - - expect(page).not_to have_link "See unfeasible investments" - expect(page).not_to have_link "See investments not selected for balloting phase" - end - - scenario "are not seen publishing prices" do - budget.update!(phase: :publishing_prices) - - visit budget_group_path(budget, group) - - expect(page).not_to have_link "See unfeasible investments" - expect(page).not_to have_link "See investments not selected for balloting phase" - end - - scenario "are seen balloting" do - budget.update!(phase: :balloting) - - visit budget_group_path(budget, group) - - expect(page).to have_link "See unfeasible investments" - expect(page).to have_link "See investments not selected for balloting phase" - end - - scenario "are seen on finished budgets" do - budget.update!(phase: :finished) - - visit budget_group_path(budget, group) - - expect(page).to have_link "See unfeasible investments" - expect(page).to have_link "See investments not selected for balloting phase" - end - end - scenario "Take into account headings with the same name from a different budget" do group1 = create(:budget_group, budget: budget, name: "New York") heading1 = create(:budget_heading, group: group1, name: "Brooklyn") diff --git a/spec/system/budgets/groups_spec.rb b/spec/system/budgets/groups_spec.rb index 99488b4f8..5bc07eb38 100644 --- a/spec/system/budgets/groups_spec.rb +++ b/spec/system/budgets/groups_spec.rb @@ -26,25 +26,6 @@ describe "Budget Groups" do expect(first_heading.name).to appear_before(last_heading.name) end - scenario "Links to investment filters" do - create(:budget_heading, group: group, name: "Southwest") - budget.update!(phase: "finished") - - visit budget_group_path(budget, group) - - click_link "See unfeasible investments" - - expect(page).to have_css "h3", exact_text: "Unfeasible investments" - expect(page).to have_link "Southwest" - expect(page).not_to have_link "See unfeasible investments" - - click_link "See investments not selected for balloting phase" - - expect(page).to have_css "h3", exact_text: "Investments not selected for balloting phase" - expect(page).to have_link "Southwest" - expect(page).not_to have_link "See investments not selected for balloting phase unfeasible investments" - end - scenario "Back link" do visit budget_group_path(budget, group) diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 95cefbe1a..002d17dc2 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1458,7 +1458,7 @@ describe "Budget Investments" do end end - scenario "Highlight voted heading except with unfeasible filter" do + scenario "Highlight voted heading" do budget.update!(phase: "balloting") user = create(:user, :level_two) @@ -1476,14 +1476,6 @@ describe "Budget Investments" do expect(page).to have_css("#budget_heading_#{heading_1.id}.is-active") expect(page).to have_css("#budget_heading_#{heading_2.id}") - - click_link "See unfeasible investments" - - within("#headings") do - expect(page).to have_css("#budget_heading_#{heading_1.id}") - expect(page).to have_css("#budget_heading_#{heading_2.id}") - expect(page).not_to have_css(".is-active") - end end scenario "Ballot is visible" do