From 2fd65c64502206a744df63c965db495417064d2e Mon Sep 17 00:00:00 2001
From: Jack S <jacopo.scotti@softwire.com>
Date: Fri, 12 May 2023 15:25:25 +0100
Subject: [PATCH] Move actions to helper

---
 app/helpers/log_actions_helper.rb           | 42 +++++++++++++++++++++
 app/policies/log_policy.rb                  |  2 +
 app/views/logs/delete_confirmation.html.erb |  2 +-
 app/views/logs/edit.html.erb                | 22 +----------
 spec/policies/log_policy_spec.rb            | 24 +++++++++++-
 5 files changed, 68 insertions(+), 24 deletions(-)
 create mode 100644 app/helpers/log_actions_helper.rb

diff --git a/app/helpers/log_actions_helper.rb b/app/helpers/log_actions_helper.rb
new file mode 100644
index 000000000..daca9dd30
--- /dev/null
+++ b/app/helpers/log_actions_helper.rb
@@ -0,0 +1,42 @@
+module LogActionsHelper
+  include GovukLinkHelper
+
+  def edit_actions_for_log(log)
+    back = back_button_for(log)
+    delete = delete_button_for_log(log)
+
+    return if back.nil? && delete.nil?
+
+    content_tag(:div, class: "govuk-button-group") do
+      safe_join([back, delete])
+    end
+  end
+
+private
+
+  def back_button_for(log)
+    if log.completed?
+      if log.bulk_uploaded?
+        if log.lettings?
+          govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(log.bulk_upload)
+        else
+          govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_sales_result_path(log.bulk_upload)
+        end
+      elsif log.lettings?
+        govuk_button_link_to "Back to lettings logs", lettings_logs_path
+      elsif log.sales?
+        govuk_button_link_to "Back to sales logs", sales_logs_path
+      end
+    end
+  end
+
+  def delete_button_for_log(log)
+    if LogPolicy.new(current_user, log).destroy?
+      govuk_button_link_to(
+        "Delete log",
+        lettings_log_delete_confirmation_path(log),
+        warning: true,
+      )
+    end
+  end
+end
diff --git a/app/policies/log_policy.rb b/app/policies/log_policy.rb
index 80c7dceb8..ced6a32a7 100644
--- a/app/policies/log_policy.rb
+++ b/app/policies/log_policy.rb
@@ -7,6 +7,8 @@ class LogPolicy
   end
 
   def destroy?
+    return false unless log && user
+
     # Return false if the log is not editable.
     return false unless log.collection_period_open?
 
diff --git a/app/views/logs/delete_confirmation.html.erb b/app/views/logs/delete_confirmation.html.erb
index 34105943f..247e6b1ba 100644
--- a/app/views/logs/delete_confirmation.html.erb
+++ b/app/views/logs/delete_confirmation.html.erb
@@ -16,7 +16,7 @@
       <%= govuk_button_to(
         "Delete this log",
         @log.lettings? ? lettings_log_path(@log) : sales_log_path(@log),
-        method: :delete
+        method: :delete,
       ) %>
       <%= govuk_button_link_to(
         "Cancel",
diff --git a/app/views/logs/edit.html.erb b/app/views/logs/edit.html.erb
index 70777b357..170aefe4d 100644
--- a/app/views/logs/edit.html.erb
+++ b/app/views/logs/edit.html.erb
@@ -38,26 +38,6 @@
 
     <%= render "tasklist" %>
 
-    <% if @log.completed? %>
-      <% if @log.bulk_uploaded? %>
-        <% if @log.lettings? %>
-          <%= govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(@log.bulk_upload) %>
-        <% else %>
-          <%= govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_sales_result_path(@log.bulk_upload) %>
-        <% end %>
-      <% elsif @log.lettings? %>
-        <%= govuk_button_link_to "Back to lettings logs", lettings_logs_path %>
-      <% elsif @log.sales? %>
-        <%= govuk_button_link_to "Back to sales logs", sales_logs_path %>
-      <% end %>
-    <% end %>
-
-    <% if LogPolicy.new(current_user, @log).destroy? %>
-      <%= govuk_button_link_to(
-        "Delete log",
-        lettings_log_delete_confirmation_path(@log),
-        warning: true,
-      ) %>
-    <% end %>
+    <%= edit_actions_for_log(@log) %>
   </div>
 </div>
diff --git a/spec/policies/log_policy_spec.rb b/spec/policies/log_policy_spec.rb
index 8d219535c..39bef7467 100644
--- a/spec/policies/log_policy_spec.rb
+++ b/spec/policies/log_policy_spec.rb
@@ -6,6 +6,26 @@ RSpec.describe LogPolicy do
   permissions :destroy? do
     let(:log) { create(:lettings_log, :setup_completed) }
 
+    context "when log nil" do
+      before do
+        allow(log).to receive(:collection_period_open?).and_return(false)
+      end
+
+      it "does not allow deletion of log" do
+        expect(policy).not_to permit(build(:user, :support), nil)
+      end
+    end
+
+    context "when user nil" do
+      before do
+        allow(log).to receive(:collection_period_open?).and_return(false)
+      end
+
+      it "does not allow deletion of log" do
+        expect(policy).not_to permit(nil, build(:lettings_log, :setup_completed))
+      end
+    end
+
     context "when collection period closed" do
       before do
         allow(log).to receive(:collection_period_open?).and_return(false)
@@ -14,7 +34,7 @@ RSpec.describe LogPolicy do
       it "does not allow deletion of log" do
         expect(log).to receive(:collection_period_open?)
 
-        expect(policy).not_to permit(nil, log)
+        expect(policy).not_to permit(build(:user, :support), log)
       end
     end
 
@@ -32,7 +52,7 @@ RSpec.describe LogPolicy do
           expect(log).to receive(:setup_completed?)
           expect(log).to receive(:collection_period_open?)
 
-          expect(policy).not_to permit(nil, log)
+          expect(policy).not_to permit(build(:user, :support), log)
         end
       end