refactor after code review

app/controllers/organisations_controller.rb

@@ -114,8 +114,7 @@ class OrganisationsController < ApplicationController
   end
 
   def email_lettings_csv
-    codes_only_export = params.require(:codes_only) == "true"
-    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export)
+    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export?)
     redirect_to lettings_logs_csv_confirmation_organisation_path
   end
 
@@ -148,8 +147,7 @@ class OrganisationsController < ApplicationController
   end
 
   def email_sales_csv
-    codes_only_export = params.require(:codes_only) == "true"
-    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export, "sales")
+    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export?, "sales")
     redirect_to sales_logs_csv_confirmation_organisation_path
   end
 
@@ -163,6 +161,10 @@ private
     params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no)
   end
 
+  def codes_only_export?
+    params.require(:codes_only) == "true"
+  end
+
   def search_term
     params["search"]
   end

app/helpers/logs_helper.rb

@@ -1,10 +1,8 @@
 module LogsHelper
   def log_type_for_controller(controller)
-    case controller.class.to_s
-    when "LettingsLogsController"
-      "lettings"
-    when "SalesLogsController"
-      "sales"
+    case controller.class.name
+    when "LettingsLogsController" then "lettings"
+    when "SalesLogsController" then "sales"
     else
       raise "Log type not found for #{controller.class}"
     end
@@ -12,10 +10,8 @@ module LogsHelper
 
   def bulk_upload_path_for_controller(controller, id:)
     case log_type_for_controller(controller)
-    when "lettings"
-      bulk_upload_lettings_log_path(id:)
-    when "sales"
-      bulk_upload_sales_log_path(id:)
+    when "lettings" then bulk_upload_lettings_log_path(id:)
+    when "sales" then bulk_upload_sales_log_path(id:)
     end
   end
 
@@ -26,25 +22,29 @@ module LogsHelper
 
   def search_label_for_controller(controller)
     case log_type_for_controller(controller)
-    when "lettings"
-      "Search by log ID, tenant code, property reference or postcode"
-    when "sales"
-      "Search by log ID, purchaser code or postcode"
+    when "lettings" then "Search by log ID, tenant code, property reference or postcode"
+    when "sales" then "Search by log ID, purchaser code or postcode"
     end
   end
 
-  def csv_download_url_for_controller(controller_type:, search:, codes_only:)
-    case log_type_for_controller(controller_type)
+  def csv_download_url_for_controller(controller:, search:, codes_only:)
+    case log_type_for_controller(controller)
     when "lettings" then csv_download_lettings_logs_path(search:, codes_only:)
     when "sales" then csv_download_sales_logs_path(search:, codes_only:)
     end
   end
 
+  def logs_path_for_controller(controller)
+    case log_type_for_controller(controller)
+    when "lettings" then lettings_logs_path
+    when "sales" then sales_logs_path
+    end
+  end
+
   def csv_download_url_by_log_type(log_type, organisation, search:, codes_only:)
-    if log_type == :lettings
-      lettings_logs_csv_download_organisation_path(organisation, search:, codes_only:)
-    else
-      sales_logs_csv_download_organisation_path(organisation, search:, codes_only:)
+    case log_type
+    when :lettings then lettings_logs_csv_download_organisation_path(organisation, search:, codes_only:)
+    when :sales then sales_logs_csv_download_organisation_path(organisation, search:, codes_only:)
     end
   end
 end

app/jobs/email_csv_job.rb

@@ -6,11 +6,12 @@ class EmailCsvJob < ApplicationJob
   EXPIRATION_TIME = 3.hours.to_i
 
   def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, codes_only_export = false, log_type = "lettings") # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params
-    if log_type == "lettings"
+    case log_type
+    when "lettings"
       unfiltered_logs = organisation.present? && user.support? ? LettingsLog.visible.where(owning_organisation_id: organisation.id) : user.lettings_logs.visible
       filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user)
       csv_string = filtered_logs.to_csv(user, codes_only_export:)
-    else
+    when "sales"
       unfiltered_logs = organisation.present? && user.support? ? SalesLog.visible.where(owning_organisation_id: organisation.id) : user.sales_logs.visible
       filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user)
       export_type = codes_only_export ? "codes" : "labels"

app/views/logs/csv_confirmation.html.erb

@@ -9,8 +9,7 @@
     <p class="govuk-body">Open your email inbox and click the link to download your CSV file.</p>
 
     <p class="govuk-body">
-      <% logs_path = controller.class.name.start_with?("Lettings") ? lettings_logs_path : sales_logs_path %>
-      <%= govuk_link_to "Return to logs", logs_path %>
+      <%= govuk_link_to "Return to logs", logs_path_for_controller(controller) %>
     </p>
   </div>
 </div>

app/views/logs/index.html.erb

@@ -78,8 +78,8 @@
                  searched: @searched,
                  item_label:,
                  total_count: @total_count,
-                 csv_download_url: csv_download_url_for_controller(controller_type: controller, search: @search_term, codes_only: false),
-                 csv_codes_only_download_url: csv_download_url_for_controller(controller_type: controller, search: @search_term, codes_only: true),
+                 csv_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: false),
+                 csv_codes_only_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: true),
                } %>
     <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
   </div>