From c607b8a323455d28d711c92cc539c3d48a6819a6 Mon Sep 17 00:00:00 2001 From: wandji Date: Mon, 8 Jan 2024 12:01:06 +0100 Subject: [PATCH] Improve migration for repository_row_connections [SCI-9908] (#6863) --- app/helpers/database_helper.rb | 22 ------------------- ...22085412_add_repository_row_connections.rb | 13 +++++++---- db/schema.rb | 9 ++++---- lib/tasks/sign_up_constraint.rake | 15 +++++-------- spec/models/repository_row_connection_spec.rb | 1 - 5 files changed, 20 insertions(+), 40 deletions(-) diff --git a/app/helpers/database_helper.rb b/app/helpers/database_helper.rb index fbda4a84f..6e228fbab 100644 --- a/app/helpers/database_helper.rb +++ b/app/helpers/database_helper.rb @@ -60,26 +60,4 @@ module DatabaseHelper "AS t WHERE t.id = #{id};" ).getvalue(0, 0).to_i end - - # Adds a check constraint to the table - def add_check_constraint(table, constraint_name, constraint) - ActiveRecord::Base.connection.execute( - "ALTER TABLE " \ - "#{table} " \ - "DROP CONSTRAINT IF EXISTS #{constraint_name}, " \ - "ADD CONSTRAINT " \ - "#{constraint_name} " \ - "CHECK ( #{constraint} ) " \ - "not valid;" - ) - end - - # Remove constraint from the table - def drop_constraint(table, constraint_name) - ActiveRecord::Base.connection.execute( - "ALTER TABLE " \ - "#{table} " \ - "DROP CONSTRAINT IF EXISTS #{constraint_name}; " - ) - end end diff --git a/db/migrate/20230922085412_add_repository_row_connections.rb b/db/migrate/20230922085412_add_repository_row_connections.rb index c5979fe7e..12504896b 100644 --- a/db/migrate/20230922085412_add_repository_row_connections.rb +++ b/db/migrate/20230922085412_add_repository_row_connections.rb @@ -3,19 +3,24 @@ class AddRepositoryRowConnections < ActiveRecord::Migration[7.0] def change create_table :repository_row_connections do |t| - t.references :parent, index: true, foreign_key: { to_table: :repository_rows } - t.references :child, index: true, foreign_key: { to_table: :repository_rows } + t.references :parent, index: true, foreign_key: { to_table: :repository_rows }, null: false + t.references :child, index: true, foreign_key: { to_table: :repository_rows }, null: false t.references :created_by, foreign_key: { to_table: :users } t.references :last_modified_by, foreign_key: { to_table: :users } t.timestamps end - add_index :repository_row_connections, %i(parent_id child_id), unique: true - change_table :repository_rows, bulk: true do |t| t.integer :parent_connections_count t.integer :child_connections_count end + + add_index :repository_row_connections, + 'LEAST(parent_id, child_id), GREATEST(parent_id, child_id)', + name: 'index_repository_row_connections_on_connection_pair', + unique: true + add_check_constraint :repository_row_connections, 'parent_id != child_id', + name: 'constraint_repository_row_connections_on_self_connection' end end diff --git a/db/schema.rb b/db/schema.rb index 6ccee7679..ea04169c0 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: 2023_11_28_123835) do +ActiveRecord::Schema[7.0].define(version: 2023_12_07_092907) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gist" enable_extension "pg_trgm" @@ -792,17 +792,18 @@ ActiveRecord::Schema[7.0].define(version: 2023_11_28_123835) do end create_table "repository_row_connections", force: :cascade do |t| - t.bigint "parent_id" - t.bigint "child_id" + t.bigint "parent_id", null: false + t.bigint "child_id", null: false t.bigint "created_by_id" t.bigint "last_modified_by_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index "LEAST(parent_id, child_id), GREATEST(parent_id, child_id)", name: "index_repository_row_connections_on_connection_pair", unique: true t.index ["child_id"], name: "index_repository_row_connections_on_child_id" t.index ["created_by_id"], name: "index_repository_row_connections_on_created_by_id" t.index ["last_modified_by_id"], name: "index_repository_row_connections_on_last_modified_by_id" - t.index ["parent_id", "child_id"], name: "index_repository_row_connections_on_parent_id_and_child_id", unique: true t.index ["parent_id"], name: "index_repository_row_connections_on_parent_id" + t.check_constraint "parent_id <> child_id", name: "constraint_repository_row_connections_on_self_connection" end create_table "repository_rows", force: :cascade do |t| diff --git a/lib/tasks/sign_up_constraint.rake b/lib/tasks/sign_up_constraint.rake index b85b64dba..fcdeb081d 100644 --- a/lib/tasks/sign_up_constraint.rake +++ b/lib/tasks/sign_up_constraint.rake @@ -2,8 +2,6 @@ namespace :sign_up_constraint do desc 'Adds email domain constraint to the users table. '\ 'E.g: scinote.net' task :email_domain, [:domain] => :environment do |_, args| - include DatabaseHelper - if args.blank? || args[:domain].blank? puts 'Please add the email domain' return @@ -12,19 +10,18 @@ namespace :sign_up_constraint do domain = args[:domain] domain = domain.strip.gsub(/\./, '\\.') - add_check_constraint( - 'users', - 'email_must_be_company_email', - "email ~* '^[A-Za-z0-9._%-+]+@#{domain}'" + ActiveRecord::Migration.add_check_constraint( + :users, + "email ~* '^[A-Za-z0-9._%-+]+@#{domain}'", + name: 'email_must_be_company_email', + validate: false ) puts "Created the following domain constraint: #{args[:domain]}" end desc 'Remove email domain constraint from the users table.' task remove_domain: :environment do - include DatabaseHelper - - drop_constraint('users', 'email_must_be_company_email') + ActiveRecord::Migration.remove_check_constraint :users, name: 'email_must_be_company_email' puts 'Email constraint has been removed' end end diff --git a/spec/models/repository_row_connection_spec.rb b/spec/models/repository_row_connection_spec.rb index 63110814b..9bf615589 100644 --- a/spec/models/repository_row_connection_spec.rb +++ b/spec/models/repository_row_connection_spec.rb @@ -12,7 +12,6 @@ RSpec.describe RepositoryRowConnection, type: :model do it { should have_db_column(:child_id).of_type(:integer) } it { should have_db_column(:created_by_id).of_type(:integer) } it { should have_db_column(:last_modified_by_id).of_type(:integer) } - it { should have_db_index([:parent_id, :child_id]).unique(true) } it { should have_db_index(:parent_id) } it { should have_db_index(:child_id) } it { should have_db_index(:created_by_id) }