From 0994794b251c12167880a3d15f3ab387b8b919a7 Mon Sep 17 00:00:00 2001
From: Rachael Booth <Rachael.Booth@softwire.com>
Date: Thu, 4 Jul 2024 14:35:21 +0100
Subject: [PATCH 01/11] CLDC-3543: Task to handle unpended logs - dry runs only
 (#2494)

* CLDC-3543: Task to handle unpended logs

* Enforce dry run only
---
 lib/tasks/handle_unpended_logs.rake | 162 ++++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)
 create mode 100644 lib/tasks/handle_unpended_logs.rake

diff --git a/lib/tasks/handle_unpended_logs.rake b/lib/tasks/handle_unpended_logs.rake
new file mode 100644
index 000000000..fda99687e
--- /dev/null
+++ b/lib/tasks/handle_unpended_logs.rake
@@ -0,0 +1,162 @@
+desc "Deduplicates logs where we have inadvertently turned some pending logs to in progress / completed"
+task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, _args|
+  dry_run = true # args[:perform_updates].blank? || args[:perform_updates] != "true"
+
+  pg = ActiveRecord::Base.connection
+  query = "SELECT \"versions\".* FROM \"versions\" WHERE \"versions\".\"item_type\" = 'LettingsLog' AND whodunnit is null AND ((object_changes like '%status:\n- 3\n- 1%') OR (object_changes like '%status:\n- 3\n- 2%'))"
+  results = pg.execute(query)
+
+  duplicate_log_attributes = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge]
+
+  seen = [].to_set
+
+  output = CSV.generate(headers: true) do |csv|
+    csv << ["Log ID", "Collection Year", "Current Status", "Owning Org", "Owner Id", "Owner Email", "Outcome", "Reason"]
+    results.each do |result|
+      next if result["object_changes"].count("\n") <= 7 && result["object_changes"].include?("status:\n- 3\n- 2")
+
+      id = YAML.safe_load(result["object"], permitted_classes: [Time, BigDecimal])["id"]
+      next if seen.include?(id)
+
+      log = LettingsLog.find(id)
+
+      if log.updated_at != result["created_at"]
+        has_been_manually_updated = false
+        v = log.versions.last
+        while !has_been_manually_updated && v.created_at != result["created_at"]
+          if !v.whodunnit.nil?
+            has_been_manually_updated = true
+          else
+            v = v.previous
+          end
+        end
+
+        if has_been_manually_updated
+          seen.add(id)
+          csv << [id, log.collection_start_year, log.status, log.owning_organisation_name, log.assigned_to_id, log.assigned_to.email, "Leave", "Log updated in UI"]
+          next
+        end
+      end
+
+      # This is the normal query for duplicates but without the check that the logs are visible (i.e. not deleted/pending)
+      duplicates = LettingsLog.where.not(id: log.id)
+                              .where.not(startdate: nil)
+                              .where.not(sex1: nil)
+                              .where.not(ecstat1: nil)
+                              .where.not(needstype: nil)
+                              .age1_answered
+                              .tcharge_answered
+                              .chcharge_answered
+                              .location_for_log_answered(log)
+                              .postcode_for_log_answered(log)
+                              .where(log.slice(*duplicate_log_attributes))
+
+      duplicate_count = duplicates.length
+
+      if duplicate_count.zero?
+        seen.add(id)
+        csv << [id, log.collection_start_year, log.status, log.owning_organisation_name, log.assigned_to_id, log.assigned_to.email, "Leave", "Log has no duplicates"]
+        next
+      end
+
+      visible_duplicates = duplicates.where(status: %w[in_progress completed])
+      deleted_duplicates = duplicates.where(status: %w[deleted])
+
+      if visible_duplicates.length.zero? && deleted_duplicates.any? { |dup| dup.discarded_at > result["created_at"] }
+        seen.add(id)
+        csv << [id, log.collection_start_year, log.status, log.owning_organisation_name, log.assigned_to_id, log.assigned_to.email, "Leave", "Log has no visible duplicates and at least one duplicate has been deleted since being affected"]
+        next
+      end
+
+      if visible_duplicates.length.zero?
+        seen.add(id)
+        csv << [id, log.collection_start_year, log.status, log.owning_organisation_name, log.assigned_to_id, log.assigned_to.email, "Leave", "Log has no visible duplicates"]
+        next
+      end
+
+      unaffected_duplicates = []
+      affected_updated_duplicates = []
+      affected_non_updated_duplicates = [log]
+      visible_duplicates.each do |dup|
+        vquery = "SELECT \"versions\".* FROM \"versions\" WHERE \"versions\".\"item_type\" = 'LettingsLog' AND \"versions\".\"item_id\" = #{dup.id} AND whodunnit is null AND ((object_changes like '%status:\n- 3\n- 1%') OR (object_changes like '%status:\n- 3\n- 2%'))"
+        res = pg.execute(vquery)
+
+        if res.count.zero?
+          unaffected_duplicates.push(dup)
+        elsif res[0]["object_changes"].count("\n") <= 7 && res[0]["object_changes"].include?("status:\n- 3\n- 2")
+          unaffected_duplicates.push(dup)
+        else
+          has_been_manually_updated = false
+          v = dup.versions.last
+          while !has_been_manually_updated && v.created_at != res[0]["created_at"]
+            if !v.whodunnit.nil?
+              has_been_manually_updated = true
+            else
+              v = v.previous
+            end
+          end
+
+          if has_been_manually_updated
+            affected_updated_duplicates.push(dup)
+          else
+            affected_non_updated_duplicates.push(dup)
+          end
+        end
+      end
+
+      unless unaffected_duplicates.empty?
+        unaffected_logs_reference = "log#{unaffected_duplicates.length > 1 ? 's' : ''} #{unaffected_duplicates.map(&:id).join(', ')}"
+        affected_updated_duplicates.each do |d|
+          unless seen.include?(d.id)
+            seen.add(d.id)
+            csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Leave", "Log updated in UI"]
+          end
+        end
+        affected_non_updated_duplicates.each do |d|
+          seen.add(d.id)
+          csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of unaffected log(s)", unaffected_logs_reference]
+          # unless dry_run
+          # d.discard!
+          # end
+        end
+        next
+      end
+
+      unless affected_updated_duplicates.empty?
+        updated_logs_reference = "log#{affected_updated_duplicates.length > 1 ? 's' : ''} #{affected_updated_duplicates.map(&:id).join(', ')}"
+        affected_updated_duplicates.each do |d|
+          unless seen.include?(d.id)
+            seen.add(d.id)
+            csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Leave", "Log updated in UI"]
+          end
+        end
+        affected_non_updated_duplicates.each do |d|
+          seen.add(d.id)
+          csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of log(s) which have been updated since being affected", updated_logs_reference]
+          # unless dry_run
+          # d.discard!
+          # end
+        end
+        next
+      end
+
+      latest_created = affected_non_updated_duplicates.max_by(&:created_at)
+      seen.add(latest_created.id)
+      csv << [latest_created.id, latest_created.collection_start_year, latest_created.status, latest_created.owning_organisation_name, latest_created.assigned_to_id, latest_created.assigned_to.email, "Leave", "Log is the most recently created of a duplicate group"]
+
+      affected_non_updated_duplicates.each do |d|
+        next unless d.id != latest_created.id
+
+        seen.add(d.id)
+        csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of more recently created affected log", latest_created.id]
+        # unless dry_run
+        # d.discard!
+        # end
+      end
+    end
+  end
+
+  s3_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"])
+  output_file = "HandleUnpendedLogs_#{dry_run ? 'dry_run' : ''}_#{Time.zone.now}.csv"
+  s3_service.write_file(output_file, output)
+end

