Improve navigation and filters in proposals (decidim#13004)

* Store filters selections in session and allow removing all filters

* Implement next and prev links from proposal show

* Inspect session

* Use with_indifferent_access with session admin_filters

* Revert "Inspect session"

This reverts commit 495a33b.

* Refactor adjacents navigation links

* Use spans and adjust margins

* Fix wrong spelling

* Fix spelling

* Ensure the session filtered collection query returns an id

* style filters bar

* better space handling for containers

* Add tests

* Change remove all filters text

* Remove font-normal class remove all filters tag

* Fix expected text in tests

* Add comment to adjacent_items method

* Move method call to before_action filter

* Define index method explicitly in controller

* Change translation

Co-authored-by: Andrés Pereira de Lucena <[email protected]>

* Remove unnecessary method call

Co-authored-by: Andrés Pereira de Lucena <[email protected]>

---------

Co-authored-by: Hugoren Martinako <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
(cherry picked from commit 49bd028)

decidim-admin/app/controllers/concerns/decidim/admin/filterable.rb

@@ -20,12 +20,15 @@ module Filterable
                       :filters,
                       :filters_with_values,
                       :find_dynamic_translation,
+                      :filter_prefix_key,
                       :query,
                       :query_params,
                       :query_params_with,
                       :query_params_without,
+                      :blank_query_params,
                       :ransack_params,
-                      :search_field_predicate
+                      :search_field_predicate,
+                      :adjacent_items
 
         delegate :categories, to: :current_component
         delegate :scopes, to: :current_organization
@@ -36,10 +39,70 @@ def query
 
         private
 
+        def check_admin_session_filters
+          if (current_filters = ransack_params).present?
+            admin_session_filters = session["admin_filters"] || {}
+            return if admin_session_filters[filter_prefix_key] == current_filters
+
+            current_filters = {} if current_filters[:reset_filters] == "true"
+
+            admin_session_filters[filter_prefix_key] = current_filters
+            session["admin_filters"] = admin_session_filters
+
+            redirect_to url_for(query_params.merge(q: {})) if current_filters.blank?
+          else
+            @session_filter_params = {} unless session_filter_params.is_a?(Hash)
+            redirect_to url_for(query_params_with(session_filter_params)) if session_filter_params.present?
+          end
+        end
+
         def filtered_collection
           paginate(query.result)
         end
 
+        def session_filtered_collection
+          @session_filtered_collection ||= begin
+            query = base_query.ransack(session_filter_params, search_context: :admin, auth_object: current_user).result
+            # The limit reorders as pagination does
+            query.limit(query.count)
+          end
+        end
+
+        # This method takes the query used by filter and selects the id of
+        # each item of the filtered collection (this extra select id avoids
+        # some errors where the SQL of the filtered collection query uses
+        # aliases and the id is not available in the result) and uses the lag
+        # and lead window functions which returns the previous and next ids in
+        # the query
+        def adjacent_items(item)
+          query =
+            <<-SQL.squish
+              WITH
+                collection AS (#{session_filtered_collection.select(:id).to_sql}),
+                successors AS (
+                  SELECT
+                    id,
+                    Lag(id, 1) OVER () prev_item,
+                    Lead(id, 1) OVER () next_item
+                  FROM
+                    collection
+                )
+              SELECT
+                prev_item,
+                next_item
+              FROM
+                successors
+              WHERE
+                successors.id = #{item.id}
+            SQL
+
+          (ActiveRecord::Base.connection.exec_query(query).first || {}).compact_blank.transform_values { |id| collection.find_by(id:) }
+        end
+
+        def filter_prefix_key
+          @filter_prefix_key ||= controller_name.to_sym
+        end
+
         def base_query
           raise NotImplementedError, "A base query is needed to filter admin resources"
         end
@@ -63,14 +126,26 @@ def ransack_params
           query_params[:q] || {}
         end
 
+        def session_filter_params
+          @session_filter_params ||= (session["admin_filters"] || {}).with_indifferent_access.fetch(filter_prefix_key, {})
+        end
+
         # For injecting ransack params while keeping query params in links.
         def query_params_with(hash)
           query_params.merge(q: ransack_params.merge(hash))
         end
 
         # For rejecting ransack params while keeping query params in links.
-        def query_params_without(*filters)
-          query_params.merge(q: ransack_params.except(*filters))
+        def query_params_without(*)
+          q = ransack_params.except(*)
+
+          return blank_query_params if q.blank?
+
+          query_params.merge(q:)
+        end
+
+        def blank_query_params
+          query_params.merge(q: { reset_filters: true })
         end
 
         # Ransack predicate to use in the search_form_for.

decidim-admin/app/helpers/decidim/admin/filterable_helper.rb

@@ -98,9 +98,14 @@ def applied_filters_hidden_field_tags
       end
 
       def applied_filters_tags(i18n_ctx)
-        ransack_params.slice(*filters).map do |filter, value|
+        tags = ransack_params.slice(*filters).map do |filter, value|
           applied_filter_tag(filter, value, filterable_i18n_scope_from_ctx(i18n_ctx))
-        end.join.html_safe
+        end
+        return if tags.blank?
+
+        tags << remove_all_filters_tag if tags.count > 1
+
+        tags.join.html_safe
       end
 
       def applied_filter_tag(filter, value, i18n_scope)
@@ -111,6 +116,13 @@ def applied_filter_tag(filter, value, i18n_scope)
         end
       end
 
+      def remove_all_filters_tag
+        link_to(url_for(blank_query_params), class: "label bg-transparent") do
+          concat t("decidim.admin.filters.remove_all")
+          concat icon("delete-bin-line", aria_label: t("decidim.admin.filters.remove_all"), role: "img")
+        end
+      end
+
       def remove_filter_icon_link(filter)
         icon_link_to(
           "delete-bin-line",
@@ -125,6 +137,10 @@ def filterable_i18n_scope_from_ctx(i18n_ctx)
         i18n_scope += ".#{i18n_ctx}" if i18n_ctx
         i18n_scope
       end
+
+      def filtered_adjacent_paths(item, path_method)
+        adjacent_items(item).transform_values(&method(path_method))
+      end
     end
   end
 end

decidim-admin/app/packs/stylesheets/decidim/admin/_cards.scss

@@ -107,7 +107,7 @@
 }
 
 .fcell .label {
-  @apply p-2 mb-2;
+  @apply p-2;
 }
 
 .card-section-draggable-list {

decidim-admin/app/views/decidim/admin/shared/_adjacent_navigation.html.erb

@@ -0,0 +1,30 @@
+<% adjacent_paths ||= {} %>
+<div class="flex justify-between mb-4">
+  <span>
+    <% if adjacent_paths["prev_item"].present? %>
+      <%= link_to(
+            adjacent_paths["prev_item"],
+            class: "button button__sm button__text-secondary",
+            title: t("previous_title", scope: "decidim.admin.shared.adjacent_navigation"),
+            rel: "prev"
+          ) do %>
+        <%= icon "arrow-left-s-line", class: "fill-current" %>
+        <%= t("previous", scope: "decidim.admin.shared.adjacent_navigation") %>
+      <% end %>
+    <% end %>
+  </span>
+
+  <span>
+    <% if adjacent_paths["next_item"].present? %>
+      <%= link_to(
+            adjacent_paths["next_item"],
+            class: "button button__sm button__text-secondary",
+            title: t("next_title", scope: "decidim.admin.shared.adjacent_navigation"),
+            rel: "next"
+          ) do %>
+          <%= t("next", scope: "decidim.admin.shared.adjacent_navigation") %>
+        <%= icon "arrow-right-s-line", class: "fill-current" %>
+      <% end %>
+    <% end %>
+  </span>
+</div>

decidim-admin/app/views/decidim/admin/shared/_filters.html.erb

@@ -31,6 +31,6 @@
     <% end %>
   </div>
 </div>
-<div class="fcell grid-x" data-applied-filters-tags>
+<div class="fcell flex items-center gap-2" data-applied-filters-tags>
   <%= applied_filters_tags(i18n_ctx) %>
 </div>

decidim-admin/config/locales/en.yml

@@ -453,6 +453,7 @@ en:
           values:
             'false': Published
             'true': Unpublished
+        remove_all: Remove all
         scope_id_eq:
           label: Scope
         search_label: Search
@@ -968,6 +969,11 @@ en:
           help: These tokens are used to publicly share this unpublished resource to any user. They will be hidden when the resource is published. Click on the token's share icon to visit the shareable URL.
           title: Share tokens
       shared:
+        adjacent_navigation:
+          next: Next
+          next_title: Next item
+          previous: Previous
+          previous_title: Previous item
         gallery:
           add_images: Add images
           edit_images: Edit images

decidim-proposals/app/controllers/decidim/proposals/admin/proposals_controller.rb

@@ -14,6 +14,10 @@ class ProposalsController < Admin::ApplicationController
         helper_method :proposals, :query, :form_presenter, :proposal, :proposal_ids
         helper Proposals::Admin::ProposalBulkActionsHelper
 
+        before_action :check_admin_session_filters, only: [:index]
+
+        def index; end
+
         def show
           @notes_form = form(ProposalNoteForm).instance
           @answer_form = form(Admin::ProposalAnswerForm).from_model(proposal)

decidim-proposals/app/views/decidim/proposals/admin/proposals/index.html.erb

@@ -16,8 +16,8 @@
     <%= render partial: "decidim/proposals/admin/proposals/bulk_actions/assign_to_valuator" %>
     <%= render partial: "decidim/proposals/admin/proposals/bulk_actions/unassign_from_valuator" %>
   </div>
-  <%= admin_filter_selector(:proposals) %>
-  <div class="table-scroll mt-16">
+  <%= admin_filter_selector(filter_prefix_key) %>
+  <div class="table-scroll mt-8">
     <table class="table-list">
       <thead>
         <tr>

decidim-proposals/app/views/decidim/proposals/admin/proposals/show.html.erb

@@ -1,6 +1,7 @@
 <% add_decidim_page_title(translated_attribute(proposal.title)) %>
 <div class="component__show">
   <div class="card">
+    <%= render partial: "decidim/admin/shared/adjacent_navigation", locals: { adjacent_paths: filtered_adjacent_paths(proposal, :proposal_path) } %>
     <div class="component__show_header">
       <h2 class="component__show_header-title">
         <%= decidim_html_escape(present(proposal).title).html_safe %>

decidim-proposals/spec/system/admin/filter_proposals_spec.rb

@@ -115,6 +115,104 @@ def proposal_without_state(token)
     end
   end
 
+  context "when filtering by multiple filters" do
+    let!(:scope1) { create(:scope, organization:, name: { "en" => "Scope1" }) }
+    let!(:scope2) { create(:scope, organization:, name: { "en" => "Scope2" }) }
+    let!(:answered_proposal_with_scope1) { create(:proposal, :with_answer, component:, scope: scope1) }
+    let!(:unanswered_proposal_with_scope1) { create(:proposal, component:, scope: scope1) }
+    let!(:answered_proposal_with_scope2) { create(:proposal, :with_answer, component:, scope: scope2) }
+    let!(:unanswered_proposal_with_scope2) { create(:proposal, component:, scope: scope2) }
+
+    before do
+      apply_filter("Answered", "Answered")
+      visit_component_admin
+    end
+
+    it_behaves_like "a filtered collection", options: "Scope", filter: "Scope1" do
+      let(:in_filter) { translated(answered_proposal_with_scope1.title) }
+      let(:not_in_filter) { translated(answered_proposal_with_scope2.title) }
+    end
+
+    context "when multiple filters are checked" do
+      before do
+        apply_filter("Scope", "Scope1")
+      end
+
+      it "has a link to remove all filters" do
+        expect(page).to have_content(translated(answered_proposal_with_scope1.title))
+        expect(page).to have_no_content(translated(unanswered_proposal_with_scope1.title))
+        expect(page).to have_no_content(translated(answered_proposal_with_scope2.title))
+        expect(page).to have_no_content(translated(unanswered_proposal_with_scope2.title))
+
+        within("[data-applied-filters-tags]") do
+          expect(page).to have_content("Remove all")
+        end
+
+        within("[data-applied-filters-tags]") do
+          click_on("Remove all")
+        end
+
+        expect(page).to have_content(translated(answered_proposal_with_scope1.title))
+        expect(page).to have_content(translated(unanswered_proposal_with_scope1.title))
+        expect(page).to have_content(translated(answered_proposal_with_scope2.title))
+        expect(page).to have_content(translated(unanswered_proposal_with_scope2.title))
+        expect(page).to have_css("[data-applied-filters-tags]", exact_text: "")
+      end
+
+      it "stores the filters in the session and recovers it when visiting the component page" do
+        within("tr[data-id='#{answered_proposal_with_scope1.id}']") do
+          click_on("Answer proposal")
+        end
+
+        within "form.edit_proposal_answer" do
+          fill_in_i18n_editor(
+            :proposal_answer_answer,
+            "#proposal_answer-answer-tabs",
+            en: "An answer does not change the filters"
+          )
+          click_on "Answer"
+        end
+
+        within("[data-applied-filters-tags]") do
+          expect(page).to have_content("Answered: Answered")
+          expect(page).to have_content("Scope: Scope1")
+          expect(page).to have_content("Remove all")
+        end
+
+        filter_params = CGI.parse(URI.parse(page.current_url).query)
+        expect(filter_params["q[scope_id_eq]"]).to eq([scope1.id.to_s])
+        expect(filter_params["q[with_any_state]"]).to eq(["state_published"])
+      end
+    end
+
+    context "when the admin visits a proposal from a filtered list" do
+      let!(:other_proposal_with_scope2) { create(:proposal, :with_answer, component:, scope: scope2) }
+
+      before { visit_component_admin }
+
+      it "can navigate through the proposals of the filtered list" do
+        ids = find_all("tr[data-id]").map { |node| node["data-id"].to_i }
+
+        within("tr[data-id='#{ids[0]}']") do
+          click_on("Answer proposal")
+        end
+
+        expect(page).to have_no_link("Previous")
+        expect(page).to have_link("Next", href: %r{/manage/proposals/#{ids[1]}$})
+
+        click_on("Next")
+
+        expect(page).to have_link("Previous", href: %r{/manage/proposals/#{ids[0]}$})
+        expect(page).to have_link("Next", href: %r{/manage/proposals/#{ids[2]}$})
+
+        click_on("Next")
+
+        expect(page).to have_link("Previous", href: %r{/manage/proposals/#{ids[1]}$})
+        expect(page).to have_no_link("Next")
+      end
+    end
+  end
+
   context "when searching by ID or title" do
     let!(:proposal1) { create(:proposal, component:) }
     let!(:proposal2) { create(:proposal, component:) }