From 9c766025c4ae2e47722e8efb595f5cb61ca1dbe7 Mon Sep 17 00:00:00 2001
From: Jack S <jacopo.scotti@softwire.com>
Date: Tue, 16 May 2023 07:24:25 +0100
Subject: [PATCH] Address comments

---
 app/controllers/lettings_logs_controller.rb | 32 ++++++++++-----------
 app/controllers/sales_logs_controller.rb    | 22 ++++++--------
 app/policies/lettings_log_policy.rb         |  2 ++
 app/policies/sales_log_policy.rb            |  2 ++
 4 files changed, 27 insertions(+), 31 deletions(-)
 create mode 100644 app/policies/lettings_log_policy.rb
 create mode 100644 app/policies/sales_log_policy.rb

diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb
index e20fad70a..a3ad65170 100644
--- a/app/controllers/lettings_logs_controller.rb
+++ b/app/controllers/lettings_logs_controller.rb
@@ -1,5 +1,8 @@
 class LettingsLogsController < LogsController
-  before_action :find_resource, except: %i[create index edit delete_confirmation]
+  rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
+
+  before_action :find_resource, only: %i[update show]
+
   before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :authenticate_scope!, only: %i[download_csv email_csv]
@@ -55,22 +58,19 @@ class LettingsLogsController < LogsController
   end
 
   def edit
-    @log = current_user.lettings_logs.find_by(id: params[:id])
-    if @log
-      if @log.unresolved
-        redirect_to(send(@log.form.unresolved_log_path, @log))
-      else
-        render("logs/edit", locals: { current_user: })
-      end
+    @log = current_user.lettings_logs.find(params[:id])
+
+    if @log.unresolved
+      redirect_to(send(@log.form.unresolved_log_path, @log))
     else
-      render_not_found
+      render("logs/edit", locals: { current_user: })
     end
   end
 
   def destroy
-    render_not_found and return unless @log
+    @log = LettingsLog.visible.find(params[:id])
 
-    authorize @log, policy_class: LogPolicy
+    authorize @log
 
     @log.discard!
 
@@ -78,11 +78,9 @@ class LettingsLogsController < LogsController
   end
 
   def delete_confirmation
-    @log = LettingsLog.visible.find_by(id: params[:lettings_log_id])
+    @log = LettingsLog.visible.find(params[:lettings_log_id])
 
-    render_not_found and return unless @log
-
-    authorize @log, :destroy?, policy_class: LogPolicy
+    authorize @log, :destroy?
 
     render "logs/delete_confirmation"
   end
@@ -113,14 +111,14 @@ class LettingsLogsController < LogsController
     end
   end
 
+private
+
   def org_params
     super.merge(
       { "managing_organisation_id" => current_user.organisation.id },
     )
   end
 
-private
-
   def authenticate_scope!
     head :unauthorized and return if codes_only_export? && !current_user.support?
   end
diff --git a/app/controllers/sales_logs_controller.rb b/app/controllers/sales_logs_controller.rb
index a9f86a1f9..7ed52be2d 100644
--- a/app/controllers/sales_logs_controller.rb
+++ b/app/controllers/sales_logs_controller.rb
@@ -1,4 +1,6 @@
 class SalesLogsController < LogsController
+  rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
+
   before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :authenticate_scope!, only: %i[download_csv email_csv]
@@ -32,20 +34,14 @@ class SalesLogsController < LogsController
   end
 
   def edit
-    @log = current_user.sales_logs.visible.find_by(id: params[:id])
-    if @log
-      render "logs/edit", locals: { current_user: }
-    else
-      render_not_found
-    end
+    @log = current_user.sales_logs.visible.find(params[:id])
+    render "logs/edit", locals: { current_user: }
   end
 
   def destroy
-    @log = SalesLog.visible.find_by(id: params[:id])
+    @log = SalesLog.visible.find(params[:id])
 
-    render_not_found and return unless @log
-
-    authorize @log, policy_class: LogPolicy
+    authorize @log
 
     @log.discard!
 
@@ -53,11 +49,9 @@ class SalesLogsController < LogsController
   end
 
   def delete_confirmation
-    @log = SalesLog.visible.find_by(id: params[:sales_log_id])
-
-    render_not_found and return unless @log
+    @log = SalesLog.visible.find(params[:sales_log_id])
 
-    authorize @log, :destroy?, policy_class: LogPolicy
+    authorize @log, :destroy?
 
     render "logs/delete_confirmation"
   end
diff --git a/app/policies/lettings_log_policy.rb b/app/policies/lettings_log_policy.rb
new file mode 100644
index 000000000..f340b56ce
--- /dev/null
+++ b/app/policies/lettings_log_policy.rb
@@ -0,0 +1,2 @@
+class LettingsLogPolicy < LogPolicy
+end
diff --git a/app/policies/sales_log_policy.rb b/app/policies/sales_log_policy.rb
new file mode 100644
index 000000000..61de31f31
--- /dev/null
+++ b/app/policies/sales_log_policy.rb
@@ -0,0 +1,2 @@
+class SalesLogPolicy < LogPolicy
+end