From 905f869460c7f018343b2c6bdcd221a216e61ac1 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Mon, 8 Jul 2024 10:57:25 +0100
Subject: [PATCH 02/11] CLDC-2939 Link to organisations from user pages (#2489)

* Link to organisations from user pages

* Fix test
---
 app/helpers/tab_nav_helper.rb          |  3 ++-
 app/views/users/show.html.erb          |  2 +-
 spec/helpers/tab_nav_helper_spec.rb    |  8 ++++----
 spec/requests/users_controller_spec.rb | 11 ++++++++++-
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb
index 24408be28..27542094d 100644
--- a/app/helpers/tab_nav_helper.rb
+++ b/app/helpers/tab_nav_helper.rb
@@ -19,7 +19,8 @@ module TabNavHelper
   end
 
   def org_cell(user)
+    org_name = current_user.support? ? govuk_link_to(user.organisation.name, lettings_logs_organisation_path(user.organisation)) : user.organisation.name
     role = "<span class=\"app-!-colour-muted\">#{user.role.to_s.humanize}</span>"
-    [user.organisation.name, role].join("\n")
+    [org_name, role].join("\n")
   end
 end
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index c62fccf2c..acb53d51d 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -67,7 +67,7 @@
 
       <%= summary_list.with_row do |row|
             row.with_key { "Organisation" }
-            row.with_value { @user.organisation.name }
+            row.with_value { current_user.support? ? govuk_link_to(@user.organisation.name, lettings_logs_organisation_path(@user.organisation)) : @user.organisation.name }
             row.with_action
           end %>
 
diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb
index 7b4efc5eb..9dbf92e7a 100644
--- a/spec/helpers/tab_nav_helper_spec.rb
+++ b/spec/helpers/tab_nav_helper_spec.rb
@@ -2,21 +2,21 @@ require "rails_helper"
 
 RSpec.describe TabNavHelper do
   let(:organisation) { FactoryBot.create(:organisation) }
-  let(:user) { FactoryBot.build(:user, organisation:) }
+  let(:current_user) { FactoryBot.build(:user, organisation:) }
   let(:scheme) { FactoryBot.create(:scheme, service_name: "Some name") }
   let(:location) { FactoryBot.create(:location, scheme:) }
 
   describe "#user_cell" do
     it "returns user link and email separated by a newline character" do
-      expected_html = "<a class=\"govuk-link\" href=\"/users\">#{user.name}</a>\n<span class=\"govuk-visually-hidden\">User </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{user.email}</span>"
-      expect(user_cell(user)).to match(expected_html)
+      expected_html = "<a class=\"govuk-link\" href=\"/users\">#{current_user.name}</a>\n<span class=\"govuk-visually-hidden\">User </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{current_user.email}</span>"
+      expect(user_cell(current_user)).to match(expected_html)
     end
   end
 
   describe "#org_cell" do
     it "returns the users org name and role separated by a newline character" do
       expected_html = "DLUHC\n<span class=\"app-!-colour-muted\">Data provider</span>"
-      expect(org_cell(user)).to match(expected_html)
+      expect(org_cell(current_user)).to match(expected_html)
     end
   end
 
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 9784aa0dc..bb0a1cca3 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -1188,7 +1188,7 @@ RSpec.describe UsersController, type: :request do
     describe "#index" do
       let!(:other_user) { create(:user, organisation: user.organisation, name: "User 2", email: "other@example.com") }
       let!(:inactive_user) { create(:user, organisation: user.organisation, active: false, name: "User 3", email: "inactive@example.com", last_sign_in_at: Time.zone.local(2022, 10, 10)) }
-      let!(:other_org_user) { create(:user, name: "User 4", email: "otherorg@otherexample.com", organisation: create(:organisation, :without_dpc)) }
+      let!(:other_org_user) { create(:user, name: "User 4", email: "otherorg@otherexample.com", organisation: create(:organisation, :without_dpc, name: "Other name")) }
 
       before do
         get "/users", headers:, params: {}
@@ -1201,6 +1201,11 @@ RSpec.describe UsersController, type: :request do
         expect(page).to have_content(other_org_user.name)
       end
 
+      it "links to user organisations" do
+        expect(page).to have_link(user.organisation.name, href: "/organisations/#{user.organisation.id}/lettings-logs", count: 3)
+        expect(page).to have_link(other_org_user.organisation.name, href: "/organisations/#{other_org_user.organisation.id}/lettings-logs", count: 1)
+      end
+
       it "shows last logged in date for all users" do
         expect(page).to have_content("10 October 2022")
       end
@@ -1432,6 +1437,10 @@ RSpec.describe UsersController, type: :request do
             expect(page).to have_link("Change", text: "if a key contact")
           end
 
+          it "links to user organisation" do
+            expect(page).to have_link(other_user.organisation.name, href: "/organisations/#{other_user.organisation.id}/lettings-logs")
+          end
+
           it "does not show option to resend confirmation email" do
             expect(page).not_to have_button("Resend invite link")
           end

From f6a0cc67e5c0dfd30dfc2b03eb5f89756fd01847 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Mon, 8 Jul 2024 15:58:01 +0100
Subject: [PATCH 03/11] Bump rails_admin from 3.1.2 to 3.1.3 (#2500)

Bumps [rails_admin](https://github.com/sferik/rails_admin) from 3.1.2 to 3.1.3.
- [Changelog](https://github.com/railsadminteam/rails_admin/blob/v3.1.3/CHANGELOG.md)
- [Commits](https://github.com/sferik/rails_admin/compare/v3.1.2...v3.1.3)

---
updated-dependencies:
- dependency-name: rails_admin
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
---
 Gemfile      |  2 +-
 Gemfile.lock | 22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Gemfile b/Gemfile
index 412769b2e..f43a1b7d9 100644
--- a/Gemfile
+++ b/Gemfile
@@ -63,7 +63,7 @@ gem "possessive"
 gem "auto_strip_attributes"
 # Use sidekiq for background processing
 gem "method_source", "~> 1.1"
-gem "rails_admin", "~> 3.0"
+gem "rails_admin", "~> 3.1"
 gem "ruby-openai"
 gem "sidekiq"
 gem "sidekiq-cron"
diff --git a/Gemfile.lock b/Gemfile.lock
index 8f9ea9b52..5c8f48017 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -104,7 +104,7 @@ GEM
     bindex (0.8.1)
     bootsnap (1.18.3)
       msgpack (~> 1.2)
-    builder (3.2.4)
+    builder (3.3.0)
     bundler-audit (0.9.1)
       bundler (>= 1.2.0, < 3)
       thor (~> 1.0)
@@ -128,7 +128,7 @@ GEM
       launchy
     childprocess (5.0.0)
     coderay (1.1.3)
-    concurrent-ruby (1.3.1)
+    concurrent-ruby (1.3.3)
     connection_pool (2.4.1)
     crack (1.0.0)
       bigdecimal
@@ -163,7 +163,7 @@ GEM
       rainbow
       rubocop
       smart_properties
-    erubi (1.12.0)
+    erubi (1.13.0)
     et-orbi (1.2.7)
       tzinfo
     event_stream_parser (1.0.0)
@@ -241,13 +241,13 @@ GEM
     matrix (0.4.2)
     method_source (1.1.0)
     mini_mime (1.1.5)
-    minitest (5.23.1)
+    minitest (5.24.1)
     msgpack (1.7.2)
     multipart-post (2.4.1)
     nested_form (0.3.2)
     net-http (0.4.1)
       uri
-    net-imap (0.4.12)
+    net-imap (0.4.14)
       date
       net-protocol
     net-pop (0.1.2)
@@ -257,11 +257,11 @@ GEM
     net-smtp (0.5.0)
       net-protocol
     nio4r (2.7.3)
-    nokogiri (1.16.5-arm64-darwin)
+    nokogiri (1.16.6-arm64-darwin)
       racc (~> 1.4)
-    nokogiri (1.16.5-x86_64-darwin)
+    nokogiri (1.16.6-x86_64-darwin)
       racc (~> 1.4)
-    nokogiri (1.16.5-x86_64-linux)
+    nokogiri (1.16.6-x86_64-linux)
       racc (~> 1.4)
     notifications-ruby-client (6.0.0)
       jwt (>= 1.5, < 3)
@@ -333,7 +333,7 @@ GEM
     rails-html-sanitizer (1.6.0)
       loofah (~> 2.21)
       nokogiri (~> 1.14)
-    rails_admin (3.1.2)
+    rails_admin (3.1.3)
       activemodel-serializers-xml (>= 1.0)
       kaminari (>= 0.14, < 2.0)
       nested_form (~> 0.3)
@@ -484,7 +484,7 @@ GEM
     websocket-extensions (0.1.5)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.6.15)
+    zeitwerk (2.6.16)
 
 PLATFORMS
   arm64-darwin-21
@@ -535,7 +535,7 @@ DEPENDENCIES
   rack-attack
   rack-mini-profiler (~> 2.0)
   rails (~> 7.0.8.3)
-  rails_admin (~> 3.0)
+  rails_admin (~> 3.1)
   redis (~> 4.8)
   roo
   rspec-rails

From ababb3894ca1931d5ade6b93f7a5d1c20e699d54 Mon Sep 17 00:00:00 2001
From: Rachael Booth <Rachael.Booth@softwire.com>
Date: Wed, 10 Jul 2024 10:25:50 +0100
Subject: [PATCH 04/11] CLDC-3543: Allow live run of handle unpended logs task
 (#2496)

---
 lib/tasks/handle_unpended_logs.rake | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/tasks/handle_unpended_logs.rake b/lib/tasks/handle_unpended_logs.rake
index fda99687e..02c976682 100644
--- a/lib/tasks/handle_unpended_logs.rake
+++ b/lib/tasks/handle_unpended_logs.rake
@@ -1,6 +1,6 @@
 desc "Deduplicates logs where we have inadvertently turned some pending logs to in progress / completed"
 task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, _args|
-  dry_run = true # args[:perform_updates].blank? || args[:perform_updates] != "true"
+  dry_run = args[:perform_updates].blank? || args[:perform_updates] != "true"
 
   pg = ActiveRecord::Base.connection
   query = "SELECT \"versions\".* FROM \"versions\" WHERE \"versions\".\"item_type\" = 'LettingsLog' AND whodunnit is null AND ((object_changes like '%status:\n- 3\n- 1%') OR (object_changes like '%status:\n- 3\n- 2%'))"
@@ -115,9 +115,9 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, _args
         affected_non_updated_duplicates.each do |d|
           seen.add(d.id)
           csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of unaffected log(s)", unaffected_logs_reference]
-          # unless dry_run
-          # d.discard!
-          # end
+          unless dry_run
+            d.discard!
+          end
         end
         next
       end
@@ -133,9 +133,9 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, _args
         affected_non_updated_duplicates.each do |d|
           seen.add(d.id)
           csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of log(s) which have been updated since being affected", updated_logs_reference]
-          # unless dry_run
-          # d.discard!
-          # end
+          unless dry_run
+            d.discard!
+          end
         end
         next
       end
@@ -149,9 +149,9 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, _args
 
         seen.add(d.id)
         csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of more recently created affected log", latest_created.id]
-        # unless dry_run
-        # d.discard!
-        # end
+        unless dry_run
+          d.discard!
+        end
       end
     end
   end

From b553ec031aff8f279d44247df341a43ac7d938f1 Mon Sep 17 00:00:00 2001
From: Rachael Booth <Rachael.Booth@softwire.com>
Date: Wed, 10 Jul 2024 12:16:06 +0100
Subject: [PATCH 05/11] CLDC-3543: Fix args reference in allow live run of
 handle unpended logs task (#2503)

---
 lib/tasks/handle_unpended_logs.rake | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tasks/handle_unpended_logs.rake b/lib/tasks/handle_unpended_logs.rake
index 02c976682..695e6d93f 100644
--- a/lib/tasks/handle_unpended_logs.rake
+++ b/lib/tasks/handle_unpended_logs.rake
@@ -1,5 +1,5 @@
 desc "Deduplicates logs where we have inadvertently turned some pending logs to in progress / completed"
-task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, _args|
+task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args|
   dry_run = args[:perform_updates].blank? || args[:perform_updates] != "true"
 
   pg = ActiveRecord::Base.connection

From 8d9f30610ccd626afa9e618ecdfab17e42ed29ab Mon Sep 17 00:00:00 2001
From: Rachael Booth <Rachael.Booth@softwire.com>
Date: Wed, 10 Jul 2024 15:17:14 +0100
Subject: [PATCH 06/11] CLDC-3543: Don't validate logs when discarding them in
 handle unpended logs task (#2504)

---
 lib/tasks/handle_unpended_logs.rake | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/tasks/handle_unpended_logs.rake b/lib/tasks/handle_unpended_logs.rake
index 695e6d93f..d3837ee06 100644
--- a/lib/tasks/handle_unpended_logs.rake
+++ b/lib/tasks/handle_unpended_logs.rake
@@ -115,8 +115,11 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args|
         affected_non_updated_duplicates.each do |d|
           seen.add(d.id)
           csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of unaffected log(s)", unaffected_logs_reference]
+          # rubocop:disable Style/Next
           unless dry_run
-            d.discard!
+            d.discarded_at = Time.zone.now
+            d.status = "deleted"
+            d.save!(validate: false)
           end
         end
         next
@@ -134,7 +137,9 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args|
           seen.add(d.id)
           csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of log(s) which have been updated since being affected", updated_logs_reference]
           unless dry_run
-            d.discard!
+            d.discarded_at = Time.zone.now
+            d.status = "deleted"
+            d.save!(validate: false)
           end
         end
         next
@@ -150,8 +155,11 @@ task :handle_unpended_logs, %i[perform_updates] => :environment do |_task, args|
         seen.add(d.id)
         csv << [d.id, d.collection_start_year, d.status, d.owning_organisation_name, d.assigned_to_id, d.assigned_to.email, "Delete", "Log is a duplicate of more recently created affected log", latest_created.id]
         unless dry_run
-          d.discard!
+          d.discarded_at = Time.zone.now
+          d.status = "deleted"
+          d.save!(validate: false)
         end
+        # rubocop:enable Style/Next
       end
     end
   end

From 032cecf3f830cc897e0923bee40c3de6156413fb Mon Sep 17 00:00:00 2001
From: Rachael Booth <Rachael.Booth@softwire.com>
Date: Wed, 10 Jul 2024 16:26:55 +0100
Subject: [PATCH 07/11] CLDC-3482: Task to recalculate status for 2023 logs
 missing la (#2495)

---
 .../recalculate_status_when_la_missing.rake   | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 lib/tasks/recalculate_status_when_la_missing.rake

diff --git a/lib/tasks/recalculate_status_when_la_missing.rake b/lib/tasks/recalculate_status_when_la_missing.rake
new file mode 100644
index 000000000..16af29cc7
--- /dev/null
+++ b/lib/tasks/recalculate_status_when_la_missing.rake
@@ -0,0 +1,19 @@
+desc "Recalculates status for 2023 completed logs with missing LA"
+task recalculate_status_missing_la: :environment do
+  # See reinfer_local_authority - this covers cases where postcode_full was not set that should have been returned to in progress
+  LettingsLog.filter_by_year(2023).where(needstype: 1, la: nil, status: "completed").find_each do |log|
+    log.status = log.calculate_status
+
+    unless log.save
+      Rails.logger.info "Could not save changes to lettings log #{log.id}"
+    end
+  end
+
+  SalesLog.filter_by_year(2023).where(la: nil, status: "completed").find_each do |log|
+    log.status = log.calculate_status
+
+    unless log.save
+      Rails.logger.info "Could not save changes to sales log #{log.id}"
+    end
+  end
+end

From 356d652db13de264e1debec9c76eb0a72af87bf7 Mon Sep 17 00:00:00 2001
From: Rachael Booth <Rachael.Booth@softwire.com>
Date: Wed, 10 Jul 2024 17:38:15 +0100
Subject: [PATCH 08/11] CLDC-3532: Make BU guidance back link depend on
 referrer (#2482)

* CLDC-3532: Make BU guidance back link depend on referrer

* Add tests for back_path

* Add guidance to homepage

* CLDC-3526: Update back_path

---------

Co-authored-by: Kat <katrina@kosiak.co.uk>
---
 .../bulk_upload_lettings_logs_controller.rb   |  2 +-
 .../bulk_upload_sales_logs_controller.rb      |  2 +-
 .../forms/bulk_upload_lettings/guidance.rb    | 10 ++++-
 .../forms/bulk_upload_sales/guidance.rb       | 10 ++++-
 .../forms/prepare_your_file_2023.html.erb     |  2 +-
 .../forms/prepare_your_file_2024.html.erb     |  2 +-
 .../forms/prepare_your_file_2023.html.erb     |  2 +-
 .../forms/prepare_your_file_2024.html.erb     |  2 +-
 .../layouts/_collection_resources.html.erb    |  1 +
 app/views/start/guidance.html.erb             |  2 +-
 .../bulk_upload_lettings/guidance_spec.rb     | 43 +++++++++++++++++++
 .../forms/bulk_upload_sales/guidance_spec.rb  | 43 +++++++++++++++++++
 12 files changed, 112 insertions(+), 9 deletions(-)
 create mode 100644 spec/models/forms/bulk_upload_lettings/guidance_spec.rb
 create mode 100644 spec/models/forms/bulk_upload_sales/guidance_spec.rb

diff --git a/app/controllers/bulk_upload_lettings_logs_controller.rb b/app/controllers/bulk_upload_lettings_logs_controller.rb
index 4ab636e7a..465bbc5f6 100644
--- a/app/controllers/bulk_upload_lettings_logs_controller.rb
+++ b/app/controllers/bulk_upload_lettings_logs_controller.rb
@@ -47,7 +47,7 @@ private
               when "prepare-your-file"
                 Forms::BulkUploadLettings::PrepareYourFile.new(form_params)
               when "guidance"
-                Forms::BulkUploadLettings::Guidance.new(form_params)
+                Forms::BulkUploadLettings::Guidance.new(form_params.merge(referrer: params[:referrer]))
               when "needstype"
                 Forms::BulkUploadLettings::Needstype.new(form_params)
               when "upload-your-file"
diff --git a/app/controllers/bulk_upload_sales_logs_controller.rb b/app/controllers/bulk_upload_sales_logs_controller.rb
index 18f1e89c8..56bd1d4de 100644
--- a/app/controllers/bulk_upload_sales_logs_controller.rb
+++ b/app/controllers/bulk_upload_sales_logs_controller.rb
@@ -47,7 +47,7 @@ private
               when "prepare-your-file"
                 Forms::BulkUploadSales::PrepareYourFile.new(form_params)
               when "guidance"
-                Forms::BulkUploadSales::Guidance.new(form_params)
+                Forms::BulkUploadSales::Guidance.new(form_params.merge(referrer: params[:referrer]))
               when "upload-your-file"
                 Forms::BulkUploadSales::UploadYourFile.new(form_params.merge(current_user:))
               when "checking-file"
diff --git a/app/models/forms/bulk_upload_lettings/guidance.rb b/app/models/forms/bulk_upload_lettings/guidance.rb
index 66683eb9b..b6cf5bf74 100644
--- a/app/models/forms/bulk_upload_lettings/guidance.rb
+++ b/app/models/forms/bulk_upload_lettings/guidance.rb
@@ -6,13 +6,21 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :referrer
 
       def view_path
         "bulk_upload_shared/guidance"
       end
 
       def back_path
-        bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: })
+        case referrer
+        when "prepare-your-file"
+          bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: })
+        when "home"
+          root_path
+        else
+          guidance_path
+        end
       end
 
       def lettings_legacy_template_path
diff --git a/app/models/forms/bulk_upload_sales/guidance.rb b/app/models/forms/bulk_upload_sales/guidance.rb
index 44987cd87..28ca6c3b5 100644
--- a/app/models/forms/bulk_upload_sales/guidance.rb
+++ b/app/models/forms/bulk_upload_sales/guidance.rb
@@ -6,13 +6,21 @@ module Forms
       include Rails.application.routes.url_helpers
 
       attribute :year, :integer
+      attribute :referrer
 
       def view_path
         "bulk_upload_shared/guidance"
       end
 
       def back_path
-        bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: })
+        case referrer
+        when "prepare-your-file"
+          bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: })
+        when "home"
+          root_path
+        else
+          guidance_path
+        end
       end
 
       def lettings_legacy_template_path
diff --git a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb
index 41564c7b2..21d19dba8 100644
--- a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb
@@ -9,7 +9,7 @@
 
       <span class="govuk-caption-l">Upload lettings logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">Prepare your file</h1>
-      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_lettings_log_path(id: "guidance", form: { year: @form.year }) %> before you start if you have not used bulk upload before.</p>
+      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_lettings_log_path(id: "guidance", form: { year: @form.year }, referrer: "prepare-your-file") %> before you start if you have not used bulk upload before.</p>
 
       <h2 class="govuk-heading-s">Download template</h2>
 
diff --git a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb
index 8bc375450..6413d70cb 100644
--- a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb
+++ b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb
@@ -9,7 +9,7 @@
 
       <span class="govuk-caption-l">Upload lettings logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">Prepare your file</h1>
-      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_lettings_log_path(id: "guidance", form: { year: @form.year }) %> before you start if you have not used bulk upload before.</p>
+      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_lettings_log_path(id: "guidance", form: { year: @form.year }, referrer: "prepare-your-file") %> before you start if you have not used bulk upload before.</p>
 
       <h2 class="govuk-heading-s">Download template</h2>
 
diff --git a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb
index 9b25b67ef..b9d0990be 100644
--- a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb
+++ b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb
@@ -9,7 +9,7 @@
 
       <span class="govuk-caption-l">Upload sales logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">Prepare your file</h1>
-      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_sales_log_path(id: "guidance", form: { year: @form.year }) %> before you start if you have not used bulk upload before.</p>
+      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_sales_log_path(id: "guidance", form: { year: @form.year }, referrer: "prepare-your-file") %> before you start if you have not used bulk upload before.</p>
 
       <h2 class="govuk-heading-s">Download template</h2>
 
diff --git a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb
index 15d63a957..81c947e15 100644
--- a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb
+++ b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb
@@ -9,7 +9,7 @@
 
       <span class="govuk-caption-l">Upload sales logs in bulk (<%= @form.year_combo %>)</span>
       <h1 class="govuk-heading-l">Prepare your file</h1>
-      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_sales_log_path(id: "guidance", form: { year: @form.year }) %> before you start if you have not used bulk upload before.</p>
+      <p class="govuk-body govuk-!-margin-bottom-2"><%= govuk_link_to "Read the full guidance", bulk_upload_sales_log_path(id: "guidance", form: { year: @form.year }, referrer: "prepare-your-file") %> before you start if you have not used bulk upload before.</p>
 
       <h2 class="govuk-heading-s">Download template</h2>
 
diff --git a/app/views/layouts/_collection_resources.html.erb b/app/views/layouts/_collection_resources.html.erb
index bc0e07360..acdc5cb88 100644
--- a/app/views/layouts/_collection_resources.html.erb
+++ b/app/views/layouts/_collection_resources.html.erb
@@ -1,6 +1,7 @@
 <% if current_user %>
   <h1 class="govuk-heading-l">Collection resources</h1>
   <p class="govuk-body"><strong><%= govuk_link_to "Guidance for submitting social housing lettings and sales data (CORE)", guidance_path %></strong></p>
+  <p class="govuk-body"><strong><%= govuk_link_to "How to upload logs in bulk", bulk_upload_lettings_log_path(id: "guidance", form: { year: current_collection_start_year }, referrer: "home") %></strong></p>
 <% else %>
   <h2 class="govuk-heading-m">Collection resources</h2>
 <% end %>
diff --git a/app/views/start/guidance.html.erb b/app/views/start/guidance.html.erb
index 127b56b3b..6fbd157f0 100644
--- a/app/views/start/guidance.html.erb
+++ b/app/views/start/guidance.html.erb
@@ -9,7 +9,7 @@
       <%= accordion.with_section(heading_text: "How to create logs", expanded: true) do %>
         <p class="govuk-body">There are 2 ways to create logs on CORE.</p>
         <p class="govuk-body">You can create logs one at a time by answering questions using the online form. Click the “Create a new log” button on the logs page to create logs this way.</p>
-        <p class="govuk-body">You can also create many logs at once by uploading a CSV file. This might be faster than creating logs individually if your organisation has its own database and a way to export the data. Click the “Upload logs in bulk” button on the logs page to create logs this way. For more information, <%= govuk_link_to "read the full guidance on bulk upload", bulk_upload_lettings_log_path(id: "guidance", form: { year: current_collection_start_year }) %>.</p>
+        <p class="govuk-body">You can also create many logs at once by uploading a CSV file. This might be faster than creating logs individually if your organisation has its own database and a way to export the data. Click the “Upload logs in bulk” button on the logs page to create logs this way. For more information, <%= govuk_link_to "read the full guidance on bulk upload", bulk_upload_lettings_log_path(id: "guidance", form: { year: current_collection_start_year }, referrer: "guidance") %>.</p>
         <p class="govuk-body">Once you have created and completed a log, there is nothing more you need to do to submit the data.</p>
       <% end %>
 
diff --git a/spec/models/forms/bulk_upload_lettings/guidance_spec.rb b/spec/models/forms/bulk_upload_lettings/guidance_spec.rb
new file mode 100644
index 000000000..271ff8b27
--- /dev/null
+++ b/spec/models/forms/bulk_upload_lettings/guidance_spec.rb
@@ -0,0 +1,43 @@
+require "rails_helper"
+
+RSpec.describe Forms::BulkUploadLettings::Guidance do
+  include Rails.application.routes.url_helpers
+
+  subject(:bu_guidance) { described_class.new(year:, referrer:) }
+
+  let(:year) { 2024 }
+
+  describe "#back_path" do
+    context "when referrer is prepare-your-file" do
+      let(:referrer) { "prepare-your-file" }
+
+      it "returns the prepare your file path" do
+        expect(bu_guidance.back_path).to eq bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: })
+      end
+    end
+
+    context "when referrer is home" do
+      let(:referrer) { "home" }
+
+      it "returns the root path" do
+        expect(bu_guidance.back_path).to eq root_path
+      end
+    end
+
+    context "when referrer is guidance" do
+      let(:referrer) { "guidance" }
+
+      it "returns the main guidance page path" do
+        expect(bu_guidance.back_path).to eq guidance_path
+      end
+    end
+
+    context "when referrer is absent" do
+      let(:referrer) { nil }
+
+      it "returns the main guidance page path" do
+        expect(bu_guidance.back_path).to eq guidance_path
+      end
+    end
+  end
+end
diff --git a/spec/models/forms/bulk_upload_sales/guidance_spec.rb b/spec/models/forms/bulk_upload_sales/guidance_spec.rb
new file mode 100644
index 000000000..6eacf6d0c
--- /dev/null
+++ b/spec/models/forms/bulk_upload_sales/guidance_spec.rb
@@ -0,0 +1,43 @@
+require "rails_helper"
+
+RSpec.describe Forms::BulkUploadSales::Guidance do
+  include Rails.application.routes.url_helpers
+
+  subject(:bu_guidance) { described_class.new(year:, referrer:) }
+
+  let(:year) { 2024 }
+
+  describe "#back_path" do
+    context "when referrer is prepare-your-file" do
+      let(:referrer) { "prepare-your-file" }
+
+      it "returns the prepare your file path" do
+        expect(bu_guidance.back_path).to eq bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: })
+      end
+    end
+
+    context "when referrer is home" do
+      let(:referrer) { "home" }
+
+      it "returns the root path" do
+        expect(bu_guidance.back_path).to eq root_path
+      end
+    end
+
+    context "when referrer is guidance" do
+      let(:referrer) { "guidance" }
+
+      it "returns the main guidance page path" do
+        expect(bu_guidance.back_path).to eq guidance_path
+      end
+    end
+
+    context "when referrer is absent" do
+      let(:referrer) { nil }
+
+      it "returns the main guidance page path" do
+        expect(bu_guidance.back_path).to eq guidance_path
+      end
+    end
+  end
+end

