refactor delete logs controller after tech review

improve the private method that builds the form object so that it has the flexibility to do so for all controller methods
ensure that the search term is passed to the delete logs controller when navigating through the organisations tab
ensure that noly logs for that organisation are displayed when navigating to delete logs through the organisations tab

app/controllers/delete_logs_controller.rb

@@ -4,26 +4,20 @@ class DeleteLogsController < ApplicationController
   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]
+  before_action :add_organisation_to_filters, only: %i[delete_lettings_logs_for_organisation delete_lettings_logs_for_organisation_with_selected_ids delete_lettings_logs_for_organisation_confirmation delete_sales_logs_for_organisation delete_sales_logs_for_organisation_with_selected_ids delete_sales_logs_for_organisation_confirmation]
 
   def delete_lettings_logs
-    @delete_logs_form = delete_logs_form(log_type: :lettings, paths: lettings_logs_paths)
+    @delete_logs_form = delete_logs_form(log_type: :lettings)
     render "logs/delete_logs"
   end
 
   def delete_lettings_logs_with_selected_ids
-    @delete_logs_form = delete_logs_form(selected_ids:, log_type: :lettings, paths: lettings_logs_paths)
+    @delete_logs_form = delete_logs_form(log_type: :lettings, selected_ids:)
     render "logs/delete_logs"
   end
 
   def delete_lettings_logs_confirmation
-    default_attributes = {
-      current_user:,
-      log_filters: @session_filters,
-      log_type: :lettings,
-    }.merge lettings_logs_paths
-    attributes = form_attributes.merge(default_attributes)
-    attributes[:selected_ids] = [] unless attributes.key? :selected_ids
-    @delete_logs_form = Forms::DeleteLogsForm.new(attributes)
+    @delete_logs_form = delete_logs_form(log_type: :lettings, form_params:)
     if @delete_logs_form.valid?
       render "logs/delete_logs_confirmation"
     else
@@ -39,24 +33,17 @@ class DeleteLogsController < ApplicationController
   end
 
   def delete_sales_logs
-    @delete_logs_form = delete_logs_form(log_type: :sales, paths: sales_logs_paths)
+    @delete_logs_form = delete_logs_form(log_type: :sales)
     render "logs/delete_logs"
   end
 
   def delete_sales_logs_with_selected_ids
-    @delete_logs_form = delete_logs_form(selected_ids:, log_type: :sales, paths: sales_logs_paths)
+    @delete_logs_form = delete_logs_form(log_type: :sales, selected_ids:)
     render "logs/delete_logs"
   end
 
   def delete_sales_logs_confirmation
-    default_attributes = {
-      current_user:,
-      log_filters: @session_filters,
-      log_type: :sales,
-    }.merge sales_logs_paths
-    attributes = form_attributes.merge(default_attributes)
-    attributes[:selected_ids] = [] unless attributes.key? :selected_ids
-    @delete_logs_form = Forms::DeleteLogsForm.new(attributes)
+    @delete_logs_form = delete_logs_form(log_type: :sales, form_params:)
     if @delete_logs_form.valid?
       render "logs/delete_logs_confirmation"
     else
@@ -72,24 +59,17 @@ class DeleteLogsController < ApplicationController
   end
 
   def delete_lettings_logs_for_organisation
-    @delete_logs_form = delete_logs_form(log_type: :lettings, paths: lettings_logs_for_organisation_paths)
+    @delete_logs_form = delete_logs_form(log_type: :lettings, organisation: true)
     render "logs/delete_logs"
   end
 
   def delete_lettings_logs_for_organisation_with_selected_ids
-    @delete_logs_form = delete_logs_form(selected_ids:, log_type: :lettings, paths: lettings_logs_for_organisation_paths)
+    @delete_logs_form = delete_logs_form(log_type: :lettings, organisation: true, selected_ids:)
     render "logs/delete_logs"
   end
 
   def delete_lettings_logs_for_organisation_confirmation
-    default_attributes = {
-      current_user:,
-      log_filters: @session_filters,
-      log_type: :lettings,
-    }.merge lettings_logs_for_organisation_paths
-    attributes = form_attributes.merge(default_attributes)
-    attributes[:selected_ids] = [] unless attributes.key? :selected_ids
-    @delete_logs_form = Forms::DeleteLogsForm.new(attributes)
+    @delete_logs_form = delete_logs_form(log_type: :lettings, organisation: true, form_params:)
     if @delete_logs_form.valid?
       render "logs/delete_logs_confirmation"
     else
@@ -98,31 +78,24 @@ class DeleteLogsController < ApplicationController
   end
 
   def discard_lettings_logs_for_organisation
-    logs = LettingsLog.find(params.require(:ids))
+    logs = LettingsLog.where(owning_organisation: params[:id]).find(params.require(:ids))
     discard logs
 
     redirect_to lettings_logs_organisation_path, notice: I18n.t("notification.logs_deleted", count: logs.count)
   end
 
   def delete_sales_logs_for_organisation
-    @delete_logs_form = delete_logs_form(log_type: :sales, paths: sales_logs_for_organisation_paths)
+    @delete_logs_form = delete_logs_form(log_type: :sales, organisation: true)
     render "logs/delete_logs"
   end
 
   def delete_sales_logs_for_organisation_with_selected_ids
-    @delete_logs_form = delete_logs_form(selected_ids:, log_type: :sales, paths: sales_logs_for_organisation_paths)
+    @delete_logs_form = delete_logs_form(log_type: :sales, organisation: true, selected_ids:)
     render "logs/delete_logs"
   end
 
   def delete_sales_logs_for_organisation_confirmation
