Merge pull request #4717 from consul/investment_filters_selector

Move investments filters to the sidebar

app/assets/javascripts/application.js

@@ -70,7 +70,6 @@
 //= require advanced_search
 //= require registration_form
 //= require suggest
-//= require filter_selector
 //= require forms
 //= require valuation_budget_investment_form
 //= require embed_video
@@ -135,7 +134,6 @@ var initialize_modules = function() {
   App.RegistrationForm.initialize();
   App.Suggest.initialize();
   App.Forms.initialize();
-  App.FilterSelector.initialize();
   App.ValuationBudgetInvestmentForm.initialize();
   App.EmbedVideo.initialize();
   App.FixedBar.initialize();

app/assets/javascripts/filter_selector.js

@@ -1,8 +0,0 @@
-(function() {
-  "use strict";
-  App.FilterSelector = {
-    initialize: function() {
-      App.Forms.submitOnChange(".filter-selector select");
-    }
-  };
-}).call(this);

app/assets/stylesheets/application.scss

@@ -27,7 +27,6 @@
 @import "milestones";
 @import "pages";
 @import "dashboard";
-@import "filter_selector";
 @import "legislation";
 @import "legislation_process";
 @import "legislation_process_form";

app/assets/stylesheets/budgets/investmentss/filters.scss

@@ -0,0 +1,19 @@
+.investments-filters {
+  border-top: 1px solid $border;
+
+  ul {
+    font-size: $small-font-size;
+    list-style: none;
+    margin-left: 0;
+
+    a {
+      display: inline-block;
+      padding-bottom: $line-height / 4;
+      padding-top: $line-height / 4;
+    }
+
+    [aria-current] {
+      font-weight: bold;
+    }
+  }
+}

app/assets/stylesheets/filter_selector.scss

@@ -1,14 +0,0 @@
-.filter-selector {
-  text-align: right;
-
-  label {
-    display: inline-block;
-    font-size: $small-font-size;
-    margin-right: $line-height / 2;
-    padding-top: $line-height / 2;
-  }
-
-  select {
-    width: auto;
-  }
-}

app/assets/stylesheets/layout.scss

@@ -977,13 +977,6 @@ footer {
   }
 }
 