From 8dcd80e4f89ac7123d1e01d35ac9f97178125de1 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 11 Jul 2024 09:35:47 +0100
Subject: [PATCH 09/11] Strip scheme ID (#2487)

---
 .../bulk_upload/lettings/year2023/row_parser.rb        |  6 +++---
 .../bulk_upload/lettings/year2024/row_parser.rb        |  2 +-
 .../bulk_upload/lettings/year2023/row_parser_spec.rb   | 10 ++++++++++
 .../bulk_upload/lettings/year2024/row_parser_spec.rb   |  9 +++++++++
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb
index 51f772bcc..1684d22ab 100644
--- a/app/services/bulk_upload/lettings/year2023/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb
@@ -1317,11 +1317,11 @@ private
   end
 
   def log_uses_new_scheme_id?
-    field_16&.start_with?("S")
+    field_16&.strip&.start_with?("S")
   end
 
   def log_uses_old_scheme_id?
-    field_16.present? && !field_16.start_with?("S")
+    field_16.present? && !field_16.strip.start_with?("S")
   end
 
   def scheme_field
@@ -1330,7 +1330,7 @@ private
   end
 
   def scheme_id
-    return field_16 if log_uses_new_scheme_id?
+    return field_16.strip if log_uses_new_scheme_id?
     return field_15 if log_uses_old_scheme_id?
   end
 
diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb
index c124fe09b..16e876384 100644
--- a/app/services/bulk_upload/lettings/year2024/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb
@@ -1358,7 +1358,7 @@ private
   def scheme
     return if field_5.nil? || owning_organisation.nil? || managing_organisation.nil?
 
-    @scheme ||= Scheme.where(id: (owning_organisation.owned_schemes + managing_organisation.owned_schemes).map(&:id)).find_by_id_on_multiple_fields(field_5, field_6)
+    @scheme ||= Scheme.where(id: (owning_organisation.owned_schemes + managing_organisation.owned_schemes).map(&:id)).find_by_id_on_multiple_fields(field_5.strip, field_6)
   end
 
   def location
diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
index ee8675bd3..1d6d16b5f 100644
--- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
@@ -1004,6 +1004,16 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
           end
         end
 
+        context "when scheme ID has leading spaces" do
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "  S#{scheme.id}", field_17: location.id } }
+
+          it "does not return an error" do
+            expect(parser.errors[:field_15]).to be_blank
+            expect(parser.errors[:field_16]).to be_blank
+            expect(parser.errors[:field_17]).to be_blank
+          end
+        end
+
         context "when location exists but not related" do
           let(:other_scheme) { create(:scheme, :with_old_visible_id) }
           let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) }
diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
index 175925cac..e53850d1c 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -936,6 +936,15 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
           end
         end
 
+        context "when scheme ID has leading spaces" do
+          let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: owning_org.old_visible_id, field_4: "2", field_11: "1", field_5: " S#{scheme.id}", field_6: location.id } }
+
+          it "does not return an error" do
+            expect(parser.errors[:field_5]).to be_blank
+            expect(parser.errors[:field_6]).to be_blank
+          end
+        end
+
         context "when location exists but not related" do
           let(:other_scheme) { create(:scheme, :with_old_visible_id) }
           let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) }

From ab4915c2aaf7d91919076c27f9bc837499021489 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 11 Jul 2024 12:01:16 +0100
Subject: [PATCH 10/11] CLDC-2480 Allow deleting organisations (#2459)

* Add delete confirmation page

* Allow deleting organisation

* Add delete button

* Do not display deleted organisations as an option

* Update policy

* Display informative text and delete org resources

* lint

* Update organisation labels to show when deleted

* Refactor organisation label

* Update more labels

* Remove deleted orgs from schemes and autorize delete_confirmation

* Fix tests
---
 .../lettings_log_summary_component.html.erb   |   4 +-
 .../sales_log_summary_component.html.erb      |   4 +-
 app/controllers/organisations_controller.rb   |  31 ++-
 app/helpers/organisations_helper.rb           |  12 +-
 app/helpers/schemes_helper.rb                 |   6 +-
 .../questions/managing_organisation.rb        |  16 +-
 .../form/lettings/questions/stock_owner.rb    |  11 +-
 .../sales/questions/managing_organisation.rb  |  11 +-
 .../sales/questions/owning_organisation_id.rb |  23 +-
 app/models/organisation.rb                    |  12 +
 app/policies/organisation_policy.rb           |  21 ++
 .../delete_confirmation.html.erb              |  24 ++
 app/views/organisations/show.html.erb         |   4 +
 config/locales/en.yml                         |   1 +
 config/routes.rb                              |   2 +
 ...42812_add_discarded_at_to_organisations.rb |   5 +
 db/schema.rb                                  |   3 +-
 .../questions/managing_organisation_spec.rb   |   9 +-
 .../lettings/questions/stock_owner_spec.rb    |  24 +-
 .../questions/managing_organisation_spec.rb   |   4 +
 .../questions/owning_organisation_id_spec.rb  |   9 +-
 spec/policies/organisation_policy_spec.rb     | 108 ++++++++
 .../requests/organisations_controller_spec.rb | 248 ++++++++++++++++++
 23 files changed, 542 insertions(+), 50 deletions(-)
 create mode 100644 app/views/organisations/delete_confirmation.html.erb
 create mode 100644 db/migrate/20240610142812_add_discarded_at_to_organisations.rb

diff --git a/app/components/lettings_log_summary_component.html.erb b/app/components/lettings_log_summary_component.html.erb
index 89fddad20..7686e3559 100644
--- a/app/components/lettings_log_summary_component.html.erb
+++ b/app/components/lettings_log_summary_component.html.erb
@@ -33,13 +33,13 @@
           <% if log.owning_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Owned by</dt>
-              <dd class="app-metadata__definition"><%= log.owning_organisation&.name %></dd>
+              <dd class="app-metadata__definition"><%= log.owning_organisation&.label %></dd>
             </div>
           <% end %>
           <% if log.managing_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Managed by</dt>
-              <dd class="app-metadata__definition"><%= log.managing_organisation&.name %></dd>
+              <dd class="app-metadata__definition"><%= log.managing_organisation&.label %></dd>
             </div>
           <% end %>
         </dl>
diff --git a/app/components/sales_log_summary_component.html.erb b/app/components/sales_log_summary_component.html.erb
index 60a2a7eb5..1d376aa4a 100644
--- a/app/components/sales_log_summary_component.html.erb
+++ b/app/components/sales_log_summary_component.html.erb
@@ -26,13 +26,13 @@
           <% if log.owning_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Owned by</dt>
-              <dd class="app-metadata__definition"><%= log.owning_organisation&.name %></dd>
+              <dd class="app-metadata__definition"><%= log.owning_organisation&.label %></dd>
             </div>
           <% end %>
           <% if log.managing_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Reported by</dt>
-              <dd class="app-metadata__definition"><%= log.managing_organisation&.name %></dd>
+              <dd class="app-metadata__definition"><%= log.managing_organisation&.label %></dd>
             </div>
           <% end %>
         </dl>
diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb
index 357a5f272..138baa78c 100644
--- a/app/controllers/organisations_controller.rb
+++ b/app/controllers/organisations_controller.rb
@@ -15,9 +15,9 @@ class OrganisationsController < ApplicationController
     redirect_to organisation_path(current_user.organisation) unless current_user.support?
 
     all_organisations = Organisation.order(:name)
-    @pagy, @organisations = pagy(filtered_collection(all_organisations, search_term))
+    @pagy, @organisations = pagy(filtered_collection(all_organisations.visible, search_term))
     @searched = search_term.presence
-    @total_count = all_organisations.size
+    @total_count = all_organisations.visible.size
   end
 
   def schemes
@@ -117,7 +117,6 @@ class OrganisationsController < ApplicationController
   end
 
   def update
-    selected_rent_periods = rent_period_params[:rent_periods].compact_blank
     if (current_user.data_coordinator? && org_params[:active].nil?) || current_user.support?
       if @organisation.update(org_params)
         case org_params[:active]
@@ -136,11 +135,14 @@ class OrganisationsController < ApplicationController
         else
           flash[:notice] = I18n.t("organisation.updated")
         end
-        used_rent_periods = @organisation.lettings_logs.pluck(:period).uniq.compact.map(&:to_s)
-        rent_periods_to_delete = rent_period_params[:all_rent_periods] - selected_rent_periods - used_rent_periods
-        OrganisationRentPeriod.transaction do
-          selected_rent_periods.each { |period| OrganisationRentPeriod.create(organisation: @organisation, rent_period: period) }
-          OrganisationRentPeriod.where(organisation: @organisation, rent_period: rent_periods_to_delete).destroy_all
+        if rent_period_params[:rent_periods].present?
+          selected_rent_periods = rent_period_params[:rent_periods].compact_blank
+          used_rent_periods = @organisation.lettings_logs.pluck(:period).uniq.compact.map(&:to_s)
+          rent_periods_to_delete = rent_period_params[:all_rent_periods] - selected_rent_periods - used_rent_periods
+          OrganisationRentPeriod.transaction do
+            selected_rent_periods.each { |period| OrganisationRentPeriod.create(organisation: @organisation, rent_period: period) }
+            OrganisationRentPeriod.where(organisation: @organisation, rent_period: rent_periods_to_delete).destroy_all
+          end
         end
         redirect_to details_organisation_path(@organisation)
       else
@@ -152,6 +154,17 @@ class OrganisationsController < ApplicationController
     end
   end
 
+  def delete
+    authorize @organisation
+
+    @organisation.discard!
+    redirect_to organisations_path, notice: I18n.t("notification.organisation_deleted", name: @organisation.name)
+  end
+
+  def delete_confirmation
+    authorize @organisation
+  end
+
   def lettings_logs
     organisation_logs = LettingsLog.visible.filter_by_organisation(@organisation).filter_by_years_or_nil(FormHandler.instance.years_of_available_lettings_forms)
     unpaginated_filtered_logs = filter_manager.filtered_logs(organisation_logs, search_term, session_filters)
@@ -306,7 +319,7 @@ private
   end
 
   def authenticate_scope!
-    if %w[create new lettings_logs sales_logs download_lettings_csv email_lettings_csv email_sales_csv download_sales_csv].include? action_name
+    if %w[create new lettings_logs sales_logs download_lettings_csv email_lettings_csv email_sales_csv download_sales_csv delete_confirmation delete].include? action_name
       head :unauthorized and return unless current_user.support?
     elsif current_user.organisation != @organisation && !current_user.support?
       render_not_found
diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb
index 7d37b9289..75d5869fa 100644
--- a/app/helpers/organisations_helper.rb
+++ b/app/helpers/organisations_helper.rb
@@ -19,7 +19,7 @@ module OrganisationsHelper
       { name: "Owns housing stock", value: organisation.holds_own_stock ? "Yes" : "No", editable: false },
       { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet },
       { name: "Data Sharing Agreement" },
-      { name: "Status", value: status_tag(organisation.status), editable: false },
+      { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false },
     ]
   end
 
@@ -39,9 +39,19 @@ module OrganisationsHelper
     end
   end
 
+  def delete_organisation_text(organisation)
+    if organisation.active == false && current_user.support? && !OrganisationPolicy.new(current_user, organisation).delete?
+      "<div class=\"app-!-colour-muted\">This organisation was active in an open or editable collection year, and cannot be deleted.</div>".html_safe
+    end
+  end
+
   def rent_periods_with_checked_attr(checked_periods: nil)
     RentPeriod.rent_period_mappings.each_with_object({}) do |(period_code, period_value), result|
       result[period_code] = period_value.merge(checked: checked_periods.nil? || checked_periods.include?(period_code))
     end
   end
+
+  def delete_organisation_link(organisation)
+    govuk_button_link_to "Delete this organisation", delete_confirmation_organisation_path(organisation), warning: true
+  end
 end
diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb
index 1cc89d3d5..f96f9e4c8 100644
--- a/app/helpers/schemes_helper.rb
+++ b/app/helpers/schemes_helper.rb
@@ -20,10 +20,10 @@ module SchemesHelper
   end
 
   def owning_organisation_options(current_user)
-    all_orgs = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) }
+    all_orgs = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) }
     user_org = [OpenStruct.new(id: current_user.organisation_id, name: current_user.organisation.name)]
-    stock_owners = current_user.organisation.stock_owners.map { |org| OpenStruct.new(id: org.id, name: org.name) }
-    merged_organisations = current_user.organisation.absorbed_organisations.merged_during_open_collection_period.map { |org| OpenStruct.new(id: org.id, name: org.name) }
+    stock_owners = current_user.organisation.stock_owners.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) }
+    merged_organisations = current_user.organisation.absorbed_organisations.visible.merged_during_open_collection_period.map { |org| OpenStruct.new(id: org.id, name: org.name) }
     current_user.support? ? all_orgs : user_org + stock_owners + merged_organisations
   end
 
diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb
index ea85ff207..81847876e 100644
--- a/app/models/form/lettings/questions/managing_organisation.rb
+++ b/app/models/form/lettings/questions/managing_organisation.rb
@@ -17,7 +17,8 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question
     return opts unless log
 
     if log.managing_organisation.present?
