From 74f4adcd5f7b6c076355ad18cdd3ef3c6a0158de Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Tue, 9 May 2023 11:49:16 +0200 Subject: [PATCH 01/13] Add default organization We need to link everything to an organization. If we can not link a record to a specific organization, we can use the default organization Changelog: changed --- app/models/organization.rb | 17 ++++ ...509085428_change_organizations_sequence.rb | 12 +++ ...20230509115525_add_name_to_organization.rb | 21 +++++ ...20230509131736_add_default_organization.rb | 16 ++++ db/schema_migrations/20230509085428 | 1 + db/schema_migrations/20230509115525 | 1 + db/schema_migrations/20230509131736 | 1 + db/structure.sql | 10 ++- spec/factories/organizations.rb | 9 +- ...509131736_add_default_organization_spec.rb | 20 +++++ spec/models/organization_spec.rb | 88 ++++++++++++++++++- 11 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20230509085428_change_organizations_sequence.rb create mode 100644 db/migrate/20230509115525_add_name_to_organization.rb create mode 100644 db/migrate/20230509131736_add_default_organization.rb create mode 100644 db/schema_migrations/20230509085428 create mode 100644 db/schema_migrations/20230509115525 create mode 100644 db/schema_migrations/20230509131736 create mode 100644 spec/migrations/20230509131736_add_default_organization_spec.rb diff --git a/app/models/organization.rb b/app/models/organization.rb index 73a7e84305fd57..d1f184fd7d0e19 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -2,5 +2,22 @@ # rubocop: disable Gitlab/NamespacedClass class Organization < ApplicationRecord + DEFAULT_ORGANIZATION_ID = 1 + + scope :without_default, -> { where.not(id: DEFAULT_ORGANIZATION_ID) } + + before_destroy :check_if_default_organization + + def default? + id == DEFAULT_ORGANIZATION_ID + end + + private + + def check_if_default_organization + return unless default? + + raise ActiveRecord::RecordNotDestroyed, 'Cannot delete the default organization' + end end # rubocop: enable Gitlab/NamespacedClass diff --git a/db/migrate/20230509085428_change_organizations_sequence.rb b/db/migrate/20230509085428_change_organizations_sequence.rb new file mode 100644 index 00000000000000..8010c9c290733d --- /dev/null +++ b/db/migrate/20230509085428_change_organizations_sequence.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class ChangeOrganizationsSequence < Gitlab::Database::Migration[2.1] + def up + # Modify sequence for organizations.id so id '1' is never automatically taken + execute "ALTER SEQUENCE organizations_id_seq START WITH 2 MINVALUE 2 RESTART" + end + + def down + execute "ALTER SEQUENCE organizations_id_seq START WITH 1 MINVALUE 1" + end +end diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb new file mode 100644 index 00000000000000..574d644629c689 --- /dev/null +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddNameToOrganization < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_organizations_name_unique' + + def up + add_column :organizations, :name, :text, null: false, default: '' + + add_text_limit :organizations, :name, 255 + + add_concurrent_index :organizations, :name, name: INDEX_NAME, unique: true + end + + def down + remove_concurrent_index_by_name :organizations, name: INDEX_NAME + + remove_column :organizations, :name + end +end diff --git a/db/migrate/20230509131736_add_default_organization.rb b/db/migrate/20230509131736_add_default_organization.rb new file mode 100644 index 00000000000000..1c70ed21fb13ce --- /dev/null +++ b/db/migrate/20230509131736_add_default_organization.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddDefaultOrganization < Gitlab::Database::Migration[2.1] + restrict_gitlab_migration gitlab_schema: :gitlab_main + + class Organization < MigrationRecord + end + + def up + Organization.create(id: 1, name: 'Unspecified') + end + + def down + Organization.find_by(id: 1)&.delete + end +end diff --git a/db/schema_migrations/20230509085428 b/db/schema_migrations/20230509085428 new file mode 100644 index 00000000000000..cf7214ceadcb6a --- /dev/null +++ b/db/schema_migrations/20230509085428 @@ -0,0 +1 @@ +6179fe3d8c419c58e028fc1fe5d554678976229eff88f087beec174cb669d4ce \ No newline at end of file diff --git a/db/schema_migrations/20230509115525 b/db/schema_migrations/20230509115525 new file mode 100644 index 00000000000000..e3c0ada40cd037 --- /dev/null +++ b/db/schema_migrations/20230509115525 @@ -0,0 +1 @@ +92b70129d19796653569fb730be43ea6eed7dacbce224e1323124fdf03b0a0b0 \ No newline at end of file diff --git a/db/schema_migrations/20230509131736 b/db/schema_migrations/20230509131736 new file mode 100644 index 00000000000000..593c9495eb237c --- /dev/null +++ b/db/schema_migrations/20230509131736 @@ -0,0 +1 @@ +f9545a27756e5ca05220ffebcf89e8268e0231cbd8c7af0a89d13c70f5a070ec \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 75087462e3a52a..1779eadd4109ee 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19179,13 +19179,15 @@ ALTER SEQUENCE operations_user_lists_id_seq OWNED BY operations_user_lists.id; CREATE TABLE organizations ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL + updated_at timestamp with time zone NOT NULL, + name text DEFAULT ''::text NOT NULL, + CONSTRAINT check_d130d769e0 CHECK ((char_length(name) <= 255)) ); CREATE SEQUENCE organizations_id_seq - START WITH 1 + START WITH 2 INCREMENT BY 1 - NO MINVALUE + MINVALUE 2 NO MAXVALUE CACHE 1; @@ -31668,6 +31670,8 @@ CREATE UNIQUE INDEX index_ops_feature_flags_issues_on_feature_flag_id_and_issue_ CREATE UNIQUE INDEX index_ops_strategies_user_lists_on_strategy_id_and_user_list_id ON operations_strategies_user_lists USING btree (strategy_id, user_list_id); +CREATE UNIQUE INDEX index_organizations_name_unique ON organizations USING btree (name); + CREATE UNIQUE INDEX index_organizations_on_unique_name_per_group ON customer_relations_organizations USING btree (group_id, lower(name), id); CREATE INDEX index_p_ci_runner_machine_builds_on_runner_machine_id ON ONLY p_ci_runner_machine_builds USING btree (runner_machine_id); diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index a6684a8f95f765..0484e47877befe 100644 --- a/spec/factories/organizations.rb +++ b/spec/factories/organizations.rb @@ -1,5 +1,12 @@ # frozen_string_literal: true FactoryBot.define do - factory :organization + factory :organization do + sequence(:name) { |n| "Organization ##{n}" } + + trait :default do + id { 1 } + name { 'Unspecified' } + end + end end diff --git a/spec/migrations/20230509131736_add_default_organization_spec.rb b/spec/migrations/20230509131736_add_default_organization_spec.rb new file mode 100644 index 00000000000000..363227210b8ba6 --- /dev/null +++ b/spec/migrations/20230509131736_add_default_organization_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require_migration! + +RSpec.describe AddDefaultOrganization, feature_category: :cell do + let(:organization) { table(:organizations) } + + it "correctly migrates up and down" do + reversible_migration do |migration| + migration.before -> { + expect(organization.where(id: 1, name: 'Unspecified')).to be_empty + } + migration.after -> { + expect(organization.where(id: 1, name: 'Unspecified')).not_to be_empty + } + end + end +end diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 9966a7132cebf4..a19ccdcde3dc9d 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -2,9 +2,89 @@ require 'spec_helper' -# rubocop: disable Lint/EmptyBlock -# rubocop: disable RSpec/EmptyExampleGroup RSpec.describe Organization, type: :model, feature_category: :cell do + let_it_be(:organization) { create(:organization) } + let_it_be(:default_organization) { create(:organization, :default) } + + context 'when using scopes' do + describe '.without_default' do + it 'excludes default organization' do + expect(described_class.without_default).not_to include(default_organization) + end + + it 'includes other organizations organization' do + expect(described_class.without_default).to include(organization) + end + end + end + + describe '#id' do + context 'when organization is default' do + it 'has id 1' do + expect(default_organization.id).to eq(1) + end + end + + context 'when organization is not default' do + it 'does not have id 1' do + expect(organization.id).not_to eq(1) + end + end + end + + describe '#destroy!' do + context 'when trying to delete the default organization' do + it 'raises an error' do + expect do + default_organization.destroy! + end.to raise_error(ActiveRecord::RecordNotDestroyed, 'Cannot delete the default organization') + end + end + + context 'when trying to delete a non-default organization' do + let(:to_be_removed) { create(:organization) } + + it 'does not raise error' do + expect { to_be_removed.destroy! }.not_to raise_error + end + end + end + + describe '#destroy' do + context 'when trying to delete the default organization' do + it 'returns false' do + expect(default_organization.destroy).to eq(false) + end + end + + context 'when trying to delete a non-default organization' do + let(:to_be_removed) { create(:organization) } + + it 'returns true' do + expect(to_be_removed.destroy).to eq(to_be_removed) + end + end + end + + describe '#default?' do + context 'when organization is default' do + it 'returns true' do + expect(default_organization.default?).to eq(true) + end + end + + context 'when organization is not default' do + it 'returns false' do + expect(organization.default?).to eq(false) + end + end + end + + describe '#name' do + context 'when organization is default' do + it 'returns Unspecified' do + expect(default_organization.name).to eq('Unspecified') + end + end + end end -# rubocop: enable RSpec/EmptyExampleGroup -# rubocop: enable Lint/EmptyBlock -- GitLab From 5969557e9386742a75f2bb02227bf74aa8eac984 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 11 May 2023 14:05:35 +0200 Subject: [PATCH 02/13] Seed default organization in test database --- spec/factories/organizations.rb | 6 +++++- spec/support/helpers/test_env.rb | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index 0484e47877befe..c12c75c9768381 100644 --- a/spec/factories/organizations.rb +++ b/spec/factories/organizations.rb @@ -5,8 +5,12 @@ sequence(:name) { |n| "Organization ##{n}" } trait :default do - id { 1 } + id { Organization::DEFAULT_ORGANIZATION_ID } name { 'Unspecified' } + initialize_with do + # Ensure we only use one default organization + Organization.find_by(id: Organization::DEFAULT_ORGANIZATION_ID) || new(**attributes) + end end end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index a53e1e1002c744..ceb567e54c4337 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -372,6 +372,7 @@ def forked_repo_bundle_path def seed_db Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter.upsert_restrictions + FactoryBot.create(:organization, :default) end private -- GitLab From 011c469134095bbc4c3f8388fd7487284d453602 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 11 May 2023 14:42:28 +0200 Subject: [PATCH 03/13] Add validations for Organization.name attribute --- app/models/organization.rb | 5 +++++ db/migrate/20230509115525_add_name_to_organization.rb | 2 +- db/structure.sql | 2 +- spec/models/organization_spec.rb | 8 ++++++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/organization.rb b/app/models/organization.rb index d1f184fd7d0e19..826378aaa6d752 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -8,6 +8,11 @@ class Organization < ApplicationRecord before_destroy :check_if_default_organization + validates :name, + presence: true, + length: { maximum: 255 }, + uniqueness: { case_sensitive: false } + def default? id == DEFAULT_ORGANIZATION_ID end diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb index 574d644629c689..55135f90cfd4ab 100644 --- a/db/migrate/20230509115525_add_name_to_organization.rb +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -10,7 +10,7 @@ def up add_text_limit :organizations, :name, 255 - add_concurrent_index :organizations, :name, name: INDEX_NAME, unique: true + add_concurrent_index :organizations, 'lower(name)', name: INDEX_NAME, unique: true end def down diff --git a/db/structure.sql b/db/structure.sql index 1779eadd4109ee..2e0f2b14d34ef0 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31670,7 +31670,7 @@ CREATE UNIQUE INDEX index_ops_feature_flags_issues_on_feature_flag_id_and_issue_ CREATE UNIQUE INDEX index_ops_strategies_user_lists_on_strategy_id_and_user_list_id ON operations_strategies_user_lists USING btree (strategy_id, user_list_id); -CREATE UNIQUE INDEX index_organizations_name_unique ON organizations USING btree (name); +CREATE UNIQUE INDEX index_organizations_name_unique ON organizations USING btree (lower(name)); CREATE UNIQUE INDEX index_organizations_on_unique_name_per_group ON customer_relations_organizations USING btree (group_id, lower(name), id); diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index a19ccdcde3dc9d..67821905c8edaf 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -6,6 +6,14 @@ let_it_be(:organization) { create(:organization) } let_it_be(:default_organization) { create(:organization, :default) } + describe 'validations' do + subject { create(:organization) } + + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).case_insensitive } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + end + context 'when using scopes' do describe '.without_default' do it 'excludes default organization' do -- GitLab From 2c6393ed2b542982dea033fee75a7fd87d8cdca7 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 13:17:49 +0200 Subject: [PATCH 04/13] Split off text_limit migration --- .../20230509115525_add_name_to_organization.rb | 5 +++-- ...515111314_add_text_limit_on_organization_name.rb | 13 +++++++++++++ db/schema_migrations/20230515111314 | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20230515111314_add_text_limit_on_organization_name.rb create mode 100644 db/schema_migrations/20230515111314 diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb index 55135f90cfd4ab..be1b0492a1c32f 100644 --- a/db/migrate/20230509115525_add_name_to_organization.rb +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +# rubocop:disable Migration/AddLimitToTextColumns +# limit is added in 20230515111314_add_text_limit_on_organization_name.rb class AddNameToOrganization < Gitlab::Database::Migration[2.1] disable_ddl_transaction! @@ -8,8 +10,6 @@ class AddNameToOrganization < Gitlab::Database::Migration[2.1] def up add_column :organizations, :name, :text, null: false, default: '' - add_text_limit :organizations, :name, 255 - add_concurrent_index :organizations, 'lower(name)', name: INDEX_NAME, unique: true end @@ -19,3 +19,4 @@ def down remove_column :organizations, :name end end +# rubocop:enable Migration/AddLimitToTextColumns diff --git a/db/migrate/20230515111314_add_text_limit_on_organization_name.rb b/db/migrate/20230515111314_add_text_limit_on_organization_name.rb new file mode 100644 index 00000000000000..c0b687fab94c4c --- /dev/null +++ b/db/migrate/20230515111314_add_text_limit_on_organization_name.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddTextLimitOnOrganizationName < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + def up + add_text_limit :organizations, :name, 255 + end + + def down + remove_text_limit :organizations, :name + end +end diff --git a/db/schema_migrations/20230515111314 b/db/schema_migrations/20230515111314 new file mode 100644 index 00000000000000..d2d7d2c94c4aaa --- /dev/null +++ b/db/schema_migrations/20230515111314 @@ -0,0 +1 @@ +2a011d12459e0c21832df777569a12f4f2bbdaa5f57da7dc3823147f948d7772 \ No newline at end of file -- GitLab From 2202612c6cc0f82fcb5f162819ea7e4d8ac3d424 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 13:49:41 +0200 Subject: [PATCH 05/13] Rename default organization from 'Unspecified' to 'Default' --- db/migrate/20230509131736_add_default_organization.rb | 2 +- spec/factories/organizations.rb | 2 +- .../20230509131736_add_default_organization_spec.rb | 4 ++-- spec/models/organization_spec.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/db/migrate/20230509131736_add_default_organization.rb b/db/migrate/20230509131736_add_default_organization.rb index 1c70ed21fb13ce..555b5ca76f8683 100644 --- a/db/migrate/20230509131736_add_default_organization.rb +++ b/db/migrate/20230509131736_add_default_organization.rb @@ -7,7 +7,7 @@ class Organization < MigrationRecord end def up - Organization.create(id: 1, name: 'Unspecified') + Organization.create(id: 1, name: 'Default') end def down diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index c12c75c9768381..7ff0493d140b29 100644 --- a/spec/factories/organizations.rb +++ b/spec/factories/organizations.rb @@ -6,7 +6,7 @@ trait :default do id { Organization::DEFAULT_ORGANIZATION_ID } - name { 'Unspecified' } + name { 'Default' } initialize_with do # Ensure we only use one default organization Organization.find_by(id: Organization::DEFAULT_ORGANIZATION_ID) || new(**attributes) diff --git a/spec/migrations/20230509131736_add_default_organization_spec.rb b/spec/migrations/20230509131736_add_default_organization_spec.rb index 363227210b8ba6..539216c57eed8b 100644 --- a/spec/migrations/20230509131736_add_default_organization_spec.rb +++ b/spec/migrations/20230509131736_add_default_organization_spec.rb @@ -10,10 +10,10 @@ it "correctly migrates up and down" do reversible_migration do |migration| migration.before -> { - expect(organization.where(id: 1, name: 'Unspecified')).to be_empty + expect(organization.where(id: 1, name: 'Default')).to be_empty } migration.after -> { - expect(organization.where(id: 1, name: 'Unspecified')).not_to be_empty + expect(organization.where(id: 1, name: 'Default')).not_to be_empty } end end diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 67821905c8edaf..832ed5fd50a65f 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -90,8 +90,8 @@ describe '#name' do context 'when organization is default' do - it 'returns Unspecified' do - expect(default_organization.name).to eq('Unspecified') + it 'returns Default' do + expect(default_organization.name).to eq('Default') end end end -- GitLab From 4aac5f0571d52b55e4796396e6b5ead7841ccfb1 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 14:06:04 +0200 Subject: [PATCH 06/13] Use proper name for unique index --- db/migrate/20230509115525_add_name_to_organization.rb | 2 +- db/structure.sql | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb index be1b0492a1c32f..1d54b464a18d6a 100644 --- a/db/migrate/20230509115525_add_name_to_organization.rb +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -5,7 +5,7 @@ class AddNameToOrganization < Gitlab::Database::Migration[2.1] disable_ddl_transaction! - INDEX_NAME = 'index_organizations_name_unique' + INDEX_NAME = 'unique_organizations_on_name_lower' def up add_column :organizations, :name, :text, null: false, default: '' diff --git a/db/structure.sql b/db/structure.sql index 2e0f2b14d34ef0..19c19248b3dbad 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31670,8 +31670,6 @@ CREATE UNIQUE INDEX index_ops_feature_flags_issues_on_feature_flag_id_and_issue_ CREATE UNIQUE INDEX index_ops_strategies_user_lists_on_strategy_id_and_user_list_id ON operations_strategies_user_lists USING btree (strategy_id, user_list_id); -CREATE UNIQUE INDEX index_organizations_name_unique ON organizations USING btree (lower(name)); - CREATE UNIQUE INDEX index_organizations_on_unique_name_per_group ON customer_relations_organizations USING btree (group_id, lower(name), id); CREATE INDEX index_p_ci_runner_machine_builds_on_runner_machine_id ON ONLY p_ci_runner_machine_builds USING btree (runner_machine_id); @@ -33174,6 +33172,8 @@ CREATE UNIQUE INDEX unique_index_on_system_note_metadata_id ON resource_link_eve CREATE UNIQUE INDEX unique_merge_request_metrics_by_merge_request_id ON merge_request_metrics USING btree (merge_request_id); +CREATE UNIQUE INDEX unique_organizations_on_name_lower ON organizations USING btree (lower(name)); + CREATE UNIQUE INDEX unique_packages_project_id_and_name_and_version_when_debian ON packages_packages USING btree (project_id, name, version) WHERE ((package_type = 9) AND (status <> 4)); CREATE UNIQUE INDEX unique_postgres_async_fk_validations_name_and_table_name ON postgres_async_foreign_key_validations USING btree (name, table_name); -- GitLab From f4e1cac46930e20cd462da774426fe1abb84ab88 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 14:11:57 +0200 Subject: [PATCH 07/13] Ensure column exists before removing --- db/migrate/20230509115525_add_name_to_organization.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb index 1d54b464a18d6a..1db194fc674035 100644 --- a/db/migrate/20230509115525_add_name_to_organization.rb +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -16,7 +16,7 @@ def up def down remove_concurrent_index_by_name :organizations, name: INDEX_NAME - remove_column :organizations, :name + remove_column :organizations, :name, if_exists: true end end # rubocop:enable Migration/AddLimitToTextColumns -- GitLab From 0160e568c03b37047f164ed09853a4aebd1cbb13 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 14:15:25 +0200 Subject: [PATCH 08/13] Start numbering from 1000 and not 2 --- db/migrate/20230509085428_change_organizations_sequence.rb | 2 +- db/structure.sql | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/migrate/20230509085428_change_organizations_sequence.rb b/db/migrate/20230509085428_change_organizations_sequence.rb index 8010c9c290733d..59ec8c6e1ea78d 100644 --- a/db/migrate/20230509085428_change_organizations_sequence.rb +++ b/db/migrate/20230509085428_change_organizations_sequence.rb @@ -3,7 +3,7 @@ class ChangeOrganizationsSequence < Gitlab::Database::Migration[2.1] def up # Modify sequence for organizations.id so id '1' is never automatically taken - execute "ALTER SEQUENCE organizations_id_seq START WITH 2 MINVALUE 2 RESTART" + execute "ALTER SEQUENCE organizations_id_seq START WITH 1000 MINVALUE 1000 RESTART" end def down diff --git a/db/structure.sql b/db/structure.sql index 19c19248b3dbad..8ba5cedf74047d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19185,9 +19185,9 @@ CREATE TABLE organizations ( ); CREATE SEQUENCE organizations_id_seq - START WITH 2 + START WITH 1000 INCREMENT BY 1 - MINVALUE 2 + MINVALUE 1000 NO MAXVALUE CACHE 1; -- GitLab From eb472466f2281defad9c90a9f5c63a32bc693841 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 14:16:07 +0200 Subject: [PATCH 09/13] Do not attempt to instantiate organization while running down migration --- db/migrate/20230509131736_add_default_organization.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20230509131736_add_default_organization.rb b/db/migrate/20230509131736_add_default_organization.rb index 555b5ca76f8683..a63e7171f53988 100644 --- a/db/migrate/20230509131736_add_default_organization.rb +++ b/db/migrate/20230509131736_add_default_organization.rb @@ -11,6 +11,6 @@ def up end def down - Organization.find_by(id: 1)&.delete + Organization.where(id: 1).delete_all end end -- GitLab From f9607e12fc20fcbe735a791f76a637d8503c087b Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 15:14:39 +0200 Subject: [PATCH 10/13] Translate error message --- app/models/organization.rb | 2 +- locale/gitlab.pot | 3 +++ spec/models/organization_spec.rb | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/organization.rb b/app/models/organization.rb index 826378aaa6d752..5aebe3579c35ce 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -22,7 +22,7 @@ def default? def check_if_default_organization return unless default? - raise ActiveRecord::RecordNotDestroyed, 'Cannot delete the default organization' + raise ActiveRecord::RecordNotDestroyed, _('Cannot delete the default organization') end end # rubocop: enable Gitlab/NamespacedClass diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 99ac0af463ec9e..01e661c41e6f2a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8745,6 +8745,9 @@ msgstr "" msgid "Cannot delete the default framework" msgstr "" +msgid "Cannot delete the default organization" +msgstr "" + msgid "Cannot have multiple Jira imports running at the same time" msgstr "" diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 832ed5fd50a65f..e1aac88e6408ca 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -45,7 +45,7 @@ it 'raises an error' do expect do default_organization.destroy! - end.to raise_error(ActiveRecord::RecordNotDestroyed, 'Cannot delete the default organization') + end.to raise_error(ActiveRecord::RecordNotDestroyed, _('Cannot delete the default organization')) end end -- GitLab From 96a70921611e4379af081833b5802d5206a1f969 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 15:17:18 +0200 Subject: [PATCH 11/13] Move rubocop disable to Exclude section --- .rubocop_todo/gitlab/namespaced_class.yml | 1 + app/models/organization.rb | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index 925b5188c8b98a..3a81130a47c6a3 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -244,6 +244,7 @@ Gitlab/NamespacedClass: - 'app/models/notification_setting.rb' - 'app/models/oauth_access_grant.rb' - 'app/models/oauth_access_token.rb' + - 'app/models/organization.rb' - 'app/models/out_of_context_discussion.rb' - 'app/models/pages_deployment.rb' - 'app/models/pages_domain.rb' diff --git a/app/models/organization.rb b/app/models/organization.rb index 5aebe3579c35ce..cfbbbf1183ef57 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop: disable Gitlab/NamespacedClass class Organization < ApplicationRecord DEFAULT_ORGANIZATION_ID = 1 @@ -25,4 +24,3 @@ def check_if_default_organization raise ActiveRecord::RecordNotDestroyed, _('Cannot delete the default organization') end end -# rubocop: enable Gitlab/NamespacedClass -- GitLab From e90ae30b4c33e276cc5620c7215115e0da73d9eb Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 16:33:28 +0200 Subject: [PATCH 12/13] Remove remove_index: dropping column will drop index --- db/migrate/20230509115525_add_name_to_organization.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb index 1db194fc674035..5eb469d7368ce7 100644 --- a/db/migrate/20230509115525_add_name_to_organization.rb +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -14,8 +14,6 @@ def up end def down - remove_concurrent_index_by_name :organizations, name: INDEX_NAME - remove_column :organizations, :name, if_exists: true end end -- GitLab From 54730e03f2bb075bb0b89896503ce12b9cdfdf7a Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Mon, 15 May 2023 16:36:36 +0200 Subject: [PATCH 13/13] No need for concurrent index --- db/migrate/20230509115525_add_name_to_organization.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/migrate/20230509115525_add_name_to_organization.rb b/db/migrate/20230509115525_add_name_to_organization.rb index 5eb469d7368ce7..d77fa84a70c9f0 100644 --- a/db/migrate/20230509115525_add_name_to_organization.rb +++ b/db/migrate/20230509115525_add_name_to_organization.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# rubocop:disable Migration/AddLimitToTextColumns +# rubocop:disable Migration/AddLimitToTextColumns, Migration/AddIndex # limit is added in 20230515111314_add_text_limit_on_organization_name.rb class AddNameToOrganization < Gitlab::Database::Migration[2.1] disable_ddl_transaction! @@ -10,11 +10,11 @@ class AddNameToOrganization < Gitlab::Database::Migration[2.1] def up add_column :organizations, :name, :text, null: false, default: '' - add_concurrent_index :organizations, 'lower(name)', name: INDEX_NAME, unique: true + add_index :organizations, 'lower(name)', name: INDEX_NAME, unique: true end def down remove_column :organizations, :name, if_exists: true end end -# rubocop:enable Migration/AddLimitToTextColumns +# rubocop:enable Migration/AddLimitToTextColumns, Migration/AddIndex -- GitLab