-    default_attributes = {
-      current_user:,
-      log_filters: @session_filters,
-      log_type: :sales,
-    }.merge sales_logs_for_organisation_paths
-    attributes = form_attributes.merge(default_attributes)
-    attributes[:selected_ids] = [] unless attributes.key? :selected_ids
-    @delete_logs_form = Forms::DeleteLogsForm.new(attributes)
+    @delete_logs_form = delete_logs_form(log_type: :sales, organisation: true, form_params:)
     if @delete_logs_form.valid?
       render "logs/delete_logs_confirmation"
     else
@@ -131,7 +104,7 @@ class DeleteLogsController < ApplicationController
   end
 
   def discard_sales_logs_for_organisation
-    logs = SalesLog.find(params.require(:ids))
+    logs = SalesLog.where(owning_organisation: params[:id]).find(params.require(:ids))
     discard logs
 
     redirect_to sales_logs_organisation_path, notice: I18n.t("notification.logs_deleted", count: logs.count)
@@ -139,12 +112,28 @@ class DeleteLogsController < ApplicationController
 
 private
 
-  def delete_logs_form(log_type:, paths:, selected_ids: nil)
-    Forms::DeleteLogsForm.new(current_user:, search_term:, log_filters: @session_filters, log_type:, selected_ids:, **paths)
+  def delete_logs_form(log_type:, organisation: false, selected_ids: nil, form_params: {})
+    paths = case log_type
+            when :lettings
+              organisation ? lettings_logs_for_organisation_paths : lettings_logs_paths
+            when :sales
+              organisation ? sales_logs_for_organisation_paths : sales_logs_paths
+            end
+    attributes = {
+      log_type:,
+      current_user:,
+      log_filters: @session_filters,
+      search_term:,
+      selected_ids:,
+      **paths
+    }.merge(form_params).transform_keys(&:to_sym)
+    Forms::DeleteLogsForm.new(attributes)
   end
 
-  def form_attributes
-    params.require(:forms_delete_logs_form).permit(:search_term, selected_ids: [])
+  def form_params
+    form_attributes = params.require(:forms_delete_logs_form).permit(:search_term, selected_ids: [])
+    form_attributes[:selected_ids] = [] unless form_attributes.key? :selected_ids
+    form_attributes
   end
 
   def lettings_logs_paths
@@ -179,6 +168,10 @@ private
     }
   end
 
+  def add_organisation_to_filters
+    @session_filters[:organisation] = params[:id]
+  end
+
   def search_term
     params["search"]
   end

app/controllers/organisations_controller.rb

@@ -96,7 +96,7 @@ class OrganisationsController < ApplicationController
       format.html do
         @search_term = search_term
         @pagy, @logs = pagy(unpaginated_filtered_logs)
-        @delete_logs_path = delete_lettings_logs_organisation_path
+        @delete_logs_path = delete_lettings_logs_organisation_path(search: @search_term)
         @searched = search_term.presence
         @total_count = organisation_logs.size
         @log_type = :lettings
@@ -127,7 +127,7 @@ class OrganisationsController < ApplicationController
       format.html do
         @search_term = search_term
         @pagy, @logs = pagy(unpaginated_filtered_logs)
-        @delete_logs_path = delete_sales_logs_organisation_path
+        @delete_logs_path = delete_sales_logs_organisation_path(search: @search_term)
         @searched = search_term.presence
         @total_count = organisation_logs.size
         @log_type = :sales

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(all_logs, @search_term, attributes[:log_filters], nil, @current_user)
+      @logs = FilterService.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]
@@ -22,6 +22,10 @@ module Forms
       @logs.count
     end
 
+    def table_partial_name
+      "logs/delete_logs_table_#{@log_type}"
+    end
+
   private
 
     def at_least_one_log_selected
@@ -30,7 +34,7 @@ module Forms
       end
     end
 
-    def all_logs
+    def visible_logs
       case @log_type
       when :lettings then @current_user.lettings_logs.visible
       when :sales then @current_user.sales_logs.visible

spec/requests/delete_logs_controller_spec.rb

@@ -513,7 +513,7 @@ RSpec.describe "DeleteLogs", type: :request do
         expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
           expect(arg1).to contain_exactly(log_1, log_2)
           expect(arg2).to eq search
-          expect(arg3).to eq logs_filters
+          expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
         }.and_return LettingsLog.all
 
         get delete_lettings_logs_organisation_path(id: organisation, search:)
@@ -562,7 +562,7 @@ RSpec.describe "DeleteLogs", type: :request do
         expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
           expect(arg1).to contain_exactly(log_1, log_2)
           expect(arg2).to eq search
-          expect(arg3).to eq logs_filters
+          expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
         }.and_return LettingsLog.all
 
         post delete_lettings_logs_organisation_path(id: organisation, search:, selected_ids:)
@@ -738,7 +738,7 @@ RSpec.describe "DeleteLogs", type: :request do
         expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
           expect(arg1).to contain_exactly(log_1, log_2)
           expect(arg2).to eq search
-          expect(arg3).to eq logs_filters
+          expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
         }.and_return SalesLog.all
 
         get delete_sales_logs_organisation_path(id: organisation, search:)
@@ -787,7 +787,7 @@ RSpec.describe "DeleteLogs", type: :request do
         expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
           expect(arg1).to contain_exactly(log_1, log_2)
           expect(arg2).to eq search
-          expect(arg3).to eq logs_filters
+          expect(arg3).to eq logs_filters.merge(organisation: organisation.id.to_s)
         }.and_return SalesLog.all
 
         post delete_sales_logs_organisation_path(id: organisation, search:, selected_ids:)