-      opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name })
+      org_value = log.managing_organisation.label
+      opts = opts.merge({ log.managing_organisation.id => org_value })
     end
 
     if user.support?
@@ -31,14 +32,14 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question
     end
 
     orgs = if user.support?
-             log.owning_organisation.managing_agents.filter_by_active
+             log.owning_organisation.managing_agents.visible.filter_by_active
            elsif user.organisation.absorbed_organisations.include?(log.owning_organisation)
-             user.organisation.managing_agents.filter_by_active + log.owning_organisation.managing_agents.filter_by_active
+             user.organisation.managing_agents.visible.filter_by_active + log.owning_organisation.managing_agents.visible.filter_by_active # here
            else
-             user.organisation.managing_agents.filter_by_active
+             user.organisation.managing_agents.visible.filter_by_active
            end
 
-    user.organisation.absorbed_organisations.each do |absorbed_org|
+    user.organisation.absorbed_organisations.visible.each do |absorbed_org|
       opts[absorbed_org.id] = "#{absorbed_org.name} (inactive as of #{absorbed_org.merge_date.to_fs(:govuk_date)})"
     end
 
@@ -72,7 +73,10 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question
   end
 
   def answer_label(log, _current_user = nil)
-    Organisation.find_by(id: log.managing_organisation_id)&.name
+    organisation = Organisation.find_by(id: log.managing_organisation_id)
+    return unless organisation
+
+    organisation.label
   end
 
 private
diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb
index 6ce0e5496..daa042004 100644
--- a/app/models/form/lettings/questions/stock_owner.rb
+++ b/app/models/form/lettings/questions/stock_owner.rb
@@ -17,7 +17,8 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
     return answer_opts unless log
 
     if log.owning_organisation_id.present?
-      answer_opts[log.owning_organisation.id] = log.owning_organisation.name
+      org_value = log.owning_organisation.label
+      answer_opts[log.owning_organisation.id] = org_value
     end
 
     recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period
@@ -30,7 +31,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
     end
 
     if user.support?
-      Organisation.filter_by_active.where(holds_own_stock: true).find_each do |org|
+      Organisation.visible.filter_by_active.where(holds_own_stock: true).find_each do |org|
         if org.merge_date.present?
           answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
         elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present?
@@ -40,10 +41,10 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
         end
       end
     else
-      user.organisation.stock_owners.filter_by_active.each do |stock_owner|
+      user.organisation.stock_owners.visible.filter_by_active.each do |stock_owner|
         answer_opts[stock_owner.id] = stock_owner.name
       end
-      recently_absorbed_organisations.each do |absorbed_org|
+      recently_absorbed_organisations.visible.each do |absorbed_org|
         answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock?
       end
     end
@@ -64,7 +65,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
   def hidden_in_check_answers?(_log, user = nil)
     return false if user.support?
 
-    stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
+    stock_owners = user.organisation.stock_owners.visible + user.organisation.absorbed_organisations.visible.where(holds_own_stock: true)
 
     if user.organisation.holds_own_stock?
       stock_owners.count.zero?
diff --git a/app/models/form/sales/questions/managing_organisation.rb b/app/models/form/sales/questions/managing_organisation.rb
index 54d7ab6e0..03609233e 100644
--- a/app/models/form/sales/questions/managing_organisation.rb
+++ b/app/models/form/sales/questions/managing_organisation.rb
@@ -17,7 +17,8 @@ class Form::Sales::Questions::ManagingOrganisation < ::Form::Question
     return opts unless log
 
     if log.managing_organisation.present?
-      opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name })
+      org_value = log.managing_organisation.label
+      opts = opts.merge({ log.managing_organisation.id => org_value })
     end
 
     if user.support?
@@ -31,14 +32,14 @@ class Form::Sales::Questions::ManagingOrganisation < ::Form::Question
     end
 
     orgs = if user.support?
-             log.owning_organisation.managing_agents.filter_by_active
+             log.owning_organisation.managing_agents.visible.filter_by_active
            elsif user.organisation.absorbed_organisations.include?(log.owning_organisation)
-             user.organisation.managing_agents.filter_by_active + log.owning_organisation.managing_agents.filter_by_active
+             user.organisation.managing_agents.visible.filter_by_active + log.owning_organisation.managing_agents.visible.filter_by_active
            else
-             user.organisation.managing_agents.filter_by_active
+             user.organisation.managing_agents.visible.filter_by_active
            end.pluck(:id, :name).to_h
 
-    user.organisation.absorbed_organisations.each do |absorbed_org|
+    user.organisation.absorbed_organisations.visible.each do |absorbed_org|
       opts[absorbed_org.id] = "#{absorbed_org.name} (inactive as of #{absorbed_org.merge_date.to_fs(:govuk_date)})"
     end
 
diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb
index dc3913882..669c43a0e 100644
--- a/app/models/form/sales/questions/owning_organisation_id.rb
+++ b/app/models/form/sales/questions/owning_organisation_id.rb
@@ -16,25 +16,22 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
     return answer_opts unless user
     return answer_opts unless log
 
-    unless user.support?
-      if log.owning_organisation_id.present?
-        answer_opts[log.owning_organisation.id] = log.owning_organisation.name
-      end
+    if log.owning_organisation_id.present?
+      org_value = log.owning_organisation.label
+      answer_opts[log.owning_organisation.id] = org_value
+    end
 
+    unless user.support?
       if user.organisation.holds_own_stock?
         answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)"
       end
 
-      user.organisation.stock_owners.filter_by_active.where(holds_own_stock: true).find_each do |org|
+      user.organisation.stock_owners.visible.filter_by_active.where(holds_own_stock: true).find_each do |org|
         answer_opts[org.id] = org.name
       end
     end
 
-    if log.owning_organisation_id.present?
-      answer_opts[log.owning_organisation.id] = log.owning_organisation.name
-    end
-
-    recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period
+    recently_absorbed_organisations = user.organisation.absorbed_organisations.visible.merged_during_open_collection_period
     if !user.support? && user.organisation.holds_own_stock?
       answer_opts[user.organisation.id] = if recently_absorbed_organisations.exists? && user.organisation.available_from.present?
                                             "#{user.organisation.name} (Your organisation, active as of #{user.organisation.available_from.to_fs(:govuk_date)})"
@@ -44,7 +41,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
     end
 
     if user.support?
-      Organisation.filter_by_active.where(holds_own_stock: true).find_each do |org|
+      Organisation.visible.filter_by_active.where(holds_own_stock: true).find_each do |org|
         if org.merge_date.present?
           answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
         elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present?
@@ -54,7 +51,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
         end
       end
     else
-      recently_absorbed_organisations.each do |absorbed_org|
+      recently_absorbed_organisations.visible.each do |absorbed_org|
         answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock?
       end
     end
@@ -75,7 +72,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
   def hidden_in_check_answers?(_log, user = nil)
     return false if user.support?
 
-    stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
+    stock_owners = user.organisation.stock_owners.visible + user.organisation.absorbed_organisations.visible.where(holds_own_stock: true)
 
     if user.organisation.holds_own_stock?
       stock_owners.count.zero?
diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index 25a3fdd0d..8f77df166 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -17,6 +17,7 @@ class Organisation < ApplicationRecord
 
   belongs_to :absorbing_organisation, class_name: "Organisation", optional: true
   has_many :absorbed_organisations, class_name: "Organisation", foreign_key: "absorbing_organisation_id"
+  scope :visible, -> { where(discarded_at: nil) }
 
   def affiliated_stock_owners
     ids = []
@@ -143,6 +144,7 @@ class Organisation < ApplicationRecord
   end
 
   def status_at(date)
+    return :deleted if discarded_at.present?
     return :merged if merge_date.present? && merge_date < date
     return :deactivated unless active
 
@@ -178,4 +180,14 @@ class Organisation < ApplicationRecord
 
     false
   end
+
+  def discard!
+    owned_schemes.each(&:discard!)
+    users.each(&:discard!)
+    update!(discarded_at: Time.zone.now)
+  end
+
+  def label
+    status == :deleted ? "#{name} (deleted)" : name
+  end
 end
diff --git a/app/policies/organisation_policy.rb b/app/policies/organisation_policy.rb
index 3e0c9a946..9c27d6e91 100644
--- a/app/policies/organisation_policy.rb
+++ b/app/policies/organisation_policy.rb
@@ -13,4 +13,25 @@ class OrganisationPolicy
   def reactivate?
     user.support? && organisation.status == :deactivated
   end
+
+  def delete_confirmation?
+    delete?
+  end
+
+  def delete?
+    return false unless user.support?
+    return false unless organisation.status == :deactivated || organisation.status == :merged
+
+    !has_any_logs_in_editable_collection_period
+  end
+
+  def has_any_logs_in_editable_collection_period
+    editable_from_date = FormHandler.instance.earliest_open_for_editing_collection_start_date
+    editable_lettings_logs = organisation.lettings_logs.visible.after_date(editable_from_date)
+
+    return true if organisation.lettings_logs.visible.where(startdate: nil).any? || editable_lettings_logs.any?
+
+    editable_sales_logs = organisation.sales_logs.visible.after_date(editable_from_date)
+    organisation.sales_logs.visible.where(saledate: nil).any? || editable_sales_logs.any?
+  end
 end
diff --git a/app/views/organisations/delete_confirmation.html.erb b/app/views/organisations/delete_confirmation.html.erb
new file mode 100644
index 000000000..96dba012a
--- /dev/null
+++ b/app/views/organisations/delete_confirmation.html.erb
@@ -0,0 +1,24 @@
+<% content_for :before_content do %>
+  <% content_for :title, "Are you sure you want to delete this organisation?" %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <span class="govuk-caption-xl">Delete <%= @organisation.name %></span>
+    <h1 class="govuk-heading-xl">
+      <%= content_for(:title) %>
+    </h1>
+
+    <%= govuk_warning_text(text: "You will not be able to undo this action.") %>
+
+    <div class="govuk-button-group">
+      <%= govuk_button_to(
+        "Delete this organisation",
+        delete_organisation_path(@organisation),
+        method: :delete,
+      ) %>
+      <%= govuk_button_link_to "Cancel", organisation_path(@organisation), html: { method: :get }, secondary: true %>
+    </div>
+  </div>
+</div>
diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb
index 26a20926e..214b988f0 100644
--- a/app/views/organisations/show.html.erb
+++ b/app/views/organisations/show.html.erb
@@ -52,3 +52,7 @@
       <%= govuk_button_link_to "Reactivate this organisation", reactivate_organisation_path(@organisation) %>
     </span>
 <% end %>
+
+<% if OrganisationPolicy.new(current_user, @organisation).delete? %>
+  <%= delete_organisation_link(@organisation) %>
+<% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 537fe2114..9dcfed8c3 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -202,6 +202,7 @@ en:
     location_deleted: "%{postcode} has been deleted."
     scheme_deleted: "%{service_name} has been deleted."
     user_deleted: "%{name} has been deleted."
+    organisation_deleted: "%{name} has been deleted."
 
   validations:
     organisation:
diff --git a/config/routes.rb b/config/routes.rb
index 5361f5733..e8932ea45 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -190,6 +190,8 @@ Rails.application.routes.draw do
         get "sales-logs/filters/#{filter}", to: "sales_logs_filters#organisation_#{filter.underscore}"
         get "sales-logs/filters/update-#{filter}", to: "sales_logs_filters#update_organisation_#{filter.underscore}"
       end
+      get "delete-confirmation", to: "organisations#delete_confirmation"
+      delete "delete", to: "organisations#delete"
     end
   end
 
diff --git a/db/migrate/20240610142812_add_discarded_at_to_organisations.rb b/db/migrate/20240610142812_add_discarded_at_to_organisations.rb
new file mode 100644
index 000000000..192e7095c
--- /dev/null
+++ b/db/migrate/20240610142812_add_discarded_at_to_organisations.rb
@@ -0,0 +1,5 @@
+class AddDiscardedAtToOrganisations < ActiveRecord::Migration[7.0]
+  def change
+    add_column :organisations, :discarded_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 1430da65f..66693435f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_05_29_133005) do
+ActiveRecord::Schema[7.0].define(version: 2024_06_10_142812) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -492,6 +492,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_05_29_133005) do
     t.datetime "merge_date"
     t.bigint "absorbing_organisation_id"
     t.datetime "available_from"
+    t.datetime "discarded_at"
     t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id"
     t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true
   end
diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb
index 70814b9d5..99319fc71 100644
--- a/spec/models/form/lettings/questions/managing_organisation_spec.rb
+++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb
@@ -58,6 +58,7 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do
       let(:managing_org2) { create(:organisation, name: "Managing org 2") }
       let(:managing_org3) { create(:organisation, name: "Managing org 3") }
       let(:inactive_managing_org) { create(:organisation, name: "Inactive managing org", active: false) }
+      let(:deleted_managing_org) { create(:organisation, name: "Deleted managing org", discarded_at: Time.zone.yesterday) }
 
       let(:log) { create(:lettings_log, managing_organisation: managing_org1) }
       let!(:org_rel1) do
@@ -79,6 +80,7 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do
 
       it "shows current managing agent at top, followed by user's org (with hint), followed by the active managing agents of the user's org" do
         create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: inactive_managing_org)
+        create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: deleted_managing_org)
 
         expect(question.displayed_answer_options(log, user)).to eq(options)
       end
@@ -104,7 +106,10 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do
       end
 
       before do
-        create(:organisation, name: "Inactive managing org", active: false)
+        inactive_managing_org = create(:organisation, name: "Inactive managing org", active: false)
+        create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: inactive_managing_org)
+        deleted_managing_org = create(:organisation, name: "Deleted managing org", discarded_at: Time.zone.yesterday)
+        create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: deleted_managing_org)
       end
 
       context "when org owns stock" do
@@ -192,10 +197,12 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do
     context "when organisation has merged" do
       let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) }
       let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) }
+      let!(:merged_deleted_org) { create(:organisation, name: "Merged org", holds_own_stock: false, discarded_at: Time.zone.yesterday) }
       let(:user) { create(:user, :data_coordinator, organisation: absorbing_org) }
 
       let(:log) do
         merged_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id)
+        merged_deleted_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id)
         create(:lettings_log, owning_organisation: absorbing_org, managing_organisation: nil)
       end
 
diff --git a/spec/models/form/lettings/questions/stock_owner_spec.rb b/spec/models/form/lettings/questions/stock_owner_spec.rb
index a4c7d8fd6..8443f3680 100644
--- a/spec/models/form/lettings/questions/stock_owner_spec.rb
+++ b/spec/models/form/lettings/questions/stock_owner_spec.rb
@@ -47,6 +47,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
       let(:owning_org_1) { create(:organisation, name: "Owning org 1") }
       let(:owning_org_2) { create(:organisation, name: "Owning org 2") }
       let(:inactive_owning_org) { create(:organisation, name: "Inactive owning org", active: false) }
+      let(:deleted_owning_org) { create(:organisation, name: "Deleted owning org", discarded_at: Time.zone.yesterday) }
       let!(:org_rel) do
         create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2)
       end
@@ -64,6 +65,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
 
         it "shows current stock owner at top, followed by user's org (with hint), followed by the active stock owners of the user's org" do
           create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: inactive_owning_org)
+          create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: deleted_owning_org)
           user.organisation.update!(holds_own_stock: true)
           expect(question.displayed_answer_options(log, user)).to eq(options)
         end
@@ -127,6 +129,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
 
       context "when user's org has recently absorbed other orgs and does not have available from date" do
         let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:merged_deleted_organisation) { create(:organisation, name: "Merged deleted org", discarded_at: Time.zone.yesterday) }
         let(:options) do
           {
             "" => "Select an option",
@@ -139,6 +142,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
 
         before do
           merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation)
+          merged_deleted_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation)
           allow(Time).to receive(:now).and_return(Time.zone.local(2023, 11, 10))
         end
 
@@ -172,6 +176,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
 
       context "when user's org has absorbed other orgs with parent organisations during closed collection periods" do
         let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:merged_deleted_organisation) { create(:organisation, name: "Merged deleted org", discarded_at: Time.zone.yesterday) }
         let(:options) do
           {
             "" => "Select an option",
@@ -184,6 +189,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
           allow(Time).to receive(:now).and_return(Time.zone.local(2023, 4, 2))
           org_rel.update!(child_organisation: merged_organisation)
           merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation)
