Merge pull request #3148 from matisnape/budget_investments_sorting_columns

Add sort links to admin tables

app/assets/stylesheets/admin.scss

@@ -224,6 +224,11 @@ $sidebar-active: #f4fcd0;
       label {
         color: #fff;
       }
+
+      a {
+        color: inherit;
+        white-space: nowrap;
+      }
     }
 
     .break {

app/controllers/admin/budget_investments_controller.rb

@@ -77,7 +77,8 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
 
     def load_investments
       @investments = Budget::Investment.scoped_filter(params, @current_filter)
-                                       .order_filter(params[:sort_by])
+                                       .order_filter(params)
+
       @investments = @investments.page(params[:page]) unless request.format.csv?
     end
 
@@ -133,5 +134,4 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
         end
       end
     end
-
 end

app/helpers/budget_investments_helper.rb

@@ -1,12 +1,34 @@
 module BudgetInvestmentsHelper
-  def budget_investments_sorting_options
+  def budget_investments_advanced_filters(params)
-    Budget::Investment::SORTING_OPTIONS.map do |so|
+    params.map { |af| t("admin.budget_investments.index.filters.#{af}") }.join(', ')
-      [t("admin.budget_investments.index.sort_by.#{so}"), so]
+  end
+
+  def link_to_investments_sorted_by(column)
+    direction = set_direction(params[:direction])
+    icon = set_sorting_icon(direction, column)
+
+    translation = t("admin.budget_investments.index.list.#{column}")
+
+    link_to(
+      "#{translation} <span class='#{icon}'></span>".html_safe,
+      admin_budget_budget_investments_path(sort_by: column, direction: direction)
+    )
+  end
+
+  def set_sorting_icon(direction, sort_by)
+    if sort_by.to_s == params[:sort_by]
+      if direction == "desc"
+        "icon-arrow-top"
+      else
+        "icon-arrow-down"
+      end
+    else
+      ""
     end
   end
 
-  def budget_investments_advanced_filters(params)
+  def set_direction(current_direction)
-    params.map { |af| t("admin.budget_investments.index.filters.#{af}") }.join(', ')
+    current_direction == "desc" ? "asc" : "desc"
   end
 
   def investments_minimal_view_path

app/models/budget/investment.rb

@@ -1,6 +1,6 @@
 class Budget
   class Investment < ActiveRecord::Base
-    SORTING_OPTIONS = %w(id title supports).freeze
+    SORTING_OPTIONS = {id: "id", title: "title", supports: "cached_votes_up"}.freeze
 
     include Rails.application.routes.url_helpers
     include Measurable
@@ -139,9 +139,13 @@ class Budget
       results.where("budget_investments.id IN (?)", ids)
     end
 
-    def self.order_filter(sorting_param)
+    def self.order_filter(params)
-      if sorting_param.present? && SORTING_OPTIONS.include?(sorting_param)
+      sorting_key = params[:sort_by]&.downcase&.to_sym
-        send("sort_by_#{sorting_param}")
+      allowed_sort_option = SORTING_OPTIONS[sorting_key]
+
+      if allowed_sort_option.present?
+        direction = params[:direction] == "desc" ? "desc" : "asc"
+        order("#{allowed_sort_option} #{direction}")
       else
         order(cached_votes_up: :desc).order(id: :desc)
       end

app/views/admin/budget_investments/_investments.html.erb

@@ -1,12 +1,3 @@
-<%= form_tag(admin_budget_budget_investments_path(budget: @budget), method: :get) do %>
-  <div class="small-12 medium-3 column">
-    <%= select_tag :sort_by, options_for_select(budget_investments_sorting_options, params[:sort_by]),
-                   { prompt: t("admin.budget_investments.index.sort_by.placeholder"),
-                     label: false,
-                     class: "js-submit-on-change" } %>
-  </div>
-<% end %>
-
 <%= link_to t("admin.budget_investments.index.download_current_selection"),
             admin_budget_budget_investments_path(csv_params),
             class: "float-right small clear" %>
@@ -35,9 +26,9 @@
   <table class="table-for-mobile">
     <thead>
       <tr>
-        <th><%= t("admin.budget_investments.index.list.id") %></th>
+        <th><%= link_to_investments_sorted_by :id  %></th>
-        <th class="small-3"><%= t("admin.budget_investments.index.list.title") %></th>
+        <th class="small-3"><%= link_to_investments_sorted_by :title %></th>
-        <th><%= t("admin.budget_investments.index.list.supports") %></th>
+        <th><%= link_to_investments_sorted_by :supports %></th>
         <th><%= t("admin.budget_investments.index.list.admin") %></th>
         <th>
           <%= t("admin.budget_investments.index.list.valuation_group") %>

config/locales/en/admin.yml

@@ -182,11 +182,6 @@ en:
         tags_filter_all: All tags
         advanced_filters: Advanced filters
         placeholder: Search projects
-        sort_by:
-          placeholder: Sort by
-          id: ID
-          title: Title
-          supports: Supports
         filters:
           all: All
           without_admin: Without assigned admin

config/locales/es/admin.yml

@@ -182,11 +182,6 @@ es:
         tags_filter_all: Todas las etiquetas
         advanced_filters: Filtros avanzados
         placeholder: Buscar proyectos
-        sort_by:
-          placeholder: Ordenar por
-          id: ID
-          title: Título
-          supports: Apoyos
         filters:
           all: Todos
           without_admin: Sin administrador

spec/features/admin/budget_investments_spec.rb

@@ -613,7 +613,7 @@ feature 'Admin budget investments' do
       create(:budget_investment, title: 'C Third Investment', cached_votes_up: 10, budget: budget)
     end
 
-    scenario "Default sorting" do
+    scenario "Default" do
       create(:budget_investment, title: 'D Fourth Investment', cached_votes_up: 50, budget: budget)
 
       visit admin_budget_budget_investments_path(budget)
@@ -623,11 +623,79 @@ feature 'Admin budget investments' do
       expect('A Second Investment').to appear_before('C Third Investment')
     end
 
+    context 'Ascending' do
       scenario 'Sort by ID' do
-      visit admin_budget_budget_investments_path(budget, sort_by: 'id')
+        visit admin_budget_budget_investments_path(budget, sort_by: 'id', direction: 'asc')
+
+        expect('B First Investment').to appear_before('A Second Investment')
+        expect('A Second Investment').to appear_before('C Third Investment')
+        within('th', text: 'ID') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
+      end
+
+      scenario 'Sort by title' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'title', direction: 'asc')
+
+        expect('A Second Investment').to appear_before('B First Investment')
+        expect('B First Investment').to appear_before('C Third Investment')
+        within('th', text: 'Title') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
+      end
+
+      scenario 'Sort by supports' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'supports', direction: 'asc')
 
         expect('C Third Investment').to appear_before('A Second Investment')
         expect('A Second Investment').to appear_before('B First Investment')
