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 @@ +
+
+
+

Filters

+
+ +
+ <%= 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