+          merged_deleted_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation)
           user.organisation.update!(available_from: Time.zone.local(2021, 2, 2))
         end
 
@@ -200,8 +206,9 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
       it "shows active orgs where organisation holds own stock" do
         non_stock_organisation = create(:organisation, name: "Non-stockholding org", holds_own_stock: false)
         inactive_organisation = create(:organisation, name: "Inactive org", active: false)
+        deleted_organisation = create(:organisation, name: "Deleted org", discarded_at: Time.zone.yesterday)
 
-        expected_opts = Organisation.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh|
+        expected_opts = Organisation.visible.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh|
           hsh[organisation.id] = organisation.name
           hsh
         end
@@ -209,10 +216,12 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
         expect(question.displayed_answer_options(log, user)).to eq(expected_opts)
         expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id)
         expect(question.displayed_answer_options(log, user)).not_to include(inactive_organisation.id)
+        expect(question.displayed_answer_options(log, user)).not_to include(deleted_organisation.id)
       end
 
       context "and org has recently absorbed other orgs and does not have available from date" do
         let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:merged_deleted_organisation) { create(:organisation, name: "Merged deleted org", discarded_at: Time.zone.yesterday) }
         let(:org) { create(:organisation, name: "User org") }
         let(:options) do
           {
@@ -226,6 +235,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
 
         before do
           merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: org)
+          merged_deleted_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: org)
           allow(Time).to receive(:now).and_return(Time.zone.local(2023, 11, 10))
         end
 
@@ -273,6 +283,18 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
           expect(question.hidden_in_check_answers?(nil, user)).to be false
         end
       end
+
+      context "when stock owner is deleted" do
+        before do
+          organisation_relationship = create(:organisation_relationship, child_organisation: user.organisation)
+          organisation_relationship.parent_organisation.update!(discarded_at: Time.zone.yesterday)
+        end
+
+        it "is hidden in check answers" do
+          expect(user.organisation.stock_owners.count).to eq(1)
+          expect(question.hidden_in_check_answers?(nil, user)).to be true
+        end
+      end
     end
 
     context "when org does not hold own stock", :aggregate_failures do
diff --git a/spec/models/form/sales/questions/managing_organisation_spec.rb b/spec/models/form/sales/questions/managing_organisation_spec.rb
index c9db71423..2ebbf5e01 100644
--- a/spec/models/form/sales/questions/managing_organisation_spec.rb
+++ b/spec/models/form/sales/questions/managing_organisation_spec.rb
@@ -58,6 +58,7 @@ RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do
       let(:managing_org2) { create(:organisation, name: "Managing org 2") }
       let(:managing_org3) { create(:organisation, name: "Managing org 3") }
       let(:inactive_org) { create(:organisation, name: "Inactive org", active: false) }
+      let(:deleted_org) { create(:organisation, name: "Deleted org", discarded_at: Time.zone.yesterday) }
 
       let(:log) do
         create(:lettings_log, owning_organisation: log_owning_org, managing_organisation: managing_org1,
@@ -83,6 +84,7 @@ RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do
 
         it "shows current managing agent at top, followed by the current owning organisation (with hint), followed by the active managing agents of the current owning organisation" do
           create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: inactive_org)
+          create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: deleted_org)
           log_owning_org.update!(holds_own_stock: true)
           expect(question.displayed_answer_options(log, user)).to eq(options)
         end
@@ -135,10 +137,12 @@ RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do
     context "when organisation has merged" do
       let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) }
       let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) }
+      let!(:merged_deleted_org) { create(:organisation, name: "Merged deleted org", holds_own_stock: false, discarded_at: Time.zone.yesterday) }
       let(:user) { create(:user, :support, organisation: absorbing_org) }
 
       let(:log) do
         merged_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id)
+        merged_deleted_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id)
         create(:lettings_log, owning_organisation: absorbing_org, managing_organisation: nil)
       end
 
diff --git a/spec/models/form/sales/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb
index ea904b400..bc6fad6e5 100644
--- a/spec/models/form/sales/questions/owning_organisation_id_spec.rb
+++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb
@@ -123,6 +123,11 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do
         it "shows merged organisation as an option" do
           expect(question.displayed_answer_options(log, user)).to eq(options)
         end
+
+        it "does not show absorbed organisation if it has been deleted" do
+          merged_organisation.update!(discarded_at: Time.zone.yesterday)
+          expect(question.displayed_answer_options(log, user)).to eq(options.except(merged_organisation.id))
+        end
       end
 
       context "when user's org has recently absorbed other orgs and it has available from date" do
@@ -200,8 +205,9 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do
       it "shows active orgs where organisation holds own stock" do
         non_stock_organisation = create(:organisation, holds_own_stock: false)
         inactive_org = create(:organisation, active: false)
+        deleted_organisation = create(:organisation, discarded_at: Time.zone.yesterday)
 
-        expected_opts = Organisation.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh|
+        expected_opts = Organisation.visible.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh|
           hsh[organisation.id] = organisation.name
           hsh
         end
@@ -210,6 +216,7 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do
         expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id)
         expect(question.displayed_answer_options(log, user)).not_to include(inactive_org.id)
         expect(question.displayed_answer_options(log, user)).to include(organisation_1.id)
+        expect(question.displayed_answer_options(log, user)).not_to include(deleted_organisation.id)
       end
 
       context "when an org has recently absorbed other orgs" do
diff --git a/spec/policies/organisation_policy_spec.rb b/spec/policies/organisation_policy_spec.rb
index 4fb4c6761..89cb62198 100644
--- a/spec/policies/organisation_policy_spec.rb
+++ b/spec/policies/organisation_policy_spec.rb
@@ -63,4 +63,112 @@ RSpec.describe OrganisationPolicy do
       expect(policy).not_to permit(support, organisation)
     end
   end
+
+  permissions :delete? do
+    let(:organisation) { FactoryBot.create(:organisation) }
+
+    context "with active organisation" do
+      it "does not allow deleting a organisation as a provider" do
+        expect(policy).not_to permit(data_provider, organisation)
+      end
+
+      it "does not allow allows deleting a organisation as a coordinator" do
+        expect(policy).not_to permit(data_coordinator, organisation)
+      end
+
+      it "does not allow deleting a organisation as a support user" do
+        expect(policy).not_to permit(support, organisation)
+      end
+    end
+
+    context "with deactivated organisation" do
+      before do
+        organisation.update!(active: false)
+      end
+
+      it "does not allow deleting a organisation as a provider" do
+        expect(policy).not_to permit(data_provider, organisation)
+      end
+
+      it "does not allow allows deleting a organisation as a coordinator" do
+        expect(policy).not_to permit(data_coordinator, organisation)
+      end
+
+      it "allows deleting a organisation as a support user" do
+        expect(policy).to permit(support, organisation)
+      end
+
+      context "and associated lettings logs in editable collection period" do
+        before do
+          create(:lettings_log, owning_organisation: organisation)
+        end
+
+        it "does not allow deleting a organisation as a provider" do
+          expect(policy).not_to permit(data_provider, organisation)
+        end
+
+        it "does not allow allows deleting a organisation as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, organisation)
+        end
+
+        it "does not allow deleting a organisation as a support user" do
+          expect(policy).not_to permit(support, organisation)
+        end
+      end
+
+      context "and deleted associated lettings logs in editable collection period" do
+        before do
+          create(:lettings_log, owning_organisation: organisation, discarded_at: Time.zone.yesterday)
+        end
+
+        it "does not allow deleting a organisation as a provider" do
+          expect(policy).not_to permit(data_provider, organisation)
+        end
+
+        it "does not allow allows deleting a organisation as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, organisation)
+        end
+
+        it "allows deleting a organisation as a support user" do
+          expect(policy).to permit(support, organisation)
+        end
+      end
+
+      context "and associated sales logs in editable collection period" do
+        before do
+          create(:sales_log, owning_organisation: organisation)
+        end
+
+        it "does not allow deleting a organisation as a provider" do
+          expect(policy).not_to permit(data_provider, organisation)
+        end
+
+        it "does not allow allows deleting a organisation as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, organisation)
+        end
+
+        it "does not allow deleting a organisation as a support user" do
+          expect(policy).not_to permit(support, organisation)
+        end
+      end
+
+      context "and deleted associated sales logs in editable collection period" do
+        before do
+          create(:sales_log, owning_organisation: organisation, discarded_at: Time.zone.yesterday)
+        end
+
+        it "does not allow deleting a organisation as a provider" do
+          expect(policy).not_to permit(data_provider, organisation)
+        end
+
+        it "does not allow allows deleting a organisation as a coordinator" do
+          expect(policy).not_to permit(data_coordinator, organisation)
+        end
+
+        it "allows deleting a organisation as a support user" do
+          expect(policy).to permit(support, organisation)
+        end
+      end
+    end
+  end
 end
diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb
index f30a1e4eb..68f69d9f2 100644
--- a/spec/requests/organisations_controller_spec.rb
+++ b/spec/requests/organisations_controller_spec.rb
@@ -47,6 +47,34 @@ RSpec.describe OrganisationsController, type: :request do
         expect(response).to redirect_to("/account/sign-in")
       end
     end
+
+    describe "#delete-confirmation" do
+      let(:organisation) { create(:organisation) }
+
+      before do
+        get "/organisations/#{organisation.id}/delete-confirmation"
+      end
+
+      context "when not signed in" do
+        it "redirects to the sign in page" do
+          expect(response).to redirect_to("/account/sign-in")
+        end
+      end
+    end
+
+    describe "#delete" do
+      let(:organisation) { create(:organisation) }
+
+      before do
+        delete "/organisations/#{organisation.id}/delete"
+      end
+
+      context "when not signed in" do
+        it "redirects to the sign in page" do
+          expect(response).to redirect_to("/account/sign-in")
+        end
+      end
+    end
   end
 
   context "when user is signed in" do
@@ -747,6 +775,38 @@ RSpec.describe OrganisationsController, type: :request do
           end
         end
       end
+
+      describe "#delete-confirmation" do
+        let(:organisation) { user.organisation }
+
+        before do
+          get "/organisations/#{organisation.id}/delete-confirmation"
+        end
+
+        context "with a data provider user" do
+          let(:user) { create(:user) }
+
+          it "returns 401 unauthorized" do
+            expect(response).to have_http_status(:unauthorized)
+          end
+        end
+      end
+
+      describe "#delete" do
+        let(:organisation) { user.organisation }
+
+        before do
+          delete "/organisations/#{organisation.id}/delete"
+        end
+
+        context "with a data provider user" do
+          let(:user) { create(:user) }
+
+          it "returns 401 unauthorized" do
+            expect(response).to have_http_status(:unauthorized)
+          end
+        end
+      end
     end
 
     context "with a data provider user" do
@@ -876,6 +936,38 @@ RSpec.describe OrganisationsController, type: :request do
           expect(response).to have_http_status(:unauthorized)
         end
       end
+
+      describe "#delete-confirmation" do
+        let(:organisation) { user.organisation }
+
+        before do
+          get "/organisations/#{organisation.id}/delete-confirmation"
+        end
+
+        context "with a data provider user" do
+          let(:user) { create(:user) }
+
+          it "returns 401 unauthorized" do
+            expect(response).to have_http_status(:unauthorized)
+          end
+        end
+      end
+
+      describe "#delete" do
+        let(:organisation) { user.organisation }
+
+        before do
+          delete "/organisations/#{organisation.id}/delete"
+        end
+
+        context "with a data provider user" do
+          let(:user) { create(:user) }
+
+          it "returns 401 unauthorized" do
+            expect(response).to have_http_status(:unauthorized)
+          end
+        end
+      end
     end
 
     context "with a support user" do
@@ -1431,6 +1523,89 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
+      describe "#show" do
+        let(:organisation) { create(:organisation) }
+
+        before do
+          get "/organisations/#{organisation.id}", headers:, params: {}
+        end
+
+        context "with an active organisation" do
+          it "does not render delete this organisation" do
+            follow_redirect!
+            expect(response).to have_http_status(:ok)
+            expect(page).not_to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation")
+          end
+
+          it "does not render informative text about deleting the organisation" do
+            follow_redirect!
+            expect(response).to have_http_status(:ok)
+            expect(page).not_to have_content("This organisation was active in an open or editable collection year, and cannot be deleted.")
+          end
+        end
+
+        context "with an inactive organisation" do
+          let(:organisation) { create(:organisation, active: false) }
+
+          it "renders delete this organisation" do
+            follow_redirect!
+            expect(response).to have_http_status(:ok)
+            expect(page).to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation")
+          end
+
+          context "and associated lettings logs in editable collection period" do
+            before do
+              create(:lettings_log, owning_organisation: organisation)
+              get "/organisations/#{organisation.id}"
+            end
+
+            it "does not render delete this organisation" do
+              follow_redirect!
+              expect(response).to have_http_status(:ok)
+              expect(page).not_to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation")
+            end
+
+            it "adds informative text about deleting the organisation" do
+              follow_redirect!
+              expect(response).to have_http_status(:ok)
+              expect(page).to have_content("This organisation was active in an open or editable collection year, and cannot be deleted.")
+            end
+          end
+
+          context "and associated sales logs in editable collection period" do
+            before do
+              create(:sales_log, owning_organisation: organisation)
+              get "/organisations/#{organisation.id}"
+            end
+
+            it "does not render delete this organisation" do
+              follow_redirect!
+              expect(response).to have_http_status(:ok)
+              expect(page).not_to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation")
+            end
+
+            it "adds informative text about deleting the organisation" do
+              follow_redirect!
+              expect(response).to have_http_status(:ok)
+              expect(page).to have_content("This organisation was active in an open or editable collection year, and cannot be deleted.")
+            end
+          end
+        end
+
+        context "with merged organisation" do
+          before do
+            organisation.update!(merge_date: Time.zone.yesterday)
+            get "/organisations/#{organisation.id}", headers:, params: {}
+          end
+
+          it "renders delete this organisation" do
+            follow_redirect!
+            expect(response).to have_http_status(:ok)
+            expect(page).to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation")
+          end
+        end
+      end
+
       describe "#update" do
         let(:params) { { id: organisation.id, organisation: { active:, rent_periods: [], all_rent_periods: [] } } }
 
@@ -1581,6 +1756,79 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
+      describe "#delete-confirmation" do
+        let(:organisation) { create(:organisation, active: false) }
+
+        before do
+          get "/organisations/#{organisation.id}/delete-confirmation"
+        end
+
+        it "shows the correct title" do
+          expect(page.find("h1").text).to include "Are you sure you want to delete this organisation?"
+        end
+
+        it "shows a warning to the user" do
+          expect(page).to have_selector(".govuk-warning-text", text: "You will not be able to undo this action")
+        end
+
+        it "shows a button to delete the selected organisation" do
+          expect(page).to have_selector("form.button_to button", text: "Delete this organisation")
+        end
+
+        it "the delete organisation button submits the correct data to the correct path" do
+          form_containing_button = page.find("form.button_to")
+
+          expect(form_containing_button[:action]).to eq delete_organisation_path(organisation)
+          expect(form_containing_button).to have_field "_method", type: :hidden, with: "delete"
+        end
+
+        it "shows a cancel link with the correct style" do
+          expect(page).to have_selector("a.govuk-button--secondary", text: "Cancel")
+        end
+
+        it "shows cancel link that links back to the organisation page" do
+          expect(page).to have_link(text: "Cancel", href: organisation_path(organisation))
+        end
+      end
+
+      describe "#delete" do
+        let(:organisation) { create(:organisation, active: false) }
+        let!(:org_user) { create(:user, active: false, organisation:) }
+        let!(:scheme) { create(:scheme, owning_organisation: organisation) }
+        let!(:location) { create(:location, scheme:) }
+
+        before do
+          scheme.scheme_deactivation_periods << SchemeDeactivationPeriod.new(deactivation_date: Time.zone.yesterday)
+          location.location_deactivation_periods << LocationDeactivationPeriod.new(deactivation_date: Time.zone.yesterday)
+          delete "/organisations/#{organisation.id}/delete"
+        end
+
+        it "deletes the organisation and related resources" do
+          organisation.reload
+          expect(organisation.status).to eq(:deleted)
+          expect(organisation.discarded_at).not_to be nil
+          expect(location.reload.status).to eq(:deleted)
+          expect(location.discarded_at).not_to be nil
+          expect(scheme.reload.status).to eq(:deleted)
+          expect(scheme.discarded_at).not_to be nil
+          expect(org_user.reload.status).to eq(:deleted)
+          expect(org_user.discarded_at).not_to be nil
+        end
+
+        it "redirects to the organisations list and displays a notice that the organisation has been deleted" do
+          expect(response).to redirect_to organisations_path
+          follow_redirect!
+          expect(page).to have_selector(".govuk-notification-banner--success")
+          expect(page).to have_selector(".govuk-notification-banner--success", text: "#{organisation.name} has been deleted.")
+        end
+
+        it "does not display the deleted organisation" do
+          expect(response).to redirect_to organisations_path
+          follow_redirect!
+          expect(page).not_to have_content("Organisation to delete")
+        end
+      end
+
       context "when they view the lettings logs tab" do
         let(:tenancycode) { "42" }
 

