From a996deb1cb2cb137937e6994ac62c1d63cb401b1 Mon Sep 17 00:00:00 2001 From: Ted-U <92022120+Ted-U@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:31:35 +0100 Subject: [PATCH] CLDC-1315: Model deletion cascade and foreign key constraints (#765) * failed rebase, copied onto new branch * lint * fix migration file and lint * fix migration - remove duplication * fix migration - remove duplicate foreign key * fix migration - remove duplication * lint * migration fix * fix migration and lint * keep foreign key for managing organisations --- app/models/organisation.rb | 6 ++-- app/models/scheme.rb | 4 +-- app/models/user.rb | 2 +- ...14646_add_delete_cascade_from_orgs_down.rb | 6 ++++ ...0220722133738_add_delete_caselogs_users.rb | 6 ++++ db/schema.rb | 5 +-- spec/features/organisation_spec.rb | 33 +++++++++++++++++++ 7 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20220720214646_add_delete_cascade_from_orgs_down.rb create mode 100644 db/migrate/20220722133738_add_delete_caselogs_users.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 3cae243c2..7970c7c37 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -1,10 +1,10 @@ class Organisation < ApplicationRecord - has_many :users - has_many :owned_case_logs, class_name: "CaseLog", foreign_key: "owning_organisation_id" + has_many :users, dependent: :delete_all + has_many :owned_case_logs, class_name: "CaseLog", foreign_key: "owning_organisation_id", dependent: :delete_all has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id" has_many :data_protection_confirmations has_many :organisation_rent_periods - has_many :owned_schemes, class_name: "Scheme", foreign_key: "owning_organisation_id" + has_many :owned_schemes, class_name: "Scheme", foreign_key: "owning_organisation_id", dependent: :delete_all has_many :managed_schemes, class_name: "Scheme", foreign_key: "managing_organisation_id" scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } diff --git a/app/models/scheme.rb b/app/models/scheme.rb index e899a31d4..727388fae 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -1,8 +1,8 @@ class Scheme < ApplicationRecord belongs_to :owning_organisation, class_name: "Organisation" belongs_to :managing_organisation, optional: true, class_name: "Organisation" - has_many :locations - has_many :case_logs + has_many :locations, dependent: :delete_all + has_many :case_logs, dependent: :delete_all has_paper_trail diff --git a/app/models/user.rb b/app/models/user.rb index a664acc8d..ee1a2fb5f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,7 +6,7 @@ class User < ApplicationRecord :trackable, :lockable, :two_factor_authenticatable, :confirmable, :timeoutable belongs_to :organisation - has_many :owned_case_logs, through: :organisation + has_many :owned_case_logs, through: :organisation, dependent: :delete_all has_many :managed_case_logs, through: :organisation validate :validate_email diff --git a/db/migrate/20220720214646_add_delete_cascade_from_orgs_down.rb b/db/migrate/20220720214646_add_delete_cascade_from_orgs_down.rb new file mode 100644 index 000000000..21a73077a --- /dev/null +++ b/db/migrate/20220720214646_add_delete_cascade_from_orgs_down.rb @@ -0,0 +1,6 @@ +class AddDeleteCascadeFromOrgsDown < ActiveRecord::Migration[7.0] + def change + remove_foreign_key :schemes, :organisations, column: "owning_organisation_id" + add_foreign_key :schemes, :organisations, column: "owning_organisation_id", on_delete: :cascade + end +end diff --git a/db/migrate/20220722133738_add_delete_caselogs_users.rb b/db/migrate/20220722133738_add_delete_caselogs_users.rb new file mode 100644 index 000000000..5f3d3318f --- /dev/null +++ b/db/migrate/20220722133738_add_delete_caselogs_users.rb @@ -0,0 +1,6 @@ +class AddDeleteCaselogsUsers < ActiveRecord::Migration[7.0] + def change + add_foreign_key :case_logs, :organisations, column: "owning_organisation_id", on_delete: :cascade + add_foreign_key :users, :organisations, on_delete: :cascade + end +end diff --git a/db/schema.rb b/db/schema.rb index 417f03725..aa7304022 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -380,8 +380,9 @@ ActiveRecord::Schema[7.0].define(version: 2022_08_02_125711) do end add_foreign_key "case_logs", "locations" + add_foreign_key "case_logs", "organisations", column: "owning_organisation_id", on_delete: :cascade add_foreign_key "case_logs", "schemes" add_foreign_key "locations", "schemes" - add_foreign_key "schemes", "organisations", column: "managing_organisation_id" - add_foreign_key "schemes", "organisations", column: "owning_organisation_id" + add_foreign_key "schemes", "organisations", column: "owning_organisation_id", on_delete: :cascade + add_foreign_key "users", "organisations", on_delete: :cascade end diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 118638d84..ee7ec0c37 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -218,6 +218,39 @@ RSpec.describe "User Features" do end end end + + describe "delete cascade" do + context "when the organisation is deleted" do + let!(:organisation) { FactoryBot.create(:organisation) } + let!(:user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } + let!(:scheme_to_delete) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:log_to_delete) { FactoryBot.create(:case_log, owning_organisation: user.organisation) } + + context "when organisation is deleted" do + it "child relationships ie logs, schemes and users are deleted too - application" do + organisation.destroy! + expect { organisation.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { CaseLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + context "when the organisation is deleted" do + let!(:organisation) { FactoryBot.create(:organisation) } + let!(:user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } + let!(:scheme_to_delete) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:log_to_delete) { FactoryBot.create(:case_log, :in_progress, needstype: 1, owning_organisation: user.organisation) } + + it "child relationships ie logs, schemes and users are deleted too - database" do + ActiveRecord::Base.connection.exec_query("DELETE FROM organisations WHERE id = #{organisation.id};") + expect { CaseLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + end + end end end end