From 43173da579c8351851767f4911adccce0ef9d3d4 Mon Sep 17 00:00:00 2001 From: Jack S Date: Mon, 15 May 2023 11:34:39 +0100 Subject: [PATCH] Allow deletion of delete and in progress logs --- app/models/log.rb | 4 -- app/models/sales_log.rb | 4 ++ app/policies/log_policy.rb | 10 ++--- spec/policies/log_policy_spec.rb | 75 ++++++++++++++++---------------- 4 files changed, 46 insertions(+), 47 deletions(-) diff --git a/app/models/log.rb b/app/models/log.rb index 55f4ae33d..6ef5dabf5 100644 --- a/app/models/log.rb +++ b/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 diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 243b6bd09..70ac41535 100644 --- a/app/models/sales_log.rb +++ b/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 diff --git a/app/policies/log_policy.rb b/app/policies/log_policy.rb index ced6a32a7..4ade0ad03 100644 --- a/app/policies/log_policy.rb +++ b/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 diff --git a/spec/policies/log_policy_spec.rb b/spec/policies/log_policy_spec.rb index 39bef7467..1bedfa300 100644 --- a/spec/policies/log_policy_spec.rb +++ b/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