From 42f89680e58a7410da5dba0f6c758cb97807da82 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Fri, 12 Jul 2024 17:00:06 +0100
Subject: [PATCH 11/11] CLDC-3495 Reduce persisted models in tests (#2507)

* Remove legacy user from user factory

* Build instead of creating in some model tests

* Do not create a new DPO for orgs created within user factories

* Add original setup doc

* Update documentation with updated factories

* Fix up DSA creations
---
 docs/testing.md                               | 49 +++++++++++++++++++
 .../create_log_actions_component_spec.rb      |  2 +-
 ...tion_confirmation_banner_component_spec.rb | 16 +++---
 spec/factories/organisation.rb                |  4 +-
 spec/factories/user.rb                        | 16 ++++--
 spec/features/user_spec.rb                    |  2 +-
 spec/mailers/resend_invitation_mailer_spec.rb |  1 +
 .../lettings/pages/address_matcher_spec.rb    |  2 +-
 .../lettings/pages/uprn_selection_spec.rb     |  2 +-
 spec/models/form/lettings/pages/uprn_spec.rb  |  2 +-
 .../questions/property_reference_spec.rb      |  2 +-
 .../questions/uprn_confirmation_spec.rb       | 10 ++--
 .../form/sales/pages/address_matcher_spec.rb  |  2 +-
 .../sales/pages/uprn_confirmation_spec.rb     |  6 +--
 .../form/sales/pages/uprn_selection_spec.rb   |  2 +-
 spec/models/form/sales/pages/uprn_spec.rb     |  2 +-
 .../address_line1_for_address_matcher_spec.rb |  2 +-
 .../sales/questions/buyer1_mortgage_spec.rb   |  2 +-
 .../sales/questions/buyer2_mortgage_spec.rb   |  2 +-
 .../questions/buyers_organisations_spec.rb    |  2 +-
 .../sales/questions/deposit_amount_spec.rb    |  6 +--
 .../postcode_for_address_matcher_spec.rb      |  2 +-
 .../sales/questions/uprn_confirmation_spec.rb |  8 +--
 .../sales/questions/uprn_selection_spec.rb    |  2 +-
 spec/models/form/sales/questions/uprn_spec.rb |  2 +-
 .../discounted_ownership_scheme_spec.rb       |  4 +-
 .../household_characteristics_spec.rb         |  4 +-
 .../sales/subsections/outright_sale_spec.rb   |  4 +-
 .../shared_ownership_scheme_spec.rb           |  4 +-
 spec/models/scheme_spec.rb                    |  4 +-
 spec/models/user_spec.rb                      |  1 +
 .../validations/date_validations_spec.rb      |  2 +-
 .../local_authority_validations_spec.rb       |  2 +-
 .../sales/soft_validations_spec.rb            |  2 +-
 .../validations/setup_validations_spec.rb     |  4 +-
 .../validations/tenancy_validations_spec.rb   |  6 +--
 spec/policies/user_policy_spec.rb             |  2 +-
 ...lk_upload_lettings_logs_controller_spec.rb |  2 +-
 .../bulk_upload_sales_logs_controller_spec.rb |  2 +-
 .../requests/organisations_controller_spec.rb |  3 +-
 .../_create_for_org_actions.html.erb_spec.rb  |  2 +-
 .../data_sharing_agreement.html.erb_spec.rb   |  2 +-
 .../views/organisations/show.html.erb_spec.rb | 12 ++---
 43 files changed, 135 insertions(+), 75 deletions(-)

diff --git a/docs/testing.md b/docs/testing.md
index 57e40f747..0c7e62571 100644
--- a/docs/testing.md
+++ b/docs/testing.md
@@ -31,3 +31,52 @@ bundle exec rake parallel:setup
 ```sh
 RAILS_ENV=test bundle exec rake parallel:spec
 ```
+
+## Factories for Lettings Log, Sales Log, Organisation, and User 
+Each of these factories has nested relationships and callbacks that ensure associated objects are created and linked properly. For instance, creating a `lettings_log` involves creating or associating with a `user`, which in turn is linked to an `organisation`, potentially leading to creating `organisation_rent_periods` and a `data_protection_confirmation`.
+
+This documentation outlines the objects that are created and/or persisted to the database when using FactoryBot to create or build models for LettingsLog, SalesLog, Organisation, and User. There are other factories, but they are simpler, less frequently used and don't have as much resource hierarchy.
+
+### Lettings Log
+Objects Created/Persisted:
+- **User**: The `assigned_to` user is created.
+  - **Organisation**: The `assigned_to` user’s organisation created by `User` factory.
+- **DataProtectionConfirmation**: If `organisation` does not have DSA signed, `DataProtectionConfirmation` gets created with `assigned_to` user as a `data_protection_officer`
+- **OrganisationRentPeriod**: If `log.period` is present and the `managing_organisation` does not have an `OrganisationRentPeriod` for that period, a new `OrganisationRentPeriod` is created and associated with `managing_organisation`.
+
+Example Usage:
+```
+let(:lettings_log) { create(:lettings_log) }
+```
+
+### Sales Log
+Objects Created/Persisted:
+- **User**: The `assigned_to` user is created.
+  - **Organisation**: The `assigned_to` user’s organisation created by `User` factory.
+- **DataProtectionConfirmation**: If `organisation` does not have DSA signed, `DataProtectionConfirmation` gets created with `assigned_to` user as a `data_protection_officer`
+
+Example Usage:
+```
+let(:sales_log) { create(:sales_log) }
+```
+
+### Organisation
+Objects Created/Persisted:
+- **OrganisationRentPeriod**: For each rent period in transient attribute `rent_periods`, an `OrganisationRentPeriod` is created.
+- **DataProtectionConfirmation**: If `with_dsa` is `true` (default), a `DataProtectionConfirmation` is created with a `data_protection_officer`
+- **User**: Data protection officer that signs the data protection confirmation
+
+Example Usage:
+```
+let(:organisation) { create(:organisation, rent_periods: [1, 2])}
+```
+
+### User
+Objects Created/Persisted:
+- **Organisation**: User’s organisation.
+- **DataProtectionConfirmation**: If `organisation` does not have DSA signed, `DataProtectionConfirmation` gets created with this user as a `data_protection_officer`
+
+Example Usage:
+```
+let(:user) { create(:user) }
+```
\ No newline at end of file
diff --git a/spec/components/create_log_actions_component_spec.rb b/spec/components/create_log_actions_component_spec.rb
index 8444b939e..b1caa1443 100644
--- a/spec/components/create_log_actions_component_spec.rb
+++ b/spec/components/create_log_actions_component_spec.rb
@@ -66,7 +66,7 @@ RSpec.describe CreateLogActionsComponent, type: :component do
 
     context "when not support user" do
       context "without data sharing agreement" do
-        let(:user) { create(:user, organisation: create(:organisation, :without_dpc)) }
+        let(:user) { create(:user, organisation: create(:organisation, :without_dpc), with_dsa: false) }
 
         it "does not render actions" do
           expect(component).not_to be_display_actions
diff --git a/spec/components/data_protection_confirmation_banner_component_spec.rb b/spec/components/data_protection_confirmation_banner_component_spec.rb
index 3064a1b8e..608628093 100644
--- a/spec/components/data_protection_confirmation_banner_component_spec.rb
+++ b/spec/components/data_protection_confirmation_banner_component_spec.rb
@@ -5,11 +5,11 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
 
   let(:component) { described_class.new(user:, organisation:) }
   let(:render) { render_inline(component) }
-  let(:user) { create(:user) }
+  let(:user) { create(:user, with_dsa: false) }
   let(:organisation) { user.organisation }
 
   context "when user is support and organisation is blank" do
-    let(:user) { create(:user, :support) }
+    let(:user) { create(:user, :support, with_dsa: false) }
     let(:organisation) { nil }
 
     it "does not display banner" do
@@ -37,8 +37,8 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
     context "when org does not have a signed data sharing agreement" do
       context "when user is not a DPO" do
         let(:organisation) { create(:organisation, :without_dpc) }
-        let(:user) { create(:user, organisation:) }
-        let!(:dpo) { create(:user, :data_protection_officer, organisation:) }
+        let(:user) { create(:user, organisation:, with_dsa: false) }
+        let!(:dpo) { create(:user, :data_protection_officer, organisation:, with_dsa: false) }
 
         it "displays the banner and shows DPOs" do
           expect(component.display_banner?).to eq(true)
@@ -50,7 +50,7 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
 
       context "when user is a DPO" do
         let(:organisation) { create(:organisation, :without_dpc) }
-        let(:user) { create(:user, :data_protection_officer, organisation:) }
+        let(:user) { create(:user, :data_protection_officer, organisation:, with_dsa: false) }
 
         it "displays the banner and asks to sign" do
           expect(component.display_banner?).to eq(true)
@@ -141,8 +141,8 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
       context "when org does not have a signed data sharing agreement" do
         context "when user is not a DPO" do
           let(:organisation) { create(:organisation, :without_dpc) }
-          let(:user) { create(:user, organisation:) }
-          let!(:dpo) { create(:user, :data_protection_officer, organisation:) }
+          let(:user) { create(:user, organisation:, with_dsa: false) }
+          let!(:dpo) { create(:user, :data_protection_officer, organisation:, with_dsa: false) }
 
           it "displays the banner and shows DPOs" do
             expect(component.display_banner?).to eq(true)
@@ -168,7 +168,7 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
 
         context "when user is a DPO" do
           let(:organisation) { create(:organisation, :without_dpc) }
-          let(:user) { create(:user, :data_protection_officer, organisation:) }
+          let(:user) { create(:user, :data_protection_officer, organisation:, with_dsa: false) }
 
           it "displays the banner and asks to sign" do
             expect(component.display_banner?).to eq(true)
diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb
index c42a9d6a2..f498632cd 100644
--- a/spec/factories/organisation.rb
+++ b/spec/factories/organisation.rb
@@ -25,11 +25,11 @@ FactoryBot.define do
     end
 
     after(:create) do |org, evaluator|
-      if evaluator.with_dsa
+      if evaluator.with_dsa && !org.data_protection_confirmed?
         create(
           :data_protection_confirmation,
           organisation: org,
-          data_protection_officer: org.users.any? ? org.users.first : create(:user, :data_protection_officer, organisation: org),
+          data_protection_officer: org.users.any? ? org.users.first : create(:user, :data_protection_officer, organisation: org, with_dsa: false),
         )
       end
     end
diff --git a/spec/factories/user.rb b/spec/factories/user.rb
index 22b93d2d6..956d88332 100644
--- a/spec/factories/user.rb
+++ b/spec/factories/user.rb
@@ -3,7 +3,7 @@ FactoryBot.define do
     sequence(:email) { "test#{SecureRandom.hex}@example.com" }
     name { "Danny Rojas" }
     password { "pAssword1" }
-    organisation
+    organisation { association :organisation, with_dsa: is_dpo ? false : true }
     role { "data_provider" }
     phone { "1234512345123" }
     trait :data_provider do
@@ -27,10 +27,18 @@ FactoryBot.define do
       old_user_id { SecureRandom.uuid }
     end
 
-    after(:create) do |user, evaluator|
-      FactoryBot.create(:legacy_user, old_user_id: evaluator.old_user_id, user:)
+    transient do
+      with_dsa { true }
+    end
 
-      user.reload
+    after(:create) do |user, evaluator|
+      if evaluator.with_dsa && !user.organisation.data_protection_confirmed?
+        create(
+          :data_protection_confirmation,
+          organisation: user.organisation,
+          data_protection_officer: user,
+        )
+      end
     end
   end
 end
diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb
index e3335c762..169465cb1 100644
--- a/spec/features/user_spec.rb
+++ b/spec/features/user_spec.rb
@@ -594,7 +594,7 @@ RSpec.describe "User Features" do
       end
 
       before do
-        other_user.update!(initial_confirmation_sent: true, last_sign_in_at: nil)
+        other_user.update!(initial_confirmation_sent: true, last_sign_in_at: nil, old_user_id: "old-user-id")
         allow(user).to receive(:need_two_factor_authentication?).and_return(false)
         sign_in(user)
         visit(user_path(other_user))
diff --git a/spec/mailers/resend_invitation_mailer_spec.rb b/spec/mailers/resend_invitation_mailer_spec.rb
index a5eadad20..2b76484c3 100644
--- a/spec/mailers/resend_invitation_mailer_spec.rb
+++ b/spec/mailers/resend_invitation_mailer_spec.rb
@@ -41,6 +41,7 @@ RSpec.describe ResendInvitationMailer do
       end
 
       it "sends an initial invitation" do
+        FactoryBot.create(:legacy_user, old_user_id: new_active_migrated_user.old_user_id, user: new_active_migrated_user)
         expect(notify_client).to receive(:send_email).with(email_address: "new_active_migrated_user@example.com", template_id: User::BETA_ONBOARDING_TEMPLATE_ID, personalisation:).once
         described_class.new.resend_invitation_email(new_active_migrated_user)
       end
diff --git a/spec/models/form/lettings/pages/address_matcher_spec.rb b/spec/models/form/lettings/pages/address_matcher_spec.rb
index 8cc98f480..16f571754 100644
--- a/spec/models/form/lettings/pages/address_matcher_spec.rb
+++ b/spec/models/form/lettings/pages/address_matcher_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Lettings::Pages::AddressMatcher, type: :model do
   let(:page_id) { nil }
   let(:page_definition) { nil }
   let(:subsection) { instance_double(Form::Subsection) }
-  let(:log) { create(:lettings_log) }
+  let(:log) { build(:lettings_log) }
 
   it "has correct subsection" do
     expect(page.subsection).to eq(subsection)
diff --git a/spec/models/form/lettings/pages/uprn_selection_spec.rb b/spec/models/form/lettings/pages/uprn_selection_spec.rb
index 5cbb08e93..a2e8086d0 100644
--- a/spec/models/form/lettings/pages/uprn_selection_spec.rb
+++ b/spec/models/form/lettings/pages/uprn_selection_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Lettings::Pages::UprnSelection, type: :model do
   let(:page_id) { nil }
   let(:page_definition) { nil }
   let(:subsection) { instance_double(Form::Subsection) }
-  let(:log) { create(:lettings_log) }
+  let(:log) { build(:lettings_log) }
 
   it "has correct subsection" do
     expect(page.subsection).to eq(subsection)
diff --git a/spec/models/form/lettings/pages/uprn_spec.rb b/spec/models/form/lettings/pages/uprn_spec.rb
index 089daf4bc..c41de5f6a 100644
--- a/spec/models/form/lettings/pages/uprn_spec.rb
+++ b/spec/models/form/lettings/pages/uprn_spec.rb
@@ -45,7 +45,7 @@ RSpec.describe Form::Lettings::Pages::Uprn, type: :model do
     end
 
     context "when log is present" do
-      let(:log) { create(:lettings_log) }
+      let(:log) { build(:lettings_log) }
 
       context "with 2023/24 form" do
         it "points to address page" do
diff --git a/spec/models/form/lettings/questions/property_reference_spec.rb b/spec/models/form/lettings/questions/property_reference_spec.rb
index 2f0c21b75..970bead96 100644
--- a/spec/models/form/lettings/questions/property_reference_spec.rb
+++ b/spec/models/form/lettings/questions/property_reference_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Lettings::Questions::PropertyReference, type: :model do
   let(:question_id) { nil }
   let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page, subsection: instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1)))) }
