diff --git a/db/migrate/20250203130048_add_fk_organization_users_organizations.rb b/db/migrate/20250203130048_add_fk_organization_users_organizations.rb new file mode 100644 index 0000000000000000000000000000000000000000..cb923daff0061e0d451fed665e798e67fff66c12 --- /dev/null +++ b/db/migrate/20250203130048_add_fk_organization_users_organizations.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddFkOrganizationUsersOrganizations < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.10' + + def up + add_concurrent_foreign_key :organization_users, :organizations, column: :organization_id, on_delete: :restrict, + validate: false + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :organization_users, column: :organization_id + end + end +end diff --git a/db/schema_migrations/20250203130048 b/db/schema_migrations/20250203130048 new file mode 100644 index 0000000000000000000000000000000000000000..50454c1b0f35df9c8bd1d3e0e19f04688fcd33d3 --- /dev/null +++ b/db/schema_migrations/20250203130048 @@ -0,0 +1 @@ +fcf6107996b68031f34dd3999ee18f164fcb26728efd80c40e8ab3d18f88c570 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 62156c0ff99fe947af726f374046b115c2260df2..112a34b74ef9d36fae1de74759c57e9df55463c3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -39764,6 +39764,9 @@ ALTER TABLE ONLY packages_npm_metadata ALTER TABLE ONLY push_rules ADD CONSTRAINT fk_83b29894de FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY organization_users + ADD CONSTRAINT fk_8471abad75 FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE RESTRICT NOT VALID; + ALTER TABLE ONLY merge_request_diffs ADD CONSTRAINT fk_8483f3258f FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; diff --git a/ee/spec/models/dependencies/dependency_list_export/part_spec.rb b/ee/spec/models/dependencies/dependency_list_export/part_spec.rb index 26be54d51f9a3750bdb9864d08901b46a5ac2425..70024079972e67d3bd775b95cc994065d39eed7e 100644 --- a/ee/spec/models/dependencies/dependency_list_export/part_spec.rb +++ b/ee/spec/models/dependencies/dependency_list_export/part_spec.rb @@ -53,6 +53,10 @@ it_behaves_like 'cleanup by a loose foreign key' do let_it_be(:parent) { create(:organization) } let_it_be(:model) { create(:dependency_list_export_part, organization: parent) } + + before do + parent.users.delete_all + end end end diff --git a/ee/spec/models/vulnerabilities/export/part_spec.rb b/ee/spec/models/vulnerabilities/export/part_spec.rb index 942e3c4f7325c836a64e751d2590562aad01172b..ba46abd505e604108d56db84f028a071fc03821a 100644 --- a/ee/spec/models/vulnerabilities/export/part_spec.rb +++ b/ee/spec/models/vulnerabilities/export/part_spec.rb @@ -33,6 +33,10 @@ it_behaves_like 'cleanup by a loose foreign key' do let_it_be(:parent) { create(:organization) } let_it_be(:model) { create(:vulnerability_export_part, organization: parent) } + + before do + parent.users.delete_all + end end end diff --git a/ee/spec/models/vulnerabilities/export_spec.rb b/ee/spec/models/vulnerabilities/export_spec.rb index b018128c21b2a9fd21b33f36ad8ef2fbba7875bd..c4398aa9e81d6d404ad31e1276a6cc5cc72b505f 100644 --- a/ee/spec/models/vulnerabilities/export_spec.rb +++ b/ee/spec/models/vulnerabilities/export_spec.rb @@ -295,6 +295,10 @@ it_behaves_like 'cleanup by a loose foreign key' do let_it_be(:parent) { create(:organization) } let_it_be(:model) { create(:vulnerability_export, organization: parent) } + + before do + parent.users.delete_all + end end end diff --git a/lib/tasks/gitlab/update_templates.rake b/lib/tasks/gitlab/update_templates.rake index 7b48cdbed63cad900e9f657cc918f8e54c97fd94..d0c14d784292910ff8dd34e626cf597b90469ea1 100644 --- a/lib/tasks/gitlab/update_templates.rake +++ b/lib/tasks/gitlab/update_templates.rake @@ -123,6 +123,7 @@ namespace :gitlab do if tmp_organization puts "Destroying temporary organization #{tmp_organization_path}" + tmp_organization.users.delete_all tmp_organization.destroy end diff --git a/spec/models/organizations/organization_user_spec.rb b/spec/models/organizations/organization_user_spec.rb index 0f0e0159ed1f73915b5db59fb03d1c1c41d0baca..98782eccc1c275cf223a8e389e9022a1a8d1a32d 100644 --- a/spec/models/organizations/organization_user_spec.rb +++ b/spec/models/organizations/organization_user_spec.rb @@ -83,10 +83,14 @@ end end - context 'with loose foreign key on organization_users.organization_id' do - it_behaves_like 'cleanup by a loose foreign key' do - let_it_be(:parent) { create(:organization) } - let_it_be(:model) { create(:organization_user, organization: parent) } + context 'with foreign key on organization_users.organization_id' do + let!(:organization) { create(:organization) } + let!(:organization_user) { create(:organization_user, organization: organization) } + + context 'and organization is attempted to be destroyed' do + it 'prevents destroying the organization' do + expect { organization.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey) + end end end