-.sidebar-map {
-
-  .map {
-    z-index: 0;
-  }
-}
-
 .sidebar-title {
   border-top: 2px solid $brand;
   display: inline-block;

app/components/budgets/investments/filters_component.html.erb

@@ -0,0 +1,4 @@
+<div class="investments-filters">
+  <h2 class="sidebar-title" id="investments_filters_header"><%= t("budgets.investments.index.filter") %></h2>
+  <%= link_list(*filters, "aria-labelledby": "investments_filters_header") %>
+</div>

app/components/budgets/investments/filters_component.rb

@@ -0,0 +1,19 @@
+class Budgets::Investments::FiltersComponent < ApplicationComponent
+  delegate :valid_filters, :current_filter, :link_list, :current_path_with_query_params, to: :helpers
+
+  def render?
+    valid_filters&.any?
+  end
+
+  private
+
+    def filters
+      valid_filters.map do |filter|
+        [
+          t("budgets.investments.index.filters.#{filter}"),
+          current_path_with_query_params(filter: filter, page: 1),
+          current_filter == filter
+        ]
+      end
+    end
+end

app/components/shared/filter_selector_component.html.erb

@@ -1,5 +0,0 @@
-<%= form_tag({}, method: :get, enforce_utf8: false, class: "filter-selector") do %>
-  <%= query_parameters_tags %>
-  <%= label_tag "filter_selector_filter", t("#{i18n_namespace}.filter") %>
-  <%= select_tag "filter", options_for_select(filter_options, current_filter), id: "filter_selector_filter" %>
-<% end %>

app/components/shared/filter_selector_component.rb

@@ -1,22 +0,0 @@
-class Shared::FilterSelectorComponent < ApplicationComponent
-  delegate :valid_filters, :current_filter, to: :helpers
-  attr_reader :i18n_namespace
-
-  def initialize(i18n_namespace:)
-    @i18n_namespace = i18n_namespace
-  end
-
-  private
-
-    def query_parameters_tags
-      safe_join(request.query_parameters.reject do |name, _|
-        ["page", "filter"].include?(name)
-      end.map do |name, value|
-        hidden_field_tag name, value, id: "filter_selector_#{name}"
-      end)
-    end
-
-    def filter_options
-      valid_filters.map { |filter| [t("#{i18n_namespace}.filters.#{filter}"), filter] }
-    end
-end

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
 

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 ->(c) { c.instance_variable_get(:@budget).investments_filters }, only: [:index, :show, :suggest]
-    has_filters investment_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

app/controllers/concerns/investment_filters.rb

@@ -1,27 +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
-    [
-      "not_unfeasible",
-      "unfeasible",
-      ("unselected" if @budget.publishing_prices_or_later?),
-      ("selected" if @budget.publishing_prices_or_later?),
-      ("winners" if @budget.finished?)
-    ].compact
-  end
-end

app/controllers/concerns/search.rb

@@ -12,9 +12,6 @@ module Search
   end
 
   def parse_advanced_search_terms
-    if params[:advanced_search].is_a? String
-      params[:advanced_search] = JSON.parse(params[:advanced_search].gsub("=>", ":"))
-    end
     @advanced_search_terms = params[:advanced_search] if params[:advanced_search].present?
     parse_search_date
   end

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

app/models/budget.rb

@@ -196,6 +196,14 @@ class Budget < ApplicationRecord
     end
   end
 
+  def investments_filters
+    [
+      (%w[not_unfeasible feasible unfeasible] if valuating_or_later?),
+      (%w[selected unselected] if publishing_prices_or_later?),
+      ("winners" if finished?)
+    ].compact.flatten
+  end
+
   def email_selected
     investments.selected.order(:id).each do |investment|
       Mailer.budget_investment_selected(investment).deliver_later

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 %>
-
 <main class="budget-group-show">
   <header>
     <%= back_link_to budget_path(@budget) %>
     <h1><%= t("budgets.groups.show.title") %></h1>
   </header>
 
-  <% if @current_filter == "unfeasible" %>
-    <div class="row margin-top">
-      <div class="small-12 column">
-        <h3><%= t("budgets.groups.show.unfeasible_title") %></h3>
-      </div>
-    </div>
-  <% elsif @current_filter == "unselected" %>
-    <div class="row margin-top">
-      <div class="small-12 column">
-        <h3><%= t("budgets.groups.show.unselected_title") %></h3>
-      </div>
-    </div>
-  <% end %>
-
   <div class="row margin">
     <div id="headings" class="small-12 medium-7 column select-district">
       <div class="row">
@@ -30,9 +12,8 @@
             <% slice.each do |heading| %>
               <span id="<%= dom_id(heading) %>"
                     class="<%= css_for_ballot_heading(heading) %>">
-                <%= link_to heading.name,
+                <%= link_to heading.name, budget_investments_path(heading_id: heading.id) %>
-                            budget_investments_path(heading_id: heading.id,
+                <br>
-                                                    filter: @current_filter) %><br>
               </span>
             <% end %>
           </div>
@@ -44,28 +25,4 @@
       <%= image_tag(image_path_for("map.jpg")) %>
     </div>
   </div>
-
-  <% if @budget.balloting_or_later? %>
-    <% unless @current_filter == "unfeasible" %>
-      <div class="row">
-        <div class="small-12 column">
-          <small>
-            <%= link_to t("budgets.groups.show.unfeasible"),
-                        budget_group_path(@budget, @group, filter: "unfeasible") %>
-          </small>
-        </div>
-      </div>
-    <% end %>
-
-    <% unless @current_filter == "unselected" %>
-      <div class="row">
-        <div class="small-12 column">
-          <small>
-            <%= link_to t("budgets.groups.show.unselected"),
-                        budget_group_path(@budget, @group, filter: "unselected") %>
-          </small>
-        </div>
-      </div>
-    <% end %>
-  <% end %>
 </main>

app/views/budgets/investments/_map.html.erb

@@ -1,5 +1,3 @@
-<ul class="no-bullet sidebar-map">
+<div class="map">
-  <div class="map">
+  <%= render_map(@map_location, "budgets", false, nil, @investments_map_coordinates) %>
-    <%= render_map(@map_location, "budgets", false, nil, @investments_map_coordinates) %>
+</div>
-  </div>
-</ul>

app/views/budgets/investments/_sidebar.html.erb

@@ -59,9 +59,12 @@
   </div>
 <% end %>
 
+<%= render Budgets::Investments::FiltersComponent.new %>
+
 <% if @heading && !@heading.content_blocks.where(locale: I18n.locale).empty? %>
   <%= render "budgets/investments/content_blocks" %>
 <% end %>
+
 <% if @map_location&.available? %>
   <%= render "budgets/investments/map" %>
 <% end %>

app/views/budgets/investments/index.html.erb

@@ -70,8 +70,6 @@
         <%= render("shared/order_links", i18n_namespace: "budgets.investments.index") %>
       <% end %>
 
-      <%= render Shared::FilterSelectorComponent.new(i18n_namespace: "budgets.investments.index") %>
-
       <% if investments_default_view? %>
 
         <% @investments.each do |investment| %>

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
@@ -121,7 +117,8 @@ en:
           not_logged_in: "To create a new budget investment you must %{sign_in} or %{sign_up}."
         filter: "Filtering projects by"
         filters:
-          not_unfeasible: "Not unfeasible"
+          feasible: "Feasible"
+          not_unfeasible: "Feasible or with undecided feasibility"
           selected: "Selected"
           unfeasible: "Unfeasible"
           unselected: "Unselected"

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
@@ -121,7 +117,8 @@ es:
           not_logged_in: "Para crear un nuevo proyecto de gasto debes %{sign_in} o %{sign_up}."
         filter: "Filtrando proyectos"
         filters:
-          not_unfeasible: "No inviables"
+          feasible: "Viables"
+          not_unfeasible: "Viables o con viabilidad por decidir"
           selected: "Seleccionados"
           unfeasible: "Inviables"
           unselected: "No seleccionados"

spec/components/budgets/investments/filters_component_spec.rb

@@ -0,0 +1,33 @@
+require "rails_helper"
+
+describe Budgets::Investments::FiltersComponent do
+  let(:budget) { create(:budget) }
+
+  around do |example|
+    with_request_url(Rails.application.routes.url_helpers.budget_investments_path(budget)) do
+      example.run
+    end
+  end
+
+  it "is not displayed before valuation" do
+    %w[informing accepting reviewing selecting].each do |phase|
+      budget.update!(phase: phase)
+      allow(controller).to receive(:valid_filters).and_return(budget.investments_filters)
+
+      render_inline Budgets::Investments::FiltersComponent.new
+
+      expect(page.native.inner_html).to be_empty
+    end
+  end
+
+  it "is displayed during and after valuation" do
+    Budget::Phase::kind_or_later("valuating").each do |phase|
+      budget.update!(phase: phase)
+      allow(controller).to receive(:valid_filters).and_return(budget.investments_filters)
+
+      render_inline Budgets::Investments::FiltersComponent.new
+
+      expect(page).to have_content "Filtering projects by"
+    end
+  end
+end

spec/components/shared/filter_selector_component_spec.rb

@@ -1,14 +0,0 @@
-require "rails_helper"
-
-describe Shared::FilterSelectorComponent do
-  it "renders a form with a select" do
-    component = Shared::FilterSelectorComponent.new(i18n_namespace: "budgets.investments.index")
-    allow(component).to receive(:valid_filters).and_return(["unfeasible", "winners"])
-    allow(component).to receive(:current_filter).and_return(nil)
-
-    with_request_url("/") { render_inline component }
-
-    expect(page).to have_select "Filtering projects by"
-    expect(page).to have_selector "form[method='get'].filter-selector select"
-  end
-end

spec/models/budget_spec.rb

@@ -285,6 +285,36 @@ describe Budget do
     end
   end
 
+  describe "#investments_filters" do
+    it "returns no filters before valuating" do
+      %w[informing accepting reviewing selecting].each do |phase|
+        budget.phase = phase
+
+        expect(budget.investments_filters).to be_empty
+      end
+    end
+
+    it "returns feasibility filters during valuation" do
+      budget.phase = "valuating"
+
+      expect(budget.investments_filters).to eq(%w[not_unfeasible feasible unfeasible])
+    end
+
+    it "returns feasibility and selection filters during the final voting phases" do
+      %w[publishing_prices balloting reviewing_ballots].each do |phase|
+        budget.phase = phase
+
+        expect(budget.investments_filters).to eq(%w[not_unfeasible feasible unfeasible selected unselected])
+      end
+    end
+
+    it "returns all filters after the budget has finished" do
+      budget.phase = "finished"
+
+      expect(budget.investments_filters).to eq(%w[not_unfeasible feasible unfeasible selected unselected winners])
+    end
+  end
+
   describe "#has_winning_investments?" do
     it "returns true if there is a winner investment" do
       budget.investments << build(:budget_investment, :winner, price: 3, ballot_lines_count: 2)

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")

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)
 