-  let(:lettings_log) { FactoryBot.create(:lettings_log) }
+  let(:lettings_log) { FactoryBot.build(:lettings_log) }
 
   it "has correct page" do
     expect(question.page).to eq(page)
diff --git a/spec/models/form/lettings/questions/uprn_confirmation_spec.rb b/spec/models/form/lettings/questions/uprn_confirmation_spec.rb
index ee0235c54..c0b6ef8a4 100644
--- a/spec/models/form/lettings/questions/uprn_confirmation_spec.rb
+++ b/spec/models/form/lettings/questions/uprn_confirmation_spec.rb
@@ -42,7 +42,7 @@ RSpec.describe Form::Lettings::Questions::UprnConfirmation, type: :model do
   describe "notification_banner" do
     context "when address is not present" do
       it "returns nil" do
-        log = create(:lettings_log)
+        log = build(:lettings_log)
 
         expect(question.notification_banner(log)).to be_nil
       end
@@ -50,7 +50,7 @@ RSpec.describe Form::Lettings::Questions::UprnConfirmation, type: :model do
 
     context "when address is present" do
       it "returns formatted value" do
-        log = create(:lettings_log, :setup_completed, address_line1: "1, Test Street", town_or_city: "Test Town", postcode_full: "AA1 1AA", uprn: "1", uprn_known: 1)
+        log = build(:lettings_log, :setup_completed, address_line1: "1, Test Street", town_or_city: "Test Town", postcode_full: "AA1 1AA", uprn: "1", uprn_known: 1)
 
         expect(question.notification_banner(log)).to eq(
           {
@@ -64,7 +64,7 @@ RSpec.describe Form::Lettings::Questions::UprnConfirmation, type: :model do
 
   describe "has the correct hidden_in_check_answers" do
     context "when uprn_known != 1 && uprn_confirmed == nil" do
-      let(:log) { create(:lettings_log, uprn_known: 0, uprn_confirmed: nil) }
+      let(:log) { build(:lettings_log, uprn_known: 0, uprn_confirmed: nil) }
 
       it "returns true" do
         expect(question.hidden_in_check_answers?(log)).to eq(true)
@@ -72,7 +72,7 @@ RSpec.describe Form::Lettings::Questions::UprnConfirmation, type: :model do
     end
 
     context "when uprn_known == 1 && uprn_confirmed == nil" do
-      let(:log) { create(:lettings_log, :completed, uprn_known: 1, uprn: 1, uprn_confirmed: nil) }
+      let(:log) { build(:lettings_log, :completed, uprn_known: 1, uprn: 1, uprn_confirmed: nil) }
 
       it "returns false" do
         expect(question.hidden_in_check_answers?(log)).to eq(false)
@@ -80,7 +80,7 @@ RSpec.describe Form::Lettings::Questions::UprnConfirmation, type: :model do
     end
 
     context "when uprn_known != 1 && uprn_confirmed == 1" do
-      let(:log) { create(:lettings_log) }
+      let(:log) { build(:lettings_log) }
 
       it "returns true" do
         log.uprn_known = 1
diff --git a/spec/models/form/sales/pages/address_matcher_spec.rb b/spec/models/form/sales/pages/address_matcher_spec.rb
index e2eea6fa0..6d6a4174f 100644
--- a/spec/models/form/sales/pages/address_matcher_spec.rb
+++ b/spec/models/form/sales/pages/address_matcher_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Pages::AddressMatcher, type: :model do
   let(:page_id) { nil }
   let(:page_definition) { nil }
   let(:subsection) { instance_double(Form::Subsection) }
-  let(:log) { create(:sales_log) }
+  let(:log) { build(:sales_log) }
 
   it "has correct subsection" do
     expect(page.subsection).to eq(subsection)
diff --git a/spec/models/form/sales/pages/uprn_confirmation_spec.rb b/spec/models/form/sales/pages/uprn_confirmation_spec.rb
index a09cb2f08..bd52233b6 100644
--- a/spec/models/form/sales/pages/uprn_confirmation_spec.rb
+++ b/spec/models/form/sales/pages/uprn_confirmation_spec.rb
@@ -33,7 +33,7 @@ RSpec.describe Form::Sales::Pages::UprnConfirmation, type: :model do
 
   describe "has correct routed_to?" do
     context "when uprn present && uprn_known == 1 " do
-      let(:log) { create(:sales_log) }
+      let(:log) { build(:sales_log) }
 
       it "returns true" do
         log.uprn_known = 1
@@ -43,7 +43,7 @@ RSpec.describe Form::Sales::Pages::UprnConfirmation, type: :model do
     end
 
     context "when uprn = nil" do
-      let(:log) { create(:sales_log, uprn_known: 1, uprn: nil) }
+      let(:log) { build(:sales_log, uprn_known: 1, uprn: nil) }
 
       it "returns false" do
         expect(page.routed_to?(log)).to eq(false)
@@ -51,7 +51,7 @@ RSpec.describe Form::Sales::Pages::UprnConfirmation, type: :model do
     end
 
     context "when uprn_known == 0" do
-      let(:log) { create(:sales_log, uprn_known: 0, uprn: "123456789") }
+      let(:log) { build(:sales_log, uprn_known: 0, uprn: "123456789") }
 
       it "returns false" do
         expect(page.routed_to?(log)).to eq(false)
diff --git a/spec/models/form/sales/pages/uprn_selection_spec.rb b/spec/models/form/sales/pages/uprn_selection_spec.rb
index 6013774d3..2e3f3e8f6 100644
--- a/spec/models/form/sales/pages/uprn_selection_spec.rb
+++ b/spec/models/form/sales/pages/uprn_selection_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Pages::UprnSelection, type: :model do
   let(:page_id) { nil }
   let(:page_definition) { nil }
   let(:subsection) { instance_double(Form::Subsection) }
-  let(:log) { create(:sales_log) }
+  let(:log) { build(:sales_log) }
 
   it "has correct subsection" do
     expect(page.subsection).to eq(subsection)
diff --git a/spec/models/form/sales/pages/uprn_spec.rb b/spec/models/form/sales/pages/uprn_spec.rb
index ef1c9848f..8511af372 100644
--- a/spec/models/form/sales/pages/uprn_spec.rb
+++ b/spec/models/form/sales/pages/uprn_spec.rb
@@ -45,7 +45,7 @@ RSpec.describe Form::Sales::Pages::Uprn, type: :model do
     end
 
     context "when log is present" do
-      let(:log) { create(:sales_log) }
+      let(:log) { build(:sales_log) }
 
       context "with 2023/24 form" do
         it "points to address page" do
diff --git a/spec/models/form/sales/questions/address_line1_for_address_matcher_spec.rb b/spec/models/form/sales/questions/address_line1_for_address_matcher_spec.rb
index 213fddaf3..0272d1329 100644
--- a/spec/models/form/sales/questions/address_line1_for_address_matcher_spec.rb
+++ b/spec/models/form/sales/questions/address_line1_for_address_matcher_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Questions::AddressLine1ForAddressMatcher, type: :mod
   let(:question_id) { nil }
   let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page) }
-  let(:log) { create(:sales_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") }
+  let(:log) { build(:sales_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") }
 
   it "has correct page" do
     expect(question.page).to eq(page)
diff --git a/spec/models/form/sales/questions/buyer1_mortgage_spec.rb b/spec/models/form/sales/questions/buyer1_mortgage_spec.rb
index 94d78eafa..a0790c7f8 100644
--- a/spec/models/form/sales/questions/buyer1_mortgage_spec.rb
+++ b/spec/models/form/sales/questions/buyer1_mortgage_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Questions::Buyer1Mortgage, type: :model do
   let(:question_id) { nil }
   let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page, subsection: instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1)))) }
-  let(:log) { create(:sales_log) }
+  let(:log) { build(:sales_log) }
 
   it "has correct page" do
     expect(question.page).to eq(page)
diff --git a/spec/models/form/sales/questions/buyer2_mortgage_spec.rb b/spec/models/form/sales/questions/buyer2_mortgage_spec.rb
index 6b297423c..d96f094e2 100644
--- a/spec/models/form/sales/questions/buyer2_mortgage_spec.rb
+++ b/spec/models/form/sales/questions/buyer2_mortgage_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Questions::Buyer2Mortgage, type: :model do
   let(:question_id) { nil }
   let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page, subsection: instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1)))) }
-  let(:log) { create(:sales_log) }
+  let(:log) { build(:sales_log) }
 
   it "has correct page" do
     expect(question.page).to eq(page)
diff --git a/spec/models/form/sales/questions/buyers_organisations_spec.rb b/spec/models/form/sales/questions/buyers_organisations_spec.rb
index 0c28cff6f..c997c5883 100644
--- a/spec/models/form/sales/questions/buyers_organisations_spec.rb
+++ b/spec/models/form/sales/questions/buyers_organisations_spec.rb
@@ -48,7 +48,7 @@ RSpec.describe Form::Sales::Questions::BuyersOrganisations, type: :model do
   end
 
   it "has the correct displayed answer_options" do
-    expect(question.displayed_answer_options(FactoryBot.create(:sales_log))).to eq(
+    expect(question.displayed_answer_options(FactoryBot.build(:sales_log))).to eq(
       {
         "pregyrha" => { "value" => "Their private registered provider (PRP) - housing association" },
         "pregother" => { "value" => "Other private registered provider (PRP) - housing association" },
diff --git a/spec/models/form/sales/questions/deposit_amount_spec.rb b/spec/models/form/sales/questions/deposit_amount_spec.rb
index cb32a7ee1..22119a733 100644
--- a/spec/models/form/sales/questions/deposit_amount_spec.rb
+++ b/spec/models/form/sales/questions/deposit_amount_spec.rb
@@ -28,7 +28,7 @@ RSpec.describe Form::Sales::Questions::DepositAmount, type: :model do
   end
 
   context "when the ownership type is shared" do
-    let(:log) { create(:sales_log, :completed, ownershipsch: 1, mortgageused: 2) }
+    let(:log) { build(:sales_log, :completed, ownershipsch: 1, mortgageused: 2) }
 
     it "is not marked as derived" do
       expect(question.derived?(log)).to be false
@@ -36,7 +36,7 @@ RSpec.describe Form::Sales::Questions::DepositAmount, type: :model do
   end
 
   context "when the ownership type is discounted for 2023" do
-    let(:log) { create(:sales_log, :completed, ownershipsch: 2, mortgageused: 2, saledate: Time.zone.local(2024, 3, 1)) }
+    let(:log) { build(:sales_log, :completed, ownershipsch: 2, mortgageused: 2, saledate: Time.zone.local(2024, 3, 1)) }
 
     it "is not marked as derived" do
       expect(question.derived?(log)).to be false
@@ -44,7 +44,7 @@ RSpec.describe Form::Sales::Questions::DepositAmount, type: :model do
   end
 
   context "when the ownership type is outright" do
-    let(:log) { create(:sales_log, :completed, ownershipsch: 3, mortgageused: 2) }
+    let(:log) { build(:sales_log, :completed, ownershipsch: 3, mortgageused: 2) }
 
     it "is not marked as derived when a mortgage is used" do
       log.mortgageused = 1
diff --git a/spec/models/form/sales/questions/postcode_for_address_matcher_spec.rb b/spec/models/form/sales/questions/postcode_for_address_matcher_spec.rb
index e2a2931a9..5ba7e8505 100644
--- a/spec/models/form/sales/questions/postcode_for_address_matcher_spec.rb
+++ b/spec/models/form/sales/questions/postcode_for_address_matcher_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Questions::PostcodeForAddressMatcher, type: :model d
   let(:question_id) { nil }
   let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page) }
-  let(:log) { create(:sales_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") }
+  let(:log) { build(:sales_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") }
 
   it "has correct page" do
     expect(question.page).to eq(page)
diff --git a/spec/models/form/sales/questions/uprn_confirmation_spec.rb b/spec/models/form/sales/questions/uprn_confirmation_spec.rb
index b6e9195e0..bb5688944 100644
--- a/spec/models/form/sales/questions/uprn_confirmation_spec.rb
+++ b/spec/models/form/sales/questions/uprn_confirmation_spec.rb
@@ -42,7 +42,7 @@ RSpec.describe Form::Sales::Questions::UprnConfirmation, type: :model do
   describe "notification_banner" do
     context "when address is not present" do
       it "returns nil" do
-        log = create(:sales_log)
+        log = build(:sales_log)
 
         expect(question.notification_banner(log)).to be_nil
       end
@@ -64,7 +64,7 @@ RSpec.describe Form::Sales::Questions::UprnConfirmation, type: :model do
 
   describe "has the correct hidden_in_check_answers" do
     context "when uprn_known != 1 && uprn_confirmed == nil" do
-      let(:log) { create(:sales_log, uprn_known: 0, uprn_confirmed: nil) }
+      let(:log) { build(:sales_log, uprn_known: 0, uprn_confirmed: nil) }
 
       it "returns true" do
         expect(question.hidden_in_check_answers?(log)).to eq(true)
@@ -72,7 +72,7 @@ RSpec.describe Form::Sales::Questions::UprnConfirmation, type: :model do
     end
 
     context "when uprn_known == 1 && uprn_confirmed == nil" do
-      let(:log) { create(:sales_log) }
+      let(:log) { build(:sales_log) }
 
       it "returns false" do
         log.uprn_known = 1
@@ -83,7 +83,7 @@ RSpec.describe Form::Sales::Questions::UprnConfirmation, type: :model do
     end
 
     context "when uprn_known != 1 && uprn_confirmed == 1" do
-      let(:log) { create(:sales_log, uprn_known: 1, uprn: "12345", uprn_confirmed: 1) }
+      let(:log) { build(:sales_log, uprn_known: 1, uprn: "12345", uprn_confirmed: 1) }
 
       it "returns true" do
         expect(question.hidden_in_check_answers?(log)).to eq(true)
diff --git a/spec/models/form/sales/questions/uprn_selection_spec.rb b/spec/models/form/sales/questions/uprn_selection_spec.rb
index 5f7951c3d..cd23a8e9f 100644
--- a/spec/models/form/sales/questions/uprn_selection_spec.rb
+++ b/spec/models/form/sales/questions/uprn_selection_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Form::Sales::Questions::UprnSelection, type: :model do
   let(:question_id) { nil }
   let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page, skip_href: "skip_href") }
-  let(:log) { create(:sales_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") }
+  let(:log) { build(:sales_log, :in_progress, address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") }
   let(:address_client_instance) { AddressClient.new(log.address_string) }
 
   before do
diff --git a/spec/models/form/sales/questions/uprn_spec.rb b/spec/models/form/sales/questions/uprn_spec.rb
index fc0c5caa7..afc4e4a24 100644
--- a/spec/models/form/sales/questions/uprn_spec.rb
+++ b/spec/models/form/sales/questions/uprn_spec.rb
@@ -45,7 +45,7 @@ RSpec.describe Form::Sales::Questions::Uprn, type: :model do
 
   describe "get_extra_check_answer_value" do
     context "when address is not present" do
-      let(:log) { create(:sales_log) }
+      let(:log) { build(:sales_log) }
 
       it "returns nil" do
         expect(question.get_extra_check_answer_value(log)).to be_nil
diff --git a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb
index 99e861b6d..5f20cbb91 100644
--- a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb
+++ b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb
@@ -67,7 +67,7 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model
   end
 
   context "when it is a discounted ownership scheme" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 2) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 2) }
 
     it "is displayed in tasklist" do
       expect(discounted_ownership_scheme.displayed_in_tasklist?(log)).to eq(true)
@@ -75,7 +75,7 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model
   end
 
   context "when it is not a discounted ownership scheme" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 1) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 1) }
 
     it "is displayed in tasklist" do
       expect(discounted_ownership_scheme.displayed_in_tasklist?(log)).to eq(false)
diff --git a/spec/models/form/sales/subsections/household_characteristics_spec.rb b/spec/models/form/sales/subsections/household_characteristics_spec.rb
index 2557f33c6..be78f91d3 100644
--- a/spec/models/form/sales/subsections/household_characteristics_spec.rb
+++ b/spec/models/form/sales/subsections/household_characteristics_spec.rb
@@ -378,7 +378,7 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model
   end
 
   context "when the sale is to a company buyer" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 3, companybuy: 1) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 3, companybuy: 1) }
 
     it "is not displayed in tasklist" do
       expect(household_characteristics.displayed_in_tasklist?(log)).to eq(false)
