From 91a9137c7502fad817541670f09c8bc607e50327 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Mon, 16 May 2022 15:48:10 +0100 Subject: [PATCH] CLDC-1101: Case log filter by organisation (#522) * Rename org filter * Ensure filters work when ID is passed * UI init * Lint * Fix filter nesting * Reduce width * Set checked status of filters correctly * Test filter presence * Add filter test * Rubocop * Tweak styles for autocomplete within filter Co-authored-by: Paul Robert Lloyd --- app/controllers/case_logs_controller.rb | 5 +-- app/frontend/styles/_filter-layout.scss | 6 +-- app/frontend/styles/_filter.scss | 15 +++++++ app/frontend/styles/application.scss | 8 ++++ app/helpers/filters_helper.rb | 8 ++++ app/models/case_log.rb | 2 +- app/models/organisation.rb | 2 +- app/models/user.rb | 10 ++++- app/views/case_logs/_log_filters.erb | 23 ++++++++++- app/views/filters/_radio_filter.html.erb | 20 ++++++--- app/views/filters/_select_filter.html.erb | 7 ++++ spec/helpers/filters_helper_spec.rb | 30 ++++++++++++-- spec/models/case_log_spec.rb | 25 ++++++++++++ spec/models/user_spec.rb | 8 ++++ spec/requests/case_logs_controller_spec.rb | 47 +++++++++++++++++++++- 15 files changed, 195 insertions(+), 21 deletions(-) create mode 100644 app/views/filters/_select_filter.html.erb diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index fa33ad5df..24ecb1e06 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -126,7 +126,7 @@ private if session[:case_logs_filters].present? filters = JSON.parse(session[:case_logs_filters]) filters.each do |category, values| - next if values.reject(&:empty?).blank? + next if Array(values).reject(&:empty?).blank? query = query.public_send("filter_by_#{category}", values, current_user) end @@ -137,8 +137,7 @@ private def set_session_filters new_filters = session[:case_logs_filters].present? ? JSON.parse(session[:case_logs_filters]) : {} - %i[status years].each { |filter| new_filters[filter] = params[filter] if params[filter].present? } - new_filters[:user] = [params[:user]] if params[:user].present? + current_user.case_logs_filters.each { |filter| new_filters[filter] = params[filter] if params[filter].present? } session[:case_logs_filters] = new_filters.to_json end diff --git a/app/frontend/styles/_filter-layout.scss b/app/frontend/styles/_filter-layout.scss index 0dbd1f028..b9fb838ef 100644 --- a/app/frontend/styles/_filter-layout.scss +++ b/app/frontend/styles/_filter-layout.scss @@ -3,7 +3,7 @@ } .app-filter-layout__filter { - @include govuk-media-query(desktop) { + @include govuk-media-query(wide) { float: left; min-width: govuk-grid-width("one-quarter"); } @@ -12,7 +12,7 @@ .js-enabled .app-filter-layout__filter { outline: 0 none; - @include govuk-media-query($until: desktop) { + @include govuk-media-query($until: wide) { background-color: govuk-colour("light-grey"); bottom: govuk-spacing(1); border: 1px solid govuk-colour("mid-grey"); @@ -32,7 +32,7 @@ } .app-filter-layout__content { - @include govuk-media-query(desktop) { + @include govuk-media-query(wide) { float: right; max-width: calc(#{govuk-grid-width("three-quarters")} - #{govuk-spacing(6)}); width: 100%; diff --git a/app/frontend/styles/_filter.scss b/app/frontend/styles/_filter.scss index 2b4f17953..42b7e2f6a 100644 --- a/app/frontend/styles/_filter.scss +++ b/app/frontend/styles/_filter.scss @@ -98,4 +98,19 @@ background-color: govuk-colour("white"); } } + + .autocomplete__input { + @include govuk-font(16); + background-color: govuk-colour("white"); + } + + .autocomplete__wrapper { + @include govuk-media-query(wide) { + max-width: 20ex; + } + } + + .autocomplete__option { + @include govuk-font(16); + } } diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index abe376e99..b9de7492c 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -11,6 +11,14 @@ $govuk-image-url-function: frontend-image-url; $govuk-global-styles: true; $govuk-new-link-styles: true; +// Add additional breakpoint named `wide` +$govuk-breakpoints: ( + mobile: 320px, + tablet: 641px, + desktop: 769px, + wide: 921px, +); + @import "govuk-frontend-styles"; @import "govuk-prototype-styles"; diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index c142b6e6d..d39467b76 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -4,6 +4,8 @@ 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["organisation"].present? && filter == "organisation_select" && value == :specific_org return false if selected_filters[filter].blank? selected_filters[filter].include?(value.to_s) @@ -14,4 +16,10 @@ module FiltersHelper CaseLog.statuses.keys.map { |status| statuses[status] = status.humanize } statuses end + + def selected_option(filter) + return false unless session[:case_logs_filters] + + JSON.parse(session[:case_logs_filters])[filter] + end end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index ff2a962c4..df91376cf 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -34,7 +34,7 @@ class CaseLog < ApplicationRecord belongs_to :managing_organisation, class_name: "Organisation" belongs_to :created_by, class_name: "User" - scope :for_organisation, ->(org) { where(owning_organisation: org).or(where(managing_organisation: org)) } + scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } scope :filter_by_status, ->(status, _user = nil) { where status: } scope :filter_by_years, lambda { |years, _user = nil| first_year = years.shift diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 8096c6be1..c52c3d922 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -18,7 +18,7 @@ class Organisation < ApplicationRecord validates :provider_type, presence: true def case_logs - CaseLog.for_organisation(self) + CaseLog.filter_by_organisation(self) end def completed_case_logs diff --git a/app/models/user.rb b/app/models/user.rb index 27a884fa4..09fc93c3c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,7 +36,7 @@ class User < ApplicationRecord if support? CaseLog.all else - CaseLog.for_organisation(organisation) + CaseLog.filter_by_organisation(organisation) end end @@ -88,4 +88,12 @@ class User < ApplicationRecord ROLES.except(:support) end + + def case_logs_filters + if support? + %i[status years user organisation] + else + %i[status years user] + end + end end diff --git a/app/views/case_logs/_log_filters.erb b/app/views/case_logs/_log_filters.erb index 4e294c81e..45354fb40 100644 --- a/app/views/case_logs/_log_filters.erb +++ b/app/views/case_logs/_log_filters.erb @@ -6,12 +6,31 @@
<%= form_with url: "/logs", html: { method: :get } do |f| %> <% years = {"2021": "2021/22", "2022": "2022/23"} %> - <% all_or_yours = {"all": "All", "yours": "Yours"} %> + <% all_or_yours = {"all": { label: "All" }, "yours": { label: "Yours" } } %> <%= render partial: "filters/checkbox_filter", locals: { f: f, options: years, label: "Collection year", category: "years" } %> <%= render partial: "filters/checkbox_filter", locals: { f: f, options: status_filters, label: "Status", category: "status" } %> <%= render partial: "filters/radio_filter", locals: { f: f, options: all_or_yours, label: "Logs", category: "user", } %> + <% if @current_user.support? %> + <%= render partial: "filters/radio_filter", locals: { + f: f, + options: { + "all": { label: "All" }, + "specific_org": { + label: "Specific organisation", + conditional_filter: { + type: "select", + label: "Organisation", + category: "organisation", + options: [OpenStruct.new(id: "", name: "Select an option")] + Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } + } + } + }, + label: "Organisation", + category: "organisation_select" + } %> + <% end %> <%= f.govuk_submit "Apply filters", class: "govuk-!-margin-bottom-0" %> <% end %>
- \ No newline at end of file + diff --git a/app/views/filters/_radio_filter.html.erb b/app/views/filters/_radio_filter.html.erb index aff0f6ed6..abb55eafe 100644 --- a/app/views/filters/_radio_filter.html.erb +++ b/app/views/filters/_radio_filter.html.erb @@ -1,8 +1,18 @@ <%= f.govuk_radio_buttons_fieldset category.to_sym, legend: { text: label, size: "s" }, small: true, form_group: { classes: "app-filter__group" } do %> - <% options.map do |key, option| %> - <%= f.govuk_radio_button category, key.to_s, - label: { text: option }, - checked: filter_selected?(category, key), - size: "s" %> + <% options.map do |key, option| %> + <%= f.govuk_radio_button category, key.to_s, + label: { text: option[:label] }, + checked: filter_selected?(category, key), + size: "s" do %> + <% if option[:conditional_filter] %> + <%= render partial: "filters/#{option[:conditional_filter][:type]}_filter", locals: { + f:, + collection: option[:conditional_filter][:options], + category: option[:conditional_filter][:category], + label: option[:conditional_filter][:label], + secondary: true, + } %> + <% end %> <% end %> + <% end %> <% end %> diff --git a/app/views/filters/_select_filter.html.erb b/app/views/filters/_select_filter.html.erb new file mode 100644 index 000000000..e2c71988e --- /dev/null +++ b/app/views/filters/_select_filter.html.erb @@ -0,0 +1,7 @@ +<%= f.govuk_collection_select category.to_sym, + collection, + :id, + :name, + label: { hidden: secondary }, + options: { disabled: [""], selected: selected_option(category) }, + "data-controller": "accessible-autocomplete" %> diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb index 296e336a4..e665e32a0 100644 --- a/spec/helpers/filters_helper_spec.rb +++ b/spec/helpers/filters_helper_spec.rb @@ -16,8 +16,8 @@ RSpec.describe FiltersHelper do context "when looking at the all value" do it "returns true if no filters have been set yet" do - expect(filter_selected?("user", :all)).to be_truthy - expect(filter_selected?("user", :yours)).to be_falsey + expect(filter_selected?("user", :all)).to be true + expect(filter_selected?("user", :yours)).to be false end end end @@ -28,11 +28,33 @@ RSpec.describe FiltersHelper do end it "returns false for non selected filters" do - expect(filter_selected?("status", "completed")).to be_falsey + expect(filter_selected?("status", "completed")).to be false end it "returns true for selected filter" do - expect(filter_selected?("status", "in_progress")).to be_truthy + expect(filter_selected?("status", "in_progress")).to be true + end + end + + context "when support user is using the organisation filter" do + before do + session[:case_logs_filters] = { "organisation": "1" }.to_json + end + + it "returns true for the parent organisation_select filter" do + expect(filter_selected?("organisation_select", :specific_org)).to be true + expect(filter_selected?("organisation_select", :all)).to be false + end + end + + context "when support user has not set the organisation_select filter" do + before do + session[:case_logs_filters] = {}.to_json + end + + it "defaults to all organisations" do + expect(filter_selected?("organisation_select", :all)).to be true + expect(filter_selected?("organisation_select", :specific_org)).to be false end end end diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index 4a8bc8677..005e8c057 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -1893,6 +1893,31 @@ RSpec.describe CaseLog do end end + context "when filtering by organisation" do + let(:organisation_1) { FactoryBot.create(:organisation) } + let(:organisation_2) { FactoryBot.create(:organisation) } + let(:organisation_3) { FactoryBot.create(:organisation) } + + before do + FactoryBot.create(:case_log, :in_progress, owning_organisation: organisation_1, managing_organisation: organisation_1) + FactoryBot.create(:case_log, :completed, owning_organisation: organisation_1, managing_organisation: organisation_2) + FactoryBot.create(:case_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_1) + FactoryBot.create(:case_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_2) + end + + it "filters by given organisation id" do + expect(described_class.filter_by_organisation([organisation_1.id]).count).to eq(3) + expect(described_class.filter_by_organisation([organisation_1.id, organisation_2.id]).count).to eq(4) + expect(described_class.filter_by_organisation([organisation_3.id]).count).to eq(0) + end + + it "filters by given organisation" do + expect(described_class.filter_by_organisation([organisation_1]).count).to eq(3) + expect(described_class.filter_by_organisation([organisation_1, organisation_2]).count).to eq(4) + expect(described_class.filter_by_organisation([organisation_3]).count).to eq(0) + end + end + context "when filtering on status" do it "allows filtering on a single status" do expect(described_class.filter_by_status(%w[in_progress]).count).to eq(2) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 97b1fdaf1..4cbebcdae 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -97,6 +97,10 @@ RSpec.describe User, type: :model do data_coordinator: 2, }) end + + it "can filter case logs by user, year and status" do + expect(user.case_logs_filters).to eq(%i[status years user]) + end end context "when the user is a Customer Support person" do @@ -119,6 +123,10 @@ RSpec.describe User, type: :model do support: 99, }) 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]) + end end end diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb index 9a9dd60e1..293a3997d 100644 --- a/spec/requests/case_logs_controller_spec.rb +++ b/spec/requests/case_logs_controller_spec.rb @@ -186,6 +186,7 @@ RSpec.describe CaseLogsController, type: :request do context "when filtering" do context "with status filter" do + let(:organisation_2) { FactoryBot.create(:organisation) } let!(:in_progress_case_log) do FactoryBot.create(:case_log, :in_progress, owning_organisation: organisation, @@ -193,7 +194,7 @@ RSpec.describe CaseLogsController, type: :request do end let!(:completed_case_log) do FactoryBot.create(:case_log, :completed, - owning_organisation: organisation, + owning_organisation: organisation_2, managing_organisation: organisation) end @@ -209,6 +210,12 @@ RSpec.describe CaseLogsController, type: :request do expect(page).not_to have_link(completed_case_log.id.to_s) end + it "filters on organisation" do + get "/logs?organisation[]=#{organisation_2.id}", headers: headers, params: {} + expect(page).to have_link(completed_case_log.id.to_s) + expect(page).not_to have_link(in_progress_case_log.id.to_s) + end + it "does not reset the filters" do get "/logs?status[]=in_progress", headers: headers, params: {} expect(page).to have_link(in_progress_case_log.id.to_s) @@ -344,6 +351,44 @@ RSpec.describe CaseLogsController, type: :request do it "shows the download csv link" do expect(page).to have_link("Download (CSV)", href: "/logs.csv") end + + it "does not show the organisation filter" do + expect(page).not_to have_field("organisation-field") + end + end + + context "when the user is a customer support user" do + let(:user) { FactoryBot.create(:user, :support) } + let(:org_1) { FactoryBot.create(:organisation) } + let(:org_2) { FactoryBot.create(:organisation) } + let(:tenant_code_1) { "TC5638" } + let(:tenant_code_2) { "TC8745" } + + before do + FactoryBot.create(:case_log, :in_progress, owning_organisation: org_1, tenant_code: tenant_code_1) + FactoryBot.create(:case_log, :in_progress, owning_organisation: org_2, tenant_code: tenant_code_2) + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + it "does show the organisation filter" do + get "/logs", headers:, params: {} + expect(page).to have_field("organisation-field") + end + + it "shows all logs by default" do + get "/logs", headers:, params: {} + expect(page).to have_content(tenant_code_1) + expect(page).to have_content(tenant_code_2) + end + + context "when filtering by organisation" do + it "only show the selected organisations logs" do + get "/logs?organisation_select=specific_org&organisation=#{org_1.id}", headers:, params: {} + expect(page).to have_content(tenant_code_1) + expect(page).not_to have_content(tenant_code_2) + end + end end context "when there are more than 20 logs" do