From 239f27c8951f81e220b18c0a85ffbbf880a03ba0 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Wed, 18 May 2022 15:06:15 +0100 Subject: [PATCH] Fix organisation filter (#582) * Filter values correctly * Remove filter value if hidden * Return empty string rather than nil for accessible autocomplete * Additional test for accessible automcomplete defaulting --- app/controllers/case_logs_controller.rb | 2 ++ .../conditional_filter_controller.js | 13 ++++++++++ app/frontend/controllers/index.js | 3 +++ app/helpers/filters_helper.rb | 4 +-- app/models/user.rb | 4 +-- app/views/filters/_select_filter.html.erb | 2 +- spec/features/user_spec.rb | 25 +++++++++++++++++++ spec/helpers/filters_helper_spec.rb | 23 +++++++++++++++++ spec/models/user_spec.rb | 4 +-- spec/requests/case_logs_controller_spec.rb | 8 ++++++ 10 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 app/frontend/controllers/conditional_filter_controller.js diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 24ecb1e06..a80705962 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -127,6 +127,7 @@ private filters = JSON.parse(session[:case_logs_filters]) filters.each do |category, values| next if Array(values).reject(&:empty?).blank? + next if category == "organisation" && params["organisation_select"] == "all" query = query.public_send("filter_by_#{category}", values, current_user) end @@ -138,6 +139,7 @@ private def set_session_filters new_filters = session[:case_logs_filters].present? ? JSON.parse(session[:case_logs_filters]) : {} current_user.case_logs_filters.each { |filter| new_filters[filter] = params[filter] if params[filter].present? } + new_filters = new_filters.except("organisation") if params["organisation_select"] == "all" session[:case_logs_filters] = new_filters.to_json end diff --git a/app/frontend/controllers/conditional_filter_controller.js b/app/frontend/controllers/conditional_filter_controller.js new file mode 100644 index 000000000..f4fee7eb9 --- /dev/null +++ b/app/frontend/controllers/conditional_filter_controller.js @@ -0,0 +1,13 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + initialize() { + this.clearIfHidden() + } + + clearIfHidden() { + if(this.element.style["display"] == "none") { + this.element.value = "" + } + } +} diff --git a/app/frontend/controllers/index.js b/app/frontend/controllers/index.js index 7c597baa9..d182110a0 100644 --- a/app/frontend/controllers/index.js +++ b/app/frontend/controllers/index.js @@ -6,6 +6,9 @@ import { application } from "./application" import AccessibleAutocompleteController from "./accessible_autocomplete_controller.js" application.register("accessible-autocomplete", AccessibleAutocompleteController) +import ConditionalFilterController from "./conditional_filter_controller.js" +application.register("conditional-filter", ConditionalFilterController) + import ConditionalQuestionController from "./conditional_question_controller.js" application.register("conditional-question", ConditionalQuestionController) diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index d39467b76..b5d661c94 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -4,7 +4,7 @@ module FiltersHelper selected_filters = JSON.parse(session[:case_logs_filters]) return true if selected_filters.blank? && filter == "user" && value == :all - return true if selected_filters.blank? && filter == "organisation_select" && value == :all + return true if !selected_filters.key?("organisation") && filter == "organisation_select" && value == :all return true if selected_filters["organisation"].present? && filter == "organisation_select" && value == :specific_org return false if selected_filters[filter].blank? @@ -20,6 +20,6 @@ module FiltersHelper def selected_option(filter) return false unless session[:case_logs_filters] - JSON.parse(session[:case_logs_filters])[filter] + JSON.parse(session[:case_logs_filters])[filter] || "" end end diff --git a/app/models/user.rb b/app/models/user.rb index 09fc93c3c..6cb41585b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -91,9 +91,9 @@ class User < ApplicationRecord def case_logs_filters if support? - %i[status years user organisation] + %w[status years user organisation] else - %i[status years user] + %w[status years user] end end end diff --git a/app/views/filters/_select_filter.html.erb b/app/views/filters/_select_filter.html.erb index e2c71988e..f5f6c441f 100644 --- a/app/views/filters/_select_filter.html.erb +++ b/app/views/filters/_select_filter.html.erb @@ -4,4 +4,4 @@ :name, label: { hidden: secondary }, options: { disabled: [""], selected: selected_option(category) }, - "data-controller": "accessible-autocomplete" %> + "data-controller": %w[accessible-autocomplete conditional-filter] %> diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index dc4a24591..6e197f07d 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -524,5 +524,30 @@ RSpec.describe "User Features" do click_button("Send email") end end + + context "when viewing logs" do + context "when filtering by organisation and then switching back to all organisations", js: true do + let!(:organisation) { FactoryBot.create(:organisation, name: "Filtered Org") } + + before do + allow(SecureRandom).to receive(:random_number).and_return(otp) + click_button("Sign in") + fill_in("code", with: otp) + click_button("Submit") + end + + it "clears the previously selected organisation value" do + visit("/logs") + choose("organisation-select-specific-org-field", allow_label_click: true) + expect(page).to have_field("organisation-field", with: "") + find("#organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter) + click_button("Apply filters") + expect(page).to have_current_path("/logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=specific_org&organisation=#{organisation.id}") + choose("organisation-select-all-field", allow_label_click: true) + click_button("Apply filters") + expect(page).to have_current_path("/logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=all") + end + end + end end end diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb index e665e32a0..d220b79af 100644 --- a/spec/helpers/filters_helper_spec.rb +++ b/spec/helpers/filters_helper_spec.rb @@ -57,5 +57,28 @@ RSpec.describe FiltersHelper do expect(filter_selected?("organisation_select", :specific_org)).to be false end end + + context "when the specific organisation filter is not set" do + before do + session[:case_logs_filters] = { "status" => [""], "years" => [""], "user" => "all" }.to_json + end + + it "marks the all options as checked" do + expect(filter_selected?("organisation_select", :all)).to be true + expect(filter_selected?("organisation_select", :specific_org)).to be false + end + end + end + + describe "#selected_option" do + before do + session[:case_logs_filters] = {}.to_json + end + + context "when nothing has been selected" do + it "returns an empty string" do + expect(selected_option("organisation")).to eq("") + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4cbebcdae..9dff75fe3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -99,7 +99,7 @@ RSpec.describe User, type: :model do end it "can filter case logs by user, year and status" do - expect(user.case_logs_filters).to eq(%i[status years user]) + expect(user.case_logs_filters).to eq(%w[status years user]) end end @@ -125,7 +125,7 @@ RSpec.describe User, type: :model do end it "can filter case logs by user, year, status and organisation" do - expect(user.case_logs_filters).to eq(%i[status years user organisation]) + expect(user.case_logs_filters).to eq(%w[status years user organisation]) end end end diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb index 3c91db3e4..51fe25fe3 100644 --- a/spec/requests/case_logs_controller_spec.rb +++ b/spec/requests/case_logs_controller_spec.rb @@ -392,6 +392,14 @@ RSpec.describe CaseLogsController, type: :request do expect(page).not_to have_content(tenant_code_2) end end + + context "when the support user has filtered by organisation, then switches back to all organisations" do + it "shows all organisations" do + get "http://localhost:3000/logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=all&organisation=#{org_1.id}", headers:, params: {} + expect(page).to have_content(tenant_code_1) + expect(page).to have_content(tenant_code_2) + end + end end context "when there are more than 20 logs" do