+        within('th', text: 'Supports') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
+      end
+    end
+
+    context 'Descending' do
+      scenario 'Sort by ID' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'id', direction: 'desc')
+
+        expect('C Third Investment').to appear_before('A Second Investment')
+        expect('A Second Investment').to appear_before('B First Investment')
+        within('th', text: 'ID') do
+          expect(page).to have_css(".icon-arrow-down")
+        end
+      end
+
+      scenario 'Sort by title' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'title', direction: 'desc')
+
+        expect('C Third Investment').to appear_before('B First Investment')
+        expect('B First Investment').to appear_before('A Second Investment')
+        within('th', text: 'Title') do
+          expect(page).to have_css(".icon-arrow-down")
+        end
+      end
+
+      scenario 'Sort by supports' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'supports', direction: 'desc')
+
+        expect('B First Investment').to appear_before('A Second Investment')
+        expect('A Second Investment').to appear_before('C Third Investment')
+        within('th', text: 'Supports') do
+          expect(page).to have_css(".icon-arrow-down")
+        end
+      end
+    end
+
+    context 'With no direction provided sorts ascending' do
+      scenario 'Sort by ID' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'id')
+
+        expect('B First Investment').to appear_before('A Second Investment')
+        expect('A Second Investment').to appear_before('C Third Investment')
+        within('th', text: 'ID') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
       end
 
       scenario 'Sort by title' do
@@ -635,13 +703,32 @@ feature 'Admin budget investments' do
 
         expect('A Second Investment').to appear_before('B First Investment')
         expect('B First Investment').to appear_before('C Third Investment')
+        within('th', text: 'Title') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
       end
 
       scenario 'Sort by supports' do
         visit admin_budget_budget_investments_path(budget, sort_by: 'supports')
 
+        expect('C Third Investment').to appear_before('A Second Investment')
+        expect('A Second Investment').to appear_before('B First Investment')
+        within('th', text: 'Supports') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
+      end
+    end
+
+    context 'With incorrect direction provided sorts ascending' do
+      scenario 'Sort by ID' do
+        visit admin_budget_budget_investments_path(budget, sort_by: 'id', direction: 'incorrect')
+
         expect('B First Investment').to appear_before('A Second Investment')
         expect('A Second Investment').to appear_before('C Third Investment')
+        within('th', text: 'ID') do
+          expect(page).to have_css(".icon-arrow-top")
+        end
+      end
     end
   end
 

spec/helpers/budget_investments_helper_spec.rb

@@ -0,0 +1,47 @@
+require 'rails_helper'
+
+RSpec.describe BudgetInvestmentsHelper, type: :helper do
+
+  describe "#set_direction" do
+
+    it "returns ASC if current_direction is DESC" do
+      expect(set_direction("desc")).to eq "asc"
+    end
+
+    it "returns DESC if current_direction is ASC" do
+      expect(set_direction("asc")).to eq "desc"
+    end
+
+    it "returns DESC if current_direction is nil" do
+      expect(set_direction(nil)).to eq "desc"
+    end
+  end
+
+  describe "#set_sorting_icon" do
+    let(:sort_by) { "title" }
+    let(:params)  { { sort_by: sort_by } }
+
+    it "returns arrow down if current direction is ASC" do
+      expect(set_sorting_icon("asc", sort_by)).to eq "icon-arrow-down"
+    end
+
+    it "returns arrow top if current direction is DESC" do
+      expect(set_sorting_icon("desc", sort_by)).to eq "icon-arrow-top"
+    end
+
+    it "returns arrow down if sort_by present, but no direction" do
+      expect(set_sorting_icon(nil, sort_by)).to eq "icon-arrow-down"
+    end
+
+    it "returns no icon if sort_by and direction is missing" do
+      params[:sort_by] = nil
+      expect(set_sorting_icon(nil, sort_by)).to eq ""
+    end
+
+    it "returns no icon if sort_by is incorrect" do
+      params[:sort_by] = "random"
+      expect(set_sorting_icon("asc", sort_by)).to eq ""
+    end
+  end
+
+end