@@ -386,7 +386,7 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model
   end
 
   context "when the sale is not to a company buyer" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 3, companybuy: 2) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 3, companybuy: 2) }
 
     it "is displayed in tasklist" do
       expect(household_characteristics.displayed_in_tasklist?(log)).to eq(true)
diff --git a/spec/models/form/sales/subsections/outright_sale_spec.rb b/spec/models/form/sales/subsections/outright_sale_spec.rb
index cb0f9e7b3..efb2aaad6 100644
--- a/spec/models/form/sales/subsections/outright_sale_spec.rb
+++ b/spec/models/form/sales/subsections/outright_sale_spec.rb
@@ -122,7 +122,7 @@ RSpec.describe Form::Sales::Subsections::OutrightSale, type: :model do
   end
 
   context "when it is a outright sale" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 3) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 3) }
 
     it "is displayed in tasklist" do
       expect(outright_sale.displayed_in_tasklist?(log)).to eq(true)
@@ -130,7 +130,7 @@ RSpec.describe Form::Sales::Subsections::OutrightSale, type: :model do
   end
 
   context "when it is not a outright sale" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 2) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 2) }
 
     it "is displayed in tasklist" do
       expect(outright_sale.displayed_in_tasklist?(log)).to eq(false)
diff --git a/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb b/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb
index 0935ce394..59e7bfbfb 100644
--- a/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb
+++ b/spec/models/form/sales/subsections/shared_ownership_scheme_spec.rb
@@ -77,7 +77,7 @@ RSpec.describe Form::Sales::Subsections::SharedOwnershipScheme, type: :model do
   end
 
   context "when it is a shared ownership scheme" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 1) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 1) }
 
     it "is displayed in tasklist" do
       expect(shared_ownership_scheme.displayed_in_tasklist?(log)).to eq(true)
@@ -85,7 +85,7 @@ RSpec.describe Form::Sales::Subsections::SharedOwnershipScheme, type: :model do
   end
 
   context "when it is not a shared ownership scheme" do
-    let(:log) { FactoryBot.create(:sales_log, ownershipsch: 2) }
+    let(:log) { FactoryBot.build(:sales_log, ownershipsch: 2) }
 
     it "is displayed in tasklist" do
       expect(shared_ownership_scheme.displayed_in_tasklist?(log)).to eq(false)
diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb
index 75fe66833..a04e7d465 100644
--- a/spec/models/scheme_spec.rb
+++ b/spec/models/scheme_spec.rb
@@ -305,7 +305,7 @@ RSpec.describe Scheme, type: :model do
     end
 
     context "when scheme has discarded_at value" do
-      let(:scheme) { FactoryBot.create(:scheme, discarded_at: Time.zone.now) }
+      let(:scheme) { FactoryBot.build(:scheme, discarded_at: Time.zone.now) }
 
       it "returns deleted" do
         expect(scheme.status).to eq(:deleted)
@@ -368,7 +368,7 @@ RSpec.describe Scheme, type: :model do
   describe "owning organisation" do
     let(:stock_owning_org) { FactoryBot.create(:organisation, holds_own_stock: true) }
     let(:non_stock_owning_org) { FactoryBot.create(:organisation, holds_own_stock: false) }
-    let(:scheme) { FactoryBot.create(:scheme, owning_organisation_id: stock_owning_org.id) }
+    let(:scheme) { FactoryBot.build(:scheme, owning_organisation_id: stock_owning_org.id) }
 
     context "when the owning organisation is set as a non-stock-owning organisation" do
       it "throws the correct validation error" do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 05a581ee8..edb998ac3 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -114,6 +114,7 @@ RSpec.describe User, type: :model do
     end
 
     it "can have one or more legacy users" do
+      FactoryBot.create(:legacy_user, old_user_id: user.old_user_id, user:)
       expect(user.legacy_users.size).to eq(1)
     end
 
diff --git a/spec/models/validations/date_validations_spec.rb b/spec/models/validations/date_validations_spec.rb
index 75907b5bc..59be2537e 100644
--- a/spec/models/validations/date_validations_spec.rb
+++ b/spec/models/validations/date_validations_spec.rb
@@ -4,7 +4,7 @@ RSpec.describe Validations::DateValidations do
   subject(:date_validator) { validator_class.new }
 
   let(:validator_class) { Class.new { include Validations::DateValidations } }
-  let(:record) { create(:lettings_log) }
+  let(:record) { build(:lettings_log) }
   let(:scheme) { create(:scheme, end_date: Time.zone.today - 5.days) }
   let(:scheme_no_end_date) { create(:scheme, end_date: nil) }
 
diff --git a/spec/models/validations/local_authority_validations_spec.rb b/spec/models/validations/local_authority_validations_spec.rb
index 3b903e4dc..6344bf427 100644
--- a/spec/models/validations/local_authority_validations_spec.rb
+++ b/spec/models/validations/local_authority_validations_spec.rb
@@ -4,7 +4,7 @@ RSpec.describe Validations::LocalAuthorityValidations do
   subject(:local_auth_validator) { validator_class.new }
 
   let(:validator_class) { Class.new { include Validations::LocalAuthorityValidations } }
-  let(:log) { create(:lettings_log) }
+  let(:log) { build(:lettings_log) }
 
   describe "#validate_previous_accommodation_postcode" do
     it "does not add an error if the log ppostcode_full is missing" do
diff --git a/spec/models/validations/sales/soft_validations_spec.rb b/spec/models/validations/sales/soft_validations_spec.rb
index 8ce602d36..c8e8618fc 100644
--- a/spec/models/validations/sales/soft_validations_spec.rb
+++ b/spec/models/validations/sales/soft_validations_spec.rb
@@ -1,7 +1,7 @@
 require "rails_helper"
 
 RSpec.describe Validations::Sales::SoftValidations do
-  let(:record) { create(:sales_log) }
+  let(:record) { build(:sales_log) }
 
   describe "income1 min validations" do
     context "when validating soft min" do
diff --git a/spec/models/validations/setup_validations_spec.rb b/spec/models/validations/setup_validations_spec.rb
index 5e7705c6d..7fc4dd982 100644
--- a/spec/models/validations/setup_validations_spec.rb
+++ b/spec/models/validations/setup_validations_spec.rb
@@ -4,7 +4,7 @@ RSpec.describe Validations::SetupValidations do
   subject(:setup_validator) { setup_validator_class.new }
 
   let(:setup_validator_class) { Class.new { include Validations::SetupValidations } }
-  let(:record) { create(:lettings_log) }
+  let(:record) { build(:lettings_log) }
 
   describe "tenancy start date" do
     context "when in 22/23 collection" do
@@ -853,7 +853,7 @@ RSpec.describe Validations::SetupValidations do
     end
 
     context "when updating" do
-      let(:log) { create(:lettings_log, :in_progress) }
+      let(:log) { build(:lettings_log, :in_progress) }
       let(:org_with_dpc) { create(:organisation) }
       let(:org_without_dpc) { create(:organisation, :without_dpc) }
 
diff --git a/spec/models/validations/tenancy_validations_spec.rb b/spec/models/validations/tenancy_validations_spec.rb
index f5ffc05b0..a41752616 100644
--- a/spec/models/validations/tenancy_validations_spec.rb
+++ b/spec/models/validations/tenancy_validations_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Validations::TenancyValidations do
   let(:validator_class) { Class.new { include Validations::TenancyValidations } }
 
   describe "tenancy length validations" do
-    let(:record) { FactoryBot.create(:lettings_log, :setup_completed) }
+    let(:record) { FactoryBot.build(:lettings_log, :setup_completed) }
 
     shared_examples "adds expected errors based on the tenancy length" do |tenancy_type_case, error_fields, min_tenancy_length|
       context "and tenancy type is #{tenancy_type_case[:name]}" do
@@ -276,7 +276,7 @@ RSpec.describe Validations::TenancyValidations do
   end
 
   describe "tenancy type validations" do
-    let(:record) { FactoryBot.create(:lettings_log, :setup_completed) }
+    let(:record) { FactoryBot.build(:lettings_log, :setup_completed) }
     let(:field) { "validations.other_field_missing" }
     let(:main_field_label) { "tenancy type" }
     let(:other_field) { "tenancyother" }
@@ -320,7 +320,7 @@ RSpec.describe Validations::TenancyValidations do
 
   describe "joint tenancy validation" do
     context "when the data inputter has said that there is only one member in the household" do
-      let(:record) { FactoryBot.create(:lettings_log, :setup_completed, hhmemb: 1) }
+      let(:record) { FactoryBot.build(:lettings_log, :setup_completed, hhmemb: 1) }
       let(:expected_error) { I18n.t("validations.tenancy.not_joint") }
       let(:hhmemb_expected_error) { I18n.t("validations.tenancy.joint_more_than_one_member") }
 
diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb
index e35e078e4..e2266cb48 100644
--- a/spec/policies/user_policy_spec.rb
+++ b/spec/policies/user_policy_spec.rb
@@ -201,7 +201,7 @@ RSpec.describe UserPolicy do
       end
 
       context "and user is the DPO that hasn't signed the agreement" do
-        let(:user) { create(:user, active: false, is_dpo: true) }
+        let(:user) { create(:user, active: false, is_dpo: true, with_dsa: false) }
 
         it "does not allow deleting a user as a provider" do
           expect(policy).not_to permit(data_provider, user)
diff --git a/spec/requests/bulk_upload_lettings_logs_controller_spec.rb b/spec/requests/bulk_upload_lettings_logs_controller_spec.rb
index d9a9a508d..18b208e74 100644
--- a/spec/requests/bulk_upload_lettings_logs_controller_spec.rb
+++ b/spec/requests/bulk_upload_lettings_logs_controller_spec.rb
@@ -13,7 +13,7 @@ RSpec.describe BulkUploadLettingsLogsController, type: :request do
   describe "GET /lettings-logs/bulk-upload-logs/start" do
     context "when data protection confirmation not signed" do
       let(:organisation) { create(:organisation, :without_dpc) }
-      let(:user) { create(:user, organisation:) }
+      let(:user) { create(:user, organisation:, with_dsa: false) }
 
       it "redirects to lettings index page" do
         get "/lettings-logs/bulk-upload-logs/start", params: {}
diff --git a/spec/requests/bulk_upload_sales_logs_controller_spec.rb b/spec/requests/bulk_upload_sales_logs_controller_spec.rb
index 6fcf7c15d..7cd6d4be8 100644
--- a/spec/requests/bulk_upload_sales_logs_controller_spec.rb
+++ b/spec/requests/bulk_upload_sales_logs_controller_spec.rb
@@ -13,7 +13,7 @@ RSpec.describe BulkUploadSalesLogsController, type: :request do
   describe "GET /sales-logs/bulk-upload-logs/start" do
     context "when data protection confirmation not signed" do
       let(:organisation) { create(:organisation, :without_dpc) }
-      let(:user) { create(:user, organisation:) }
+      let(:user) { create(:user, organisation:, with_dsa: false) }
 
       it "redirects to sales index page" do
         get "/sales-logs/bulk-upload-logs/start", params: {}
diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb
index 68f69d9f2..e6fb20ac7 100644
--- a/spec/requests/organisations_controller_spec.rb
+++ b/spec/requests/organisations_controller_spec.rb
@@ -1648,6 +1648,7 @@ RSpec.describe OrganisationsController, type: :request do
             allow(notify_client).to receive(:send_email).and_return(true)
 
             user_to_reactivate = create(:user, :data_coordinator, organisation:, active: false, reactivate_with_organisation: true)
+            FactoryBot.create(:legacy_user, old_user_id: user_to_reactivate.old_user_id, user: user_to_reactivate)
             user_not_to_reactivate = create(:user, :data_coordinator, organisation:, active: false, reactivate_with_organisation: false)
             patch "/organisations/#{organisation.id}", headers:, params:
           end
@@ -2144,7 +2145,7 @@ RSpec.describe OrganisationsController, type: :request do
             Timecop.unfreeze
           end
 
-          let(:user) { create(:user, is_dpo: true, organisation:) }
+          let(:user) { create(:user, is_dpo: true, organisation:, with_dsa: false) }
 
           it "returns redirects to details page" do
             post "/organisations/#{organisation.id}/data-sharing-agreement", headers: headers
diff --git a/spec/views/logs/_create_for_org_actions.html.erb_spec.rb b/spec/views/logs/_create_for_org_actions.html.erb_spec.rb
index 1f2918218..e82cb8d27 100644
--- a/spec/views/logs/_create_for_org_actions.html.erb_spec.rb
+++ b/spec/views/logs/_create_for_org_actions.html.erb_spec.rb
@@ -20,7 +20,7 @@ RSpec.describe "logs/_create_for_org_actions.html.erb" do
   end
 
   context "without data sharing agreement" do
-    let(:user) { create(:user, organisation: create(:organisation, :without_dpc)) }
+    let(:user) { create(:user, organisation: create(:organisation, :without_dpc), with_dsa: false) }
 
     it "does not include create log buttons" do
       render
diff --git a/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb b/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb
index 3c57dcf8a..8bcb29c9d 100644
--- a/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb
+++ b/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe "organisations/data_sharing_agreement.html.erb", :aggregate_failu
   let(:data_protection_confirmation) { nil }
 
   context "when dpo" do
-    let(:user) { create(:user, is_dpo: true, organisation: create(:organisation, :without_dpc)) }
+    let(:user) { create(:user, is_dpo: true, organisation: create(:organisation, :without_dpc), with_dsa: false) }
 
     it "renders dynamic content" do
       render
diff --git a/spec/views/organisations/show.html.erb_spec.rb b/spec/views/organisations/show.html.erb_spec.rb
index 4314360dc..6c9990317 100644
--- a/spec/views/organisations/show.html.erb_spec.rb
+++ b/spec/views/organisations/show.html.erb_spec.rb
@@ -11,7 +11,7 @@ RSpec.describe "organisations/show.html.erb" do
   let(:organisation_with_dsa) { create(:organisation) }
 
   context "when dpo" do
-    let(:user) { create(:user, is_dpo: true, organisation: organisation_without_dpc) }
+    let(:user) { create(:user, is_dpo: true, organisation: organisation_without_dpc, with_dsa: false) }
 
     it "includes data sharing agreement row" do
       render
@@ -32,7 +32,7 @@ RSpec.describe "organisations/show.html.erb" do
     end
 
     context "when accepted" do
-      let(:user) { create(:user, organisation: organisation_with_dsa) }
+      let(:user) { create(:user, organisation: organisation_with_dsa, with_dsa: false) }
 
       it "includes data sharing agreement row" do
         render
@@ -55,7 +55,7 @@ RSpec.describe "organisations/show.html.erb" do
   end
 
   context "when support user" do
-    let(:user) { create(:user, :support, organisation: organisation_without_dpc) }
+    let(:user) { create(:user, :support, organisation: organisation_without_dpc, with_dsa: false) }
 
     it "includes data sharing agreement row" do
       render
@@ -82,7 +82,7 @@ RSpec.describe "organisations/show.html.erb" do
     end
 
     context "when accepted" do
-      let(:user) { create(:user, :support, organisation: organisation_with_dsa) }
+      let(:user) { create(:user, :support, organisation: organisation_with_dsa, with_dsa: false) }
 
       it "includes data sharing agreement row" do
         render
@@ -125,7 +125,7 @@ RSpec.describe "organisations/show.html.erb" do
   end
 
   context "when not dpo" do
-    let(:user) { create(:user, organisation: organisation_without_dpc) }
+    let(:user) { create(:user, organisation: organisation_without_dpc, with_dsa: false) }
 
     it "includes data sharing agreement row" do
       render
@@ -149,7 +149,7 @@ RSpec.describe "organisations/show.html.erb" do
     end
 
     context "when accepted" do
-      let(:user) { create(:user, organisation: organisation_with_dsa) }
+      let(:user) { create(:user, organisation: organisation_with_dsa, with_dsa: false) }
 
       it "includes data sharing agreement row" do
         render