diff --git a/app/controllers/delete_logs_controller.rb b/app/controllers/delete_logs_controller.rb
index c2c6a3327..0b4dc3ba5 100644
--- a/app/controllers/delete_logs_controller.rb
+++ b/app/controllers/delete_logs_controller.rb
@@ -1,6 +1,4 @@
class DeleteLogsController < ApplicationController
- include Modules::LogsFilter
-
rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
before_action :session_filters, if: :current_user, except: %i[discard_lettings_logs discard_sales_logs discard_lettings_logs_for_organisation discard_sales_logs_for_organisation]
@@ -112,6 +110,15 @@ class DeleteLogsController < ApplicationController
private
+ def session_filters
+ @session_filters ||= filter_manager.session_filters
+ end
+
+ def filter_manager
+ log_type = action_name.include?("lettings") ? "lettings_logs" : "sales_logs"
+ FilterManager.new(current_user:, session:, params:, filter_type: log_type)
+ end
+
def delete_logs_form(log_type:, organisation: false, selected_ids: nil, form_params: {})
paths = case log_type
when :lettings
diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb
index 8a242b11e..ca343290b 100644
--- a/app/helpers/filters_helper.rb
+++ b/app/helpers/filters_helper.rb
@@ -11,14 +11,15 @@ module FiltersHelper
selected_filters[filter].include?(value.to_s)
end
- def any_filter_selected?
- return false unless session[:logs_filters]
-
- selected_filters = JSON.parse(session[:logs_filters])
- filter_selected?("user", "yours") ||
- selected_filters["organisation"]&.present? ||
- selected_filters["status"]&.compact_blank&.any? ||
- selected_filters["years"]&.compact_blank&.any?
+ def any_filter_selected?(filter_type)
+ filters_json = session[session_name_for(filter_type)]
+ return false unless filters_json
+
+ filters = JSON.parse(filters_json)
+ filters["user"] == "yours" ||
+ filters["organisation"]&.present? ||
+ filters["status"]&.compact_blank&.any? ||
+ filters["years"]&.compact_blank&.any?
end
def status_filters
diff --git a/app/helpers/log_list_helper.rb b/app/helpers/log_list_helper.rb
index 54ca4c668..b3e967358 100644
--- a/app/helpers/log_list_helper.rb
+++ b/app/helpers/log_list_helper.rb
@@ -1,9 +1,9 @@
module LogListHelper
- def display_delete_logs?(current_user, search_term)
+ def display_delete_logs?(current_user, search_term, filter_type)
if current_user.data_provider?
- filter_selected?("user", "yours")
+ filter_selected?("user", "yours", filter_type)
else
- any_filter_selected? || search_term.present?
+ any_filter_selected?(filter_type) || search_term.present?
end
end
diff --git a/app/models/forms/delete_logs_form.rb b/app/models/forms/delete_logs_form.rb
index 220fd15dd..6c4d8eba6 100644
--- a/app/models/forms/delete_logs_form.rb
+++ b/app/models/forms/delete_logs_form.rb
@@ -11,7 +11,7 @@ module Forms
@log_type = attributes[:log_type]
@search_term = attributes[:search_term]
@current_user = attributes[:current_user]
- @logs = FilterService.filter_logs(visible_logs, @search_term, attributes[:log_filters], nil, @current_user)
+ @logs = FilterManager.filter_logs(visible_logs, @search_term, attributes[:log_filters], nil, @current_user)
@selected_ids = attributes[:selected_ids] || @logs.map(&:id)
@delete_confirmation_path = attributes[:delete_confirmation_path]
@back_to_logs_path = attributes[:back_to_logs_path]
diff --git a/app/views/logs/_log_list.html.erb b/app/views/logs/_log_list.html.erb
index 73d0ccf9b..d41bdc9a8 100644
--- a/app/views/logs/_log_list.html.erb
+++ b/app/views/logs/_log_list.html.erb
@@ -10,7 +10,8 @@
<% end %>
- <% if logs&.any? && (display_delete_logs?(@current_user, @searched) || in_organisations_tab?) %>
+ <% if logs&.any? && (display_delete_logs?(@current_user, searched, filter_type) || in_organisations_tab?) %>
+ <%# remove @ on current user %>
<%= govuk_link_to "Delete logs", delete_logs_path, class: "app-!-colour-red" %>
<% end %>
diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb
index 7a9b34962..eac53ed7c 100644
--- a/app/views/logs/index.html.erb
+++ b/app/views/logs/index.html.erb
@@ -71,6 +71,7 @@
csv_download_url: csv_download_url_for_controller(controller:, search: @searched, codes_only: false),
csv_codes_only_download_url: csv_download_url_for_controller(controller:, search: @searched, codes_only: true),
delete_logs_path: @delete_logs_path,
+ filter_type: @filter_type,
} %>
<%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb
index a433cebd2..89f54fb1d 100644
--- a/app/views/organisations/logs.html.erb
+++ b/app/views/organisations/logs.html.erb
@@ -35,6 +35,7 @@
csv_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: false),
csv_codes_only_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: true),
delete_logs_path: @delete_logs_path,
+ filter_type: @filter_type,
} %>
<%= render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb
index 84d6030b8..fa803e6fa 100644
--- a/spec/helpers/filters_helper_spec.rb
+++ b/spec/helpers/filters_helper_spec.rb
@@ -71,15 +71,16 @@ RSpec.describe FiltersHelper do
end
describe "#any_filter_selected?" do
- let(:result) { any_filter_selected? }
+ let(:filter_type) { "lettings_logs" }
+ let(:result) { any_filter_selected?(filter_type) }
let(:serialised_filters) { filters&.to_json }
let(:filters) { nil }
before do
- session[:logs_filters] = serialised_filters if serialised_filters
+ session[:lettings_logs_filters] = serialised_filters if serialised_filters
end
- it "returns false if the session contains no logs filters" do
+ it "returns false if the session contains no filters" do
expect(result).to be_falsey
end
diff --git a/spec/helpers/log_list_helper_spec.rb b/spec/helpers/log_list_helper_spec.rb
index 2a34135b6..22836ec45 100644
--- a/spec/helpers/log_list_helper_spec.rb
+++ b/spec/helpers/log_list_helper_spec.rb
@@ -5,9 +5,10 @@ RSpec.describe LogListHelper, type: :helper do
describe "#display_delete_logs?" do
let(:search_term) { nil }
+ let(:filter_type) { "lettings_logs" }
context "when logged in as a data provider" do
- let(:result) { display_delete_logs?(user, search_term) }
+ let(:result) { display_delete_logs?(user, search_term, filter_type) }
let(:user) { create(:user) }
it "returns false if no filters or search are set" do
@@ -16,13 +17,13 @@ RSpec.describe LogListHelper, type: :helper do
end
it "returns true if the user filter is set to 'yours'" do
- allow(self).to receive(:filter_selected?).with("user", "yours").and_return true
+ allow(self).to receive(:filter_selected?).with("user", "yours", filter_type).and_return true
expect(result).to be true
end
it "returns false if any filters other than the user filter are set" do
allow(self).to receive(:filter_selected?).and_return true
- allow(self).to receive(:filter_selected?).with("user", "yours").and_return false
+ allow(self).to receive(:filter_selected?).with("user", "yours", filter_type).and_return false
expect(result).to be false
end
@@ -30,7 +31,7 @@ RSpec.describe LogListHelper, type: :helper do
let(:search_term) { "word" }
it "still returns false as long as the user filter is not set to yours" do
- allow(self).to receive(:filter_selected?).with("user", "yours").and_return false
+ allow(self).to receive(:filter_selected?).with("user", "yours", filter_type).and_return false
expect(result).to be false
end
end
@@ -39,8 +40,8 @@ RSpec.describe LogListHelper, type: :helper do
context "when logged in as a support user or data coordinator" do
let(:support_user) { create(:user, :support) }
let(:data_coordinator) { create(:user, :data_coordinator) }
- let(:support_result) { display_delete_logs?(support_user, search_term) }
- let(:coordinator_result) { display_delete_logs?(data_coordinator, search_term) }
+ let(:support_result) { display_delete_logs?(support_user, search_term, filter_type) }
+ let(:coordinator_result) { display_delete_logs?(data_coordinator, search_term, filter_type) }
let(:results) { [support_result, coordinator_result] }
it "returns false if no filters or search are set" do
diff --git a/spec/requests/delete_logs_controller_spec.rb b/spec/requests/delete_logs_controller_spec.rb
index ab017f3ea..123d3d2d0 100644
--- a/spec/requests/delete_logs_controller_spec.rb
+++ b/spec/requests/delete_logs_controller_spec.rb
@@ -14,7 +14,7 @@ RSpec.describe "DeleteLogs", type: :request do
let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
before do
- allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return LettingsLog.all
end
it "calls the filter service with the filters in the session and the search term from the query params" do
@@ -26,7 +26,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get lettings_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
@@ -59,7 +59,7 @@ RSpec.describe "DeleteLogs", type: :request do
let(:selected_ids) { log_1.id }
before do
- allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return LettingsLog.all
end
it "throws an error if selected ids are not provided" do
@@ -75,7 +75,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get lettings_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
@@ -254,7 +254,7 @@ RSpec.describe "DeleteLogs", type: :request do
let!(:log_2) { create(:sales_log, :completed, created_by: user) }
before do
- allow(FilterService).to receive(:filter_logs).and_return SalesLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return SalesLog.all
end
it "calls the filter service with the filters in the session and the search term from the query params" do
@@ -266,7 +266,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get sales_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
@@ -299,7 +299,7 @@ RSpec.describe "DeleteLogs", type: :request do
let(:selected_ids) { log_1.id }
before do
- allow(FilterService).to receive(:filter_logs).and_return SalesLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return SalesLog.all
end
it "throws an error if selected ids are not provided" do
@@ -315,7 +315,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get sales_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters
@@ -498,7 +498,7 @@ RSpec.describe "DeleteLogs", type: :request do
let!(:log_2) { create(:lettings_log, :completed, owning_organisation: organisation) }
before do
- allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return LettingsLog.all
end
it "calls the filter service with the filters in the session and the search term from the query params" do
@@ -510,7 +510,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get lettings_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
@@ -543,7 +543,7 @@ RSpec.describe "DeleteLogs", type: :request do
let(:selected_ids) { log_1.id }
before do
- allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return LettingsLog.all
end
it "throws an error if selected ids are not provided" do
@@ -559,7 +559,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get lettings_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
@@ -723,7 +723,7 @@ RSpec.describe "DeleteLogs", type: :request do
let!(:log_2) { create(:sales_log, :completed, owning_organisation: organisation) }
before do
- allow(FilterService).to receive(:filter_logs).and_return SalesLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return SalesLog.all
end
it "calls the filter service with the filters in the session and the search term from the query params" do
@@ -735,7 +735,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get sales_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
@@ -768,7 +768,7 @@ RSpec.describe "DeleteLogs", type: :request do
let(:selected_ids) { log_1.id }
before do
- allow(FilterService).to receive(:filter_logs).and_return SalesLog.all
+ allow(FilterManager).to receive(:filter_logs).and_return SalesLog.all
end
it "throws an error if selected ids are not provided" do
@@ -784,7 +784,7 @@ RSpec.describe "DeleteLogs", type: :request do
}
get sales_logs_path(logs_filters) # adds the filters to the session
- expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3|
+ expect(FilterManager).to receive(:filter_logs) { |arg1, arg2, arg3|
expect(arg1).to contain_exactly(log_1, log_2)
expect(arg2).to eq search
expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
diff --git a/spec/views/logs/delete_logs_spec.rb b/spec/views/logs/delete_logs_spec.rb
index f8a024a86..e8200d0a3 100644
--- a/spec/views/logs/delete_logs_spec.rb
+++ b/spec/views/logs/delete_logs_spec.rb
@@ -15,7 +15,7 @@ RSpec.describe "logs/delete_logs.html.erb" do
before do
sign_in user
- allow(FilterService).to receive(:filter_logs).and_return lettings_logs
+ allow(FilterManager).to receive(:filter_logs).and_return lettings_logs
assign(:delete_logs_form, delete_logs_form)
end
@@ -40,7 +40,7 @@ RSpec.describe "logs/delete_logs.html.erb" do
before do
lettings_logs << lettings_log_2
- allow(FilterService).to receive(:filter_logs).and_return lettings_logs
+ allow(FilterManager).to receive(:filter_logs).and_return lettings_logs
delete_logs_form = Forms::DeleteLogsForm.new(log_type: :lettings, current_user: user, **paths)
assign(:delete_logs_form, delete_logs_form)
end
@@ -76,7 +76,7 @@ RSpec.describe "logs/delete_logs.html.erb" do
before do
sign_in user
- allow(FilterService).to receive(:filter_logs).and_return sales_logs
+ allow(FilterManager).to receive(:filter_logs).and_return sales_logs
assign(:delete_logs_form, delete_logs_form_sales)
end