Allow deletion of delete and in progress logs

app/models/log.rb

@@ -166,10 +166,6 @@ class Log < ApplicationRecord
     bulk_upload_id.present?
   end
 
-  def setup_completed?
-    form.setup_sections.all? { |sections| sections.subsections.all? { |subsection| subsection.status(self) == :completed } }
-  end
-
 private
 
   # Handle logs that are older than previous collection start date

app/models/sales_log.rb

@@ -103,6 +103,10 @@ class SalesLog < Log
     collection_start_year < 2023
   end
 
+  def setup_completed?
+    form.setup_sections.all? { |sections| sections.subsections.all? { |subsection| subsection.status(self) == :completed } }
+  end
+
   def unresolved
     false
   end

app/policies/log_policy.rb

@@ -9,16 +9,16 @@ class LogPolicy
   def destroy?
     return false unless log && user
 
-    # Return false if the log is not editable.
+    # Can only delete editable logs
     return false unless log.collection_period_open?
 
-    # This button should not appear if the Set up section is not started.
-    return false unless log.setup_completed?
+    # Only delete logs with answered questions
+    return false unless log.in_progress? || log.completed?
 
-    # Data coordinators and support users can see this button on any log.
+    # Data coordinators and support users can delete any log
     return true if user.data_coordinator? || user.support?
 
-    # Data providers can only see this button if the log is assigned to them, even if it belongs to a parent org.
+    # Data providers can only delete the log if it is assigned to them
     log.created_by == user
   end
 end

spec/policies/log_policy_spec.rb

@@ -4,7 +4,7 @@ RSpec.describe LogPolicy do
   subject(:policy) { described_class }
 
   permissions :destroy? do
-    let(:log) { create(:lettings_log, :setup_completed) }
+    let(:log) { create(:lettings_log, :in_progress) }
 
     context "when log nil" do
       before do
@@ -22,7 +22,7 @@ RSpec.describe LogPolicy do
       end
 
       it "does not allow deletion of log" do
-        expect(policy).not_to permit(nil, build(:lettings_log, :setup_completed))
+        expect(policy).not_to permit(nil, build(:lettings_log, :in_progress))
       end
     end
 
@@ -43,64 +43,63 @@ RSpec.describe LogPolicy do
         allow(log).to receive(:collection_period_open?).and_return(true)
       end
 
-      context "when setup_completed false" do
+      context "when not started" do
         before do
-          allow(log).to receive(:setup_completed?).and_return(false)
+          allow(log).to receive(:in_progress?).and_return(false)
+          allow(log).to receive(:completed?).and_return(false)
         end
 
         it "does not allow deletion of log" do
-          expect(log).to receive(:setup_completed?)
+          expect(log).to receive(:in_progress?)
           expect(log).to receive(:collection_period_open?)
 
           expect(policy).not_to permit(build(:user, :support), log)
         end
       end
 
-      context "when setup_completed true" do
-        before do
-          allow(log).to receive(:setup_completed?).and_return(true)
-        end
-
-        context "when user is data coordinator" do
-          let(:user) { create(:user, :data_coordinator) }
+      [
+        %i[lettings_log in_progress],
+        %i[lettings_log completed],
+        %i[sales_log in_progress],
+        %i[sales_log completed],
+      ].each do |type, status|
+        let(:log) { create(type, status) }
+        context "when #{type} status: #{status}" do
+          context "when user is data coordinator" do
+            let(:user) { create(:user, :data_coordinator) }
 
-          it "does allow deletion of log" do
-            expect(log).to receive(:setup_completed?)
-            expect(log).to receive(:collection_period_open?)
+            it "does allow deletion of log" do
+              expect(log).to receive(:collection_period_open?)
 
-            expect(policy).to permit(user, log)
+              expect(policy).to permit(user, log)
+            end
           end
-        end
 
-        context "when user is support" do
-          let(:user) { create(:user, :support) }
+          context "when user is support" do
+            let(:user) { create(:user, :support) }
 
-          it "does allow deletion of log" do
-            expect(log).to receive(:setup_completed?)
-            expect(log).to receive(:collection_period_open?)
+            it "does allow deletion of log" do
+              expect(log).to receive(:collection_period_open?)
 
-            expect(policy).to permit(user, log)
+              expect(policy).to permit(user, log)
+            end
           end
-        end
 
-        context "when user is data provider" do
-          let(:user) { create(:user) }
+          context "when user is data provider" do
+            let(:user) { create(:user) }
 
-          it "does not allow deletion of log" do
-            expect(log).to receive(:setup_completed?)
-            expect(log).to receive(:collection_period_open?)
-
-            expect(policy).not_to permit(user, log)
-          end
+            it "does not allow deletion of log" do
+              expect(log).to receive(:collection_period_open?)
 
-          context "when the log is assigned to the user" do
-            let(:log) { create(:lettings_log, :setup_completed, created_by: user) }
+              expect(policy).not_to permit(user, log)
+            end
 
-            it "does allow deletion of log" do
-              expect(log).to receive(:setup_completed?)
-              expect(log).to receive(:collection_period_open?)
+            context "when the log is assigned to the user" do
+              let(:log) { create(:lettings_log, :in_progress, created_by: user) }
 
-              expect(policy).to permit(user, log)
+              it "does allow deletion of log" do
+                expect(policy).to permit(user, log)
+              end
             end
           end
         end