From 1d880812e98ef29ae2dbf2e1f2382df8416bd445 Mon Sep 17 00:00:00 2001 From: Ted-U Date: Tue, 19 Jul 2022 16:55:53 +0100 Subject: [PATCH] fix foreign key for org/scheme --- app/models/organisation.rb | 6 +++--- app/models/scheme.rb | 4 ++-- app/models/user.rb | 2 +- ...dd_stock_owning_organisation_to_schemes.rb | 2 +- .../20220719122509_add_delete_cascade.rb | 15 +++++++++++++ db/schema.rb | 14 +++++-------- spec/features/organisation_spec.rb | 21 +++++++++---------- 7 files changed, 37 insertions(+), 27 deletions(-) create mode 100644 db/migrate/20220719122509_add_delete_cascade.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 2b38d6412..4a3b50f8d 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -1,11 +1,11 @@ class Organisation < ApplicationRecord - has_many :users, dependent: :destroy - has_many :owned_case_logs, class_name: "CaseLog", foreign_key: "owning_organisation_id", dependent: :destroy + 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", dependent: :destroy + 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 bdd9671ed..9e3d98a57 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 scope :filter_by_id, ->(id) { where(id: (id.start_with?("S") ? id[1..] : id)) } scope :search_by_service_name, ->(name) { where("service_name ILIKE ?", "%#{name}%") } diff --git a/app/models/user.rb b/app/models/user.rb index 3a524ecc2..d249eb430 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,7 +5,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: :destroy has_many :managed_case_logs, through: :organisation has_paper_trail ignore: %w[last_sign_in_at diff --git a/db/migrate/20220629105452_add_stock_owning_organisation_to_schemes.rb b/db/migrate/20220629105452_add_stock_owning_organisation_to_schemes.rb index 1623eb654..9a44d8451 100644 --- a/db/migrate/20220629105452_add_stock_owning_organisation_to_schemes.rb +++ b/db/migrate/20220629105452_add_stock_owning_organisation_to_schemes.rb @@ -1,5 +1,5 @@ class AddStockOwningOrganisationToSchemes < ActiveRecord::Migration[7.0] def change - add_reference :schemes, :stock_owning_organisation, foreign_key: { to_table: :organisations } + add_reference :schemes, :stock_owning_organisation, foreign_key: { to_table: :organisations }, column: "owning_organisation_id" end end diff --git a/db/migrate/20220719122509_add_delete_cascade.rb b/db/migrate/20220719122509_add_delete_cascade.rb new file mode 100644 index 000000000..a6220a09e --- /dev/null +++ b/db/migrate/20220719122509_add_delete_cascade.rb @@ -0,0 +1,15 @@ +class AddDeleteCascade < ActiveRecord::Migration[7.0] + def up + remove_foreign_key :schemes, :organisations, column: "managing_organisation_id" + remove_foreign_key :schemes, :organisations, column: "owning_organisation_id" + add_foreign_key :schemes, :organisations, column: "managing_organisation_id", on_delete: :cascade + add_foreign_key :schemes, :organisations, column: "owning_organisation_id", on_delete: :cascade + end + + def down + remove_foreign_key :schemes, :organisations, column: "managing_organisation_id" + remove_foreign_key :schemes, :organisations, column: "owning_organisation_id" + add_foreign_key :schemes, :organisations, column: "managing_organisation_id" + add_foreign_key :schemes, :organisations, column: "owning_organisation_id" + end +end diff --git a/db/schema.rb b/db/schema.rb index 675fed6e7..67130ebba 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,9 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. - -ActiveRecord::Schema[7.0].define(version: 2022_07_15_133937) do - +ActiveRecord::Schema[7.0].define(version: 2022_07_19_122509) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -259,7 +257,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_07_15_133937) do create_table "logs_exports", force: :cascade do |t| t.datetime "created_at", default: -> { "CURRENT_TIMESTAMP" } - t.datetime "started_at", precision: nil, null: false + t.datetime "started_at", null: false t.integer "base_number", default: 1, null: false t.integer "increment_number", default: 1, null: false t.boolean "empty_export", default: false, null: false @@ -301,9 +299,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_07_15_133937) do t.integer "unspecified_units" t.string "old_org_id" t.integer "old_visible_id" - t.bigint "user_id" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true - t.index ["user_id"], name: "index_organisations_on_user_id" end create_table "schemes", force: :cascade do |t| @@ -385,7 +381,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_07_15_133937) do add_foreign_key "case_logs", "locations" add_foreign_key "case_logs", "schemes" add_foreign_key "locations", "schemes" - add_foreign_key "organisations", "users" - add_foreign_key "schemes", "organisations", column: "managing_organisation_id" - add_foreign_key "schemes", "organisations", column: "owning_organisation_id" + add_foreign_key "schemes", "organisations", column: "managing_organisation_id", on_delete: :cascade + 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 322b29db0..9c3668d3c 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -226,7 +226,7 @@ RSpec.describe "User Features" do 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: 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) } @@ -238,22 +238,21 @@ RSpec.describe "User Features" do 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: 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) } - - - 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) + + 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