spec/system/budgets/investments_spec.rb

@@ -47,6 +47,8 @@ describe "Budget Investments" do
   end
 
   scenario "Index" do
+    budget.update!(phase: "valuating")
+
     investments = [create(:budget_investment, heading: heading),
                    create(:budget_investment, heading: heading),
                    create(:budget_investment, :feasible, heading: heading)]
@@ -131,6 +133,7 @@ describe "Budget Investments" do
   scenario "Index filter by status" do
     budget.update!(phase: "finished")
 
+    create(:budget_investment, heading: heading, title: "Unclassified investment")
     create(:budget_investment, :feasible, heading: heading, title: "Feasible investment")
     create(:budget_investment, :unfeasible, heading: heading, title: "Unfeasible investment")
     create(:budget_investment, :unselected, heading: heading, title: "Unselected investment")
@@ -139,37 +142,43 @@ describe "Budget Investments" do
 
     visit budget_investments_path(budget, heading_id: heading.id)
 
-    expect(page).to have_select "Filtering projects by",
+    expect(page).to have_content "FILTERING PROJECTS BY"
-                                options: ["Not unfeasible", "Unfeasible", "Unselected", "Selected", "Winners"]
 
-    select "Unfeasible", from: "Filtering projects by"
+    click_link "Feasible"
 
     expect(page).to have_css ".budget-investment", count: 1
