From 23bd5adc517009afb0b1a2ab37af7cfcb1fbe4c8 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 1 Aug 2023 16:25:41 +0100
Subject: [PATCH] CLDC-2247 Add locations filter (#1785)
* Add location filters to the controller
* Display the status filter and clear filters
* refactor status filter into scope
* Update incomplete, make reactivated scope work in conjunction with other scopes. Lint
* specs
* styling
* Update test name
* uncomment a test
* Move filters under navigation
---
app/controllers/locations_controller.rb | 13 +++-
app/controllers/sessions_controller.rb | 3 +-
app/helpers/filters_helper.rb | 17 ++++-
app/models/location.rb | 50 +++++++++++++
app/services/filter_manager.rb | 17 ++++-
.../locations/_location_filters.html.erb | 30 ++++++++
app/views/locations/index.html.erb | 12 ++-
spec/factories/location.rb | 5 ++
spec/features/schemes_spec.rb | 37 ++++++++++
spec/models/location_spec.rb | 73 +++++++++++++++++++
spec/requests/locations_controller_spec.rb | 48 ++++++++++++
11 files changed, 293 insertions(+), 12 deletions(-)
create mode 100644 app/views/locations/_location_filters.html.erb
diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb
index e55ee10d6..f89325b41 100644
--- a/app/controllers/locations_controller.rb
+++ b/app/controllers/locations_controller.rb
@@ -6,15 +6,18 @@ class LocationsController < ApplicationController
before_action :find_location, except: %i[create index]
before_action :find_scheme
before_action :scheme_and_location_present, except: %i[create index]
+ before_action :session_filters, if: :current_user, only: %i[index]
+ before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[index]
before_action :authorize_user, except: %i[index create]
def index
authorize @scheme
- @pagy, @locations = pagy(filtered_collection(@scheme.locations, search_term))
+ @pagy, @locations = pagy(filter_manager.filtered_locations(@scheme.locations, search_term, session_filters))
@total_count = @scheme.locations.size
@searched = search_term.presence
+ @filter_type = "scheme_locations"
end
def create
@@ -297,4 +300,12 @@ private
params[:referrer] == "check_answers"
end
helper_method :return_to_check_your_answers?
+
+ def filter_manager
+ FilterManager.new(current_user:, session:, params:, filter_type: "scheme_locations")
+ end
+
+ def session_filters
+ filter_manager.session_filters
+ end
end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index cdcd05c9f..225f05f8b 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -1,8 +1,9 @@
class SessionsController < ApplicationController
def clear_filters
session[session_name_for(params[:filter_type])] = "{}"
+ path_params = params[:path_params].presence || {}
- redirect_to send("#{params[:filter_type]}_path")
+ redirect_to send("#{params[:filter_type]}_path", scheme_id: path_params[:scheme_id])
end
private
diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb
index 33e109565..e2da5b0ed 100644
--- a/app/helpers/filters_helper.rb
+++ b/app/helpers/filters_helper.rb
@@ -56,6 +56,17 @@ module FiltersHelper
}.freeze
end
+ def location_status_filters
+ {
+ "incomplete" => "Incomplete",
+ "active" => "Active",
+ "deactivating_soon" => "Deactivating soon",
+ "activating_soon" => "Activating soon",
+ "reactivating_soon" => "Reactivating soon",
+ "deactivated" => "Deactivated",
+ }.freeze
+ end
+
def selected_option(filter, filter_type)
return false unless session[session_name_for(filter_type)]
@@ -80,9 +91,9 @@ module FiltersHelper
applied_filters_count(filter_type).zero? ? "No filters applied" : "#{pluralize(applied_filters_count(filter_type), 'filter')} applied"
end
- def reset_filters_link(filter_type)
+ def reset_filters_link(filter_type, path_params = {})
if applied_filters_count(filter_type).positive?
- govuk_link_to "Clear", clear_filters_path(filter_type:)
+ govuk_link_to "Clear", clear_filters_path(filter_type:, path_params:)
end
end
@@ -104,6 +115,8 @@ private
end
def applied_filters(filter_type)
+ return {} unless session[session_name_for(filter_type)]
+
JSON.parse(session[session_name_for(filter_type)])
end
diff --git a/app/models/location.rb b/app/models/location.rb
index 0dd6ba522..f25fd7d5d 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -27,6 +27,56 @@ class Location < ApplicationRecord
scope :active_in_2_weeks, -> { where(confirmed: true).and(started_in_2_weeks) }
scope :confirmed, -> { where(confirmed: true) }
scope :unconfirmed, -> { where.not(confirmed: true) }
+ scope :filter_by_status, lambda { |statuses, _user = nil|
+ filtered_records = all
+ scopes = []
+
+ statuses.each do |status|
+ if respond_to?(status, true)
+ scopes << (status == "active" ? send("active_status") : send(status))
+ end
+ end
+
+ if scopes.any?
+ filtered_records = filtered_records
+ .left_outer_joins(:location_deactivation_periods)
+ .order("location_deactivation_periods.created_at DESC")
+ .merge(scopes.reduce(&:or))
+ end
+
+ filtered_records
+ }
+
+ scope :incomplete, lambda {
+ where.not(confirmed: true)
+ }
+
+ scope :deactivated, lambda {
+ merge(LocationDeactivationPeriod.deactivations_without_reactivation)
+ .where("location_deactivation_periods.deactivation_date <= ?", Time.zone.now)
+ }
+
+ scope :deactivating_soon, lambda {
+ merge(LocationDeactivationPeriod.deactivations_without_reactivation)
+ .where("location_deactivation_periods.deactivation_date > ?", Time.zone.now)
+ }
+
+ scope :reactivating_soon, lambda {
+ where.not("location_deactivation_periods.reactivation_date IS NULL")
+ .where("location_deactivation_periods.reactivation_date > ?", Time.zone.now)
+ }
+
+ scope :activating_soon, lambda {
+ where("startdate > ?", Time.zone.now)
+ }
+
+ scope :active_status, lambda {
+ where.not(id: joins(:location_deactivation_periods).reactivating_soon.pluck(:id))
+ .where.not(id: joins(:location_deactivation_periods).deactivated.pluck(:id))
+ .where.not(id: incomplete.pluck(:id))
+ .where.not(id: joins(:location_deactivation_periods).deactivating_soon.pluck(:id))
+ .where.not(id: activating_soon.pluck(:id))
+ }
LOCAL_AUTHORITIES = LocalAuthority.all.map { |la| [la.name, la.code] }.to_h
diff --git a/app/services/filter_manager.rb b/app/services/filter_manager.rb
index 63b13ecdc..c8665752d 100644
--- a/app/services/filter_manager.rb
+++ b/app/services/filter_manager.rb
@@ -62,6 +62,17 @@ class FilterManager
schemes
end
+ def self.filter_locations(locations, search_term, filters, user)
+ locations = filter_by_search(locations, search_term)
+
+ filters.each do |category, values|
+ next if Array(values).reject(&:empty?).blank?
+
+ locations = locations.public_send("filter_by_#{category}", values, user)
+ end
+ locations.order(created_at: :desc)
+ end
+
def serialize_filters_to_session(specific_org: false)
session[session_name_for(filter_type)] = session_filters(specific_org:).to_json
end
@@ -86,7 +97,7 @@ class FilterManager
new_filters["user"] = current_user.id.to_s if params["assigned_to"] == "you"
end
- if (filter_type.include?("schemes") || filter_type.include?("users")) && params["status"].present?
+ if (filter_type.include?("schemes") || filter_type.include?("users") || filter_type.include?("scheme_locations")) && params["status"].present?
new_filters["status"] = params["status"]
end
@@ -117,6 +128,10 @@ class FilterManager
FilterManager.filter_schemes(schemes, search_term, filters, current_user, all_orgs)
end
+ def filtered_locations(locations, search_term, filters)
+ FilterManager.filter_locations(locations, search_term, filters, current_user)
+ end
+
def bulk_upload
id = (logs_filters["bulk_upload_id"] || []).reject(&:blank?)[0]
@bulk_upload ||= current_user.bulk_uploads.find_by(id:)
diff --git a/app/views/locations/_location_filters.html.erb b/app/views/locations/_location_filters.html.erb
new file mode 100644
index 000000000..d5bdeb156
--- /dev/null
+++ b/app/views/locations/_location_filters.html.erb
@@ -0,0 +1,30 @@
+
+
+
+
+
+ <%= form_with url: scheme_locations_path(@scheme), html: { method: :get } do |f| %>
+
+
+ <%= filters_applied_text(@filter_type) %>
+
+
+ <%= reset_filters_link(@filter_type, { scheme_id: @scheme.id }) %>
+
+
+
+ <%= render partial: "filters/checkbox_filter",
+ locals: {
+ f:,
+ options: location_status_filters,
+ label: "Status",
+ category: "status",
+ } %>
+
+ <%= f.govuk_submit "Apply filters", class: "govuk-!-margin-bottom-0" %>
+ <% end %>
+
+
+
diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb
index 7641bbd48..ac6e3a0df 100644
--- a/app/views/locations/index.html.erb
+++ b/app/views/locations/index.html.erb
@@ -9,21 +9,19 @@
<% end %>
<%= render partial: "organisations/headings", locals: { main: @scheme.service_name, sub: nil } %>
+
-
-
- <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %>
+ <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %>
+ <%= render partial: "locations/location_filters" %>
+
Locations
+
<%= render SearchComponent.new(current_user:, search_label: "Search by location name or postcode", value: @searched) %>
<%= govuk_section_break(visible: true, size: "m") %>
-
-
-
-
<%= govuk_table do |table| %>
<%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %>
<%= render(SearchResultCaptionComponent.new(searched: @searched, count: @pagy.count, item_label:, total_count: @total_count, item: "locations", path: request.path)) %>
diff --git a/spec/factories/location.rb b/spec/factories/location.rb
index f43da0ac8..00fa0d77f 100644
--- a/spec/factories/location.rb
+++ b/spec/factories/location.rb
@@ -21,6 +21,11 @@ FactoryBot.define do
old_visible_id { "111" }
end
+ trait :incomplete do
+ units { nil }
+ confirmed { false }
+ end
+
trait :with_old_visible_id do
old_visible_id { rand(9_999_999).to_s }
end
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index c88b831ec..73972c4c3 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -284,6 +284,43 @@ RSpec.describe "Schemes scheme Features" do
end
end
+ context "when filtering locations" do
+ before do
+ click_link("Locations")
+ end
+
+ context "when no filters are selected" do
+ it "displays the filters component with no clear button" do
+ expect(page).to have_content("No filters applied")
+ expect(page).not_to have_content("Clear")
+ end
+ end
+
+ context "when I have selected filters" do
+ before do
+ check("Active")
+ check("Incomplete")
+ click_button("Apply filters")
+ end
+
+ it "displays the filters component with a correct count and clear button" do
+ expect(page).to have_content("2 filters applied")
+ expect(page).to have_content("Clear")
+ end
+
+ context "when clearing the filters" do
+ before do
+ click_link("Clear")
+ end
+
+ it "clears the filters and displays the filter component as before" do
+ expect(page).to have_content("No filters applied")
+ expect(page).not_to have_content("Clear")
+ end
+ end
+ end
+ end
+
context "when the user clicks add location" do
before do
click_link("Locations")
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index 1fe359136..f24a5f4fa 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -930,6 +930,79 @@ RSpec.describe Location, type: :model do
end
end
+ describe "filter by status" do
+ let!(:incomplete_location) { FactoryBot.create(:location, :incomplete, startdate: Time.zone.local(2022, 4, 1)) }
+ let!(:active_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
+ let(:deactivating_soon_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
+ let(:deactivated_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
+ let(:reactivating_soon_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
+ let!(:activating_soon_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 7, 7)) }
+
+ before do
+ Timecop.freeze(2022, 6, 7)
+ FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 8, 8), location: deactivating_soon_location)
+ deactivating_soon_location.save!
+ FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 6), location: deactivated_location)
+ deactivated_location.save!
+ FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 7), reactivation_date: Time.zone.local(2022, 6, 8), location: reactivating_soon_location)
+ reactivating_soon_location.save!
+ end
+
+ after do
+ Timecop.unfreeze
+ end
+
+ context "when filtering by incomplete status" do
+ it "returns only incomplete locations" do
+ expect(described_class.filter_by_status(%w[incomplete]).count).to eq(1)
+ expect(described_class.filter_by_status(%w[incomplete]).first).to eq(incomplete_location)
+ end
+ end
+
+ context "when filtering by active status" do
+ it "returns only active locations" do
+ expect(described_class.filter_by_status(%w[active]).count).to eq(1)
+ expect(described_class.filter_by_status(%w[active]).first).to eq(active_location)
+ end
+ end
+
+ context "when filtering by deactivating_soon status" do
+ it "returns only deactivating_soon locations" do
+ expect(described_class.filter_by_status(%w[deactivating_soon]).count).to eq(1)
+ expect(described_class.filter_by_status(%w[deactivating_soon]).first).to eq(deactivating_soon_location)
+ end
+ end
+
+ context "when filtering by deactivated status" do
+ it "returns only deactivated locations" do
+ expect(described_class.filter_by_status(%w[deactivated]).count).to eq(1)
+ expect(described_class.filter_by_status(%w[deactivated]).first).to eq(deactivated_location)
+ end
+ end
+
+ context "when filtering by reactivating_soon status" do
+ it "returns only reactivating_soon locations" do
+ expect(described_class.filter_by_status(%w[reactivating_soon]).count).to eq(1)
+ expect(described_class.filter_by_status(%w[reactivating_soon]).first).to eq(reactivating_soon_location)
+ end
+ end
+
+ context "when filtering by activating_soon status" do
+ it "returns only activating_soon locations" do
+ expect(described_class.filter_by_status(%w[activating_soon]).count).to eq(1)
+ expect(described_class.filter_by_status(%w[activating_soon]).first).to eq(activating_soon_location)
+ end
+ end
+
+ context "when filtering by multiple statuses" do
+ it "returns relevant locations" do
+ expect(described_class.filter_by_status(%w[deactivating_soon activating_soon]).count).to eq(2)
+ expect(described_class.filter_by_status(%w[deactivating_soon activating_soon])).to include(activating_soon_location)
+ expect(described_class.filter_by_status(%w[deactivating_soon activating_soon])).to include(deactivating_soon_location)
+ end
+ end
+ end
+
describe "available_from" do
context "when there is a startdate" do
let(:location) { FactoryBot.build(:location, startdate: Time.zone.local(2022, 4, 6)) }
diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb
index 52b1c087f..7dccbec1b 100644
--- a/spec/requests/locations_controller_spec.rb
+++ b/spec/requests/locations_controller_spec.rb
@@ -119,6 +119,54 @@ RSpec.describe LocationsController, type: :request do
it "returns 200" do
expect(response).to be_successful
end
+
+ context "when filtering" do
+ context "with status filter" do
+ let(:scheme) { create(:scheme, owning_organisation: user.organisation) }
+ let!(:incomplete_location) { create(:location, :incomplete, scheme:, startdate: Time.zone.local(2022, 4, 1)) }
+ let!(:active_location) { create(:location, scheme:, startdate: Time.zone.local(2022, 4, 1)) }
+ let!(:deactivated_location) { create(:location, scheme:, startdate: Time.zone.local(2022, 4, 1)) }
+
+ before do
+ create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 4, 1), location: deactivated_location)
+ end
+
+ it "shows locations for multiple selected statuses" do
+ get "/schemes/#{scheme.id}/locations?status[]=incomplete&status[]=active", headers:, params: {}
+ expect(page).to have_link(incomplete_location.postcode)
+ expect(page).to have_link(active_location.postcode)
+ end
+
+ it "shows filtered incomplete locations" do
+ get "/schemes/#{scheme.id}/locations?status[]=incomplete", headers:, params: {}
+ expect(page).to have_link(incomplete_location.postcode)
+ expect(page).not_to have_link(active_location.postcode)
+ end
+
+ it "shows filtered active locations" do
+ get "/schemes/#{scheme.id}/locations?status[]=active", headers:, params: {}
+ expect(page).to have_link(active_location.postcode)
+ expect(page).not_to have_link(incomplete_location.postcode)
+ end
+
+ it "shows filtered deactivated locations" do
+ get "/schemes/#{scheme.id}/locations?status[]=deactivated", headers:, params: {}
+ expect(page).to have_link(deactivated_location.postcode)
+ expect(page).not_to have_link(active_location.postcode)
+ expect(page).not_to have_link(incomplete_location.postcode)
+ end
+
+ it "does not reset the filters" do
+ get "/schemes/#{scheme.id}/locations?status[]=incomplete", headers:, params: {}
+ expect(page).to have_link(incomplete_location.postcode)
+ expect(page).not_to have_link(active_location.postcode)
+
+ get "/schemes/#{scheme.id}/locations", headers:, params: {}
+ expect(page).to have_link(incomplete_location.postcode)
+ expect(page).not_to have_link(active_location.postcode)
+ end
+ end
+ end
end
context "when signed in as a data coordinator user" do