+    expect(page).to have_content "Feasible investment"
+
+    click_link "Unfeasible"
+
     expect(page).to have_content "Unfeasible investment"
+    expect(page).to have_css ".budget-investment", count: 1
 
-    select "Unselected", from: "Filtering projects by"
+    click_link "Unselected"
 
-    expect(page).to have_css ".budget-investment", count: 2
+    expect(page).to have_css ".budget-investment", count: 3
     expect(page).to have_content "Unselected investment"
+    expect(page).to have_content "Unclassified investment"
     expect(page).to have_content "Feasible investment"
 
-    select "Selected", from: "Filtering projects by"
+    click_link "Selected"
 
     expect(page).to have_css ".budget-investment", count: 2
     expect(page).to have_content "Selected investment"
     expect(page).to have_content "Winner investment"
 
-    select "Winners", from: "Filtering projects by"
+    click_link "Winners"
 
     expect(page).to have_css ".budget-investment", count: 1
     expect(page).to have_content "Winner investment"
 
-    select "Not unfeasible", from: "Filtering projects by"
+    click_link "Feasible or with undecided feasibility"
 
-    expect(page).to have_css ".budget-investment", count: 4
+    expect(page).to have_css ".budget-investment", count: 5
     expect(page).to have_content "Selected investment"
     expect(page).to have_content "Unselected investment"
     expect(page).to have_content "Feasible investment"
+    expect(page).to have_content "Unclassified investment"
     expect(page).to have_content "Winner investment"
   end
 
@@ -196,6 +205,8 @@ describe "Budget Investments" do
     end
 
     scenario "Advanced search combined with filter by status" do
+      budget.update!(phase: "valuating")
+
       create(:budget_investment, :feasible, heading: heading, title: "Feasible environment")
       create(:budget_investment, :feasible, heading: heading, title: "Feasible health")
       create(:budget_investment, :unfeasible, heading: heading, title: "Unfeasible environment")
@@ -203,13 +214,10 @@ describe "Budget Investments" do
 
       visit budget_investments_path(budget, heading: heading)
 
-      click_on "Advanced search"
+      click_button "Advanced search"
-
+      fill_in "With the text", with: "environment"
-      within(".advanced-search-form") do
+      select "Last 24 hours", from: "By date"
-        fill_in "With the text", with: "environment"
+      click_button "Filter"
-        select "Last 24 hours", from: "By date"
-        click_button "Filter"
-      end
 
       expect(page).to have_content "There is 1 investment containing the term 'environment'"
       expect(page).to have_css ".budget-investment", count: 1
@@ -218,7 +226,7 @@ describe "Budget Investments" do
       expect(page).not_to have_content "Unfeasible environment"
       expect(page).not_to have_content "Unfeasible health"
 
-      select "Unfeasible", from: "Filtering projects by"
+      click_link "Unfeasible"
 
       expect(page).not_to have_content "Feasible environment"
       expect(page).to have_content "There is 1 investment containing the term 'environment'"
@@ -231,6 +239,8 @@ describe "Budget Investments" do
 
   context("Filters") do
     scenario "by unfeasibility" do
+      budget.update!(phase: "valuating")
+
       investment1 = create(:budget_investment, :unfeasible, :finished, heading: heading)
       investment2 = create(:budget_investment, :feasible, heading: heading)
       investment3 = create(:budget_investment, heading: heading)
@@ -514,7 +524,7 @@ describe "Budget Investments" do
     end
 
     scenario "Order always is random for unfeasible and unselected investments" do
-      Budget::Phase::PHASE_KINDS.each do |phase|
+      Budget::Phase::kind_or_later("valuating").each do |phase|
         budget.update!(phase: phase)
 
         visit budget_investments_path(budget, heading_id: heading.id, filter: "unfeasible")
@@ -1448,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)
 
@@ -1466,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