From e979d395ff90e1a2c1b3b6887d499de36a7ad924 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Thu, 13 Jun 2024 15:38:44 +1200 Subject: [PATCH 1/3] Add table to record placeholder contributions During an import that uses the improved user mapping from https://gitlab.com/groups/gitlab-org/-/epics/12378, initially, the users associated with the imported data are "placeholder" users (a kind of internal user) rather than the real users. The placeholder user is recorded in `Import::SourceUser#placeholder_user` Later the `Import::SourceUser` is assigned a real user, the `Import::SourceUser#assigned_to_user`, after the real user has authorized this. At this point, we need to change all imported data from being associated with the `placeholder_user` to the (real) `assigned_to_user`. The table in this commit records what imported data should be updated in this process. https://gitlab.com/gitlab-org/gitlab/-/issues/443554 Changelog: added --- .../source_user_placeholder_reference.rb | 27 +++++++++++ ...ce_user_placeholder_reference_find_by.json | 19 ++++++++ ...ort_source_user_placeholder_references.yml | 14 ++++++ ...mport_source_user_placeholder_reference.rb | 24 ++++++++++ db/schema_migrations/20240612034702 | 1 + db/structure.sql | 38 ++++++++++++++++ spec/db/schema_spec.rb | 1 + ...port_source_user_placeholder_references.rb | 11 +++++ .../source_user_placeholder_reference_spec.rb | 45 +++++++++++++++++++ 9 files changed, 180 insertions(+) create mode 100644 app/models/import/source_user_placeholder_reference.rb create mode 100644 app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json create mode 100644 db/docs/import_source_user_placeholder_references.yml create mode 100644 db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb create mode 100644 db/schema_migrations/20240612034702 create mode 100644 spec/factories/import_source_user_placeholder_references.rb create mode 100644 spec/models/import/source_user_placeholder_reference_spec.rb diff --git a/app/models/import/source_user_placeholder_reference.rb b/app/models/import/source_user_placeholder_reference.rb new file mode 100644 index 00000000000000..6977e6c63389a2 --- /dev/null +++ b/app/models/import/source_user_placeholder_reference.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Import + class SourceUserPlaceholderReference < ApplicationRecord + self.table_name = 'import_source_user_placeholder_references' + + belongs_to :source_user, class_name: 'Import::SourceUser' + belongs_to :namespace + + validates :column, :model, :namespace_id, :source_user_id, presence: true + validates :find_by_id, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true + validate :validate_find_by_present + validates :find_by_composite, + json_schema: { filename: 'import_source_user_placeholder_reference_find_by' }, + allow_nil: true + + attribute :find_by_composite, :ind_jsonb + + private + + def validate_find_by_present + return if find_by_id.present? ^ find_by_composite.present? + + errors.add(:base, :blank, message: 'find_by_id or find_by_composite must be present') + end + end +end diff --git a/app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json b/app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json new file mode 100644 index 00000000000000..6ae47a3b63ad93 --- /dev/null +++ b/app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json @@ -0,0 +1,19 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Stores composite find_by data for imported records that are mapped to placeholder users", + "type": "object", + "minProperties": 1, + "patternProperties": { + ".*": { + "oneOf": [ + { + "type": "string", + "pattern": "^[0-9]+$" + }, + { + "type": "integer" + } + ] + } + } +} diff --git a/db/docs/import_source_user_placeholder_references.yml b/db/docs/import_source_user_placeholder_references.yml new file mode 100644 index 00000000000000..87e1f72fc44ec8 --- /dev/null +++ b/db/docs/import_source_user_placeholder_references.yml @@ -0,0 +1,14 @@ +--- +table_name: import_source_user_placeholder_references +classes: +- Import::SourceUserPlaceholderReference +feature_categories: +- importers +description: Used to map placeholder user references from imported data to real users +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156241 +milestone: '17.2' +gitlab_schema: gitlab_main_cell +allow_cross_foreign_keys: +- gitlab_main_clusterwide +sharding_key: + namespace_id: namespaces diff --git a/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb b/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb new file mode 100644 index 00000000000000..38c04b2a25a68f --- /dev/null +++ b/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class CreateImportSourceUserPlaceholderReference < Gitlab::Database::Migration[2.2] + milestone '17.2' + + enable_lock_retries! + + INDEX_NAME = 'index_import_source_user_placeholder_references_on_source_user_' + + def change + create_table :import_source_user_placeholder_references do |t| + t.references :source_user, + index: { name: INDEX_NAME }, + null: false, + foreign_key: { to_table: :import_source_users, on_delete: :cascade } + t.references :namespace, null: false, index: true, foreign_key: { on_delete: :cascade } + t.bigint :find_by_id, null: true + t.timestamps_with_timezone null: false + t.text :model, limit: 150, null: false + t.text :column, limit: 50, null: false + t.jsonb :find_by_composite, null: true + end + end +end diff --git a/db/schema_migrations/20240612034702 b/db/schema_migrations/20240612034702 new file mode 100644 index 00000000000000..4c11c17c15d8fe --- /dev/null +++ b/db/schema_migrations/20240612034702 @@ -0,0 +1 @@ +91e467973c28e98ed562c70ae108f7b5cb1ed0353e3ce0c8b13f052077c75d5a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 7338fdc4a9b2d2..a353a657019d21 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10579,6 +10579,29 @@ CREATE SEQUENCE import_failures_id_seq ALTER SEQUENCE import_failures_id_seq OWNED BY import_failures.id; +CREATE TABLE import_source_user_placeholder_references ( + id bigint NOT NULL, + source_user_id bigint NOT NULL, + namespace_id bigint NOT NULL, + find_by_id bigint, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + model text NOT NULL, + "column" text NOT NULL, + find_by_composite jsonb, + CONSTRAINT check_11803c4c0f CHECK ((char_length("column") <= 50)), + CONSTRAINT check_d17bd9dd4d CHECK ((char_length(model) <= 150)) +); + +CREATE SEQUENCE import_source_user_placeholder_references_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE import_source_user_placeholder_references_id_seq OWNED BY import_source_user_placeholder_references.id; + CREATE TABLE import_source_users ( id bigint NOT NULL, placeholder_user_id bigint, @@ -20142,6 +20165,8 @@ ALTER TABLE ONLY import_export_uploads ALTER COLUMN id SET DEFAULT nextval('impo ALTER TABLE ONLY import_failures ALTER COLUMN id SET DEFAULT nextval('import_failures_id_seq'::regclass); +ALTER TABLE ONLY import_source_user_placeholder_references ALTER COLUMN id SET DEFAULT nextval('import_source_user_placeholder_references_id_seq'::regclass); + ALTER TABLE ONLY import_source_users ALTER COLUMN id SET DEFAULT nextval('import_source_users_id_seq'::regclass); ALTER TABLE ONLY incident_management_escalation_policies ALTER COLUMN id SET DEFAULT nextval('incident_management_escalation_policies_id_seq'::regclass); @@ -22315,6 +22340,9 @@ ALTER TABLE ONLY import_export_uploads ALTER TABLE ONLY import_failures ADD CONSTRAINT import_failures_pkey PRIMARY KEY (id); +ALTER TABLE ONLY import_source_user_placeholder_references + ADD CONSTRAINT import_source_user_placeholder_references_pkey PRIMARY KEY (id); + ALTER TABLE ONLY import_source_users ADD CONSTRAINT import_source_users_pkey PRIMARY KEY (id); @@ -26628,6 +26656,10 @@ CREATE INDEX index_import_failures_on_project_id_not_null ON import_failures USI CREATE INDEX index_import_failures_on_user_id_not_null ON import_failures USING btree (user_id) WHERE (user_id IS NOT NULL); +CREATE INDEX index_import_source_user_placeholder_references_on_namespace_id ON import_source_user_placeholder_references USING btree (namespace_id); + +CREATE INDEX index_import_source_user_placeholder_references_on_source_user_ ON import_source_user_placeholder_references USING btree (source_user_id); + CREATE INDEX index_import_source_users_on_namespace_id ON import_source_users USING btree (namespace_id); CREATE INDEX index_import_source_users_on_placeholder_user_id ON import_source_users USING btree (placeholder_user_id); @@ -32349,6 +32381,9 @@ ALTER TABLE ONLY namespaces_storage_limit_exclusions ALTER TABLE ONLY users_security_dashboard_projects ADD CONSTRAINT fk_rails_150cd5682c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY import_source_user_placeholder_references + ADD CONSTRAINT fk_rails_158995b934 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY import_source_users ADD CONSTRAINT fk_rails_167f82fd95 FOREIGN KEY (reassign_to_user_id) REFERENCES users(id) ON DELETE SET NULL; @@ -33711,6 +33746,9 @@ ALTER TABLE ONLY upload_states ALTER TABLE ONLY epic_metrics ADD CONSTRAINT fk_rails_d071904753 FOREIGN KEY (epic_id) REFERENCES epics(id) ON DELETE CASCADE; +ALTER TABLE ONLY import_source_user_placeholder_references + ADD CONSTRAINT fk_rails_d0b75c434e FOREIGN KEY (source_user_id) REFERENCES import_source_users(id) ON DELETE CASCADE; + ALTER TABLE ONLY subscriptions ADD CONSTRAINT fk_rails_d0c8bda804 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 4d23e268c8d01a..1b134419d3ec23 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -87,6 +87,7 @@ gitlab_subscription_histories: %w[gitlab_subscription_id hosted_plan_id namespace_id], identities: %w[user_id], import_failures: %w[project_id], + import_source_user_placeholder_references: %w[find_by_id], issues: %w[last_edited_by_id state_id], issue_emails: %w[email_message_id], jira_tracker_data: %w[jira_issue_transition_id], diff --git a/spec/factories/import_source_user_placeholder_references.rb b/spec/factories/import_source_user_placeholder_references.rb new file mode 100644 index 00000000000000..ace4149ab60ab0 --- /dev/null +++ b/spec/factories/import_source_user_placeholder_references.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :import_source_user_placeholder_reference, class: 'Import::SourceUserPlaceholderReference' do + source_user factory: :import_source_user + namespace + model { 'Note' } + column { 'author_id' } + find_by_id { 1 } + end +end diff --git a/spec/models/import/source_user_placeholder_reference_spec.rb b/spec/models/import/source_user_placeholder_reference_spec.rb new file mode 100644 index 00000000000000..aec395cfff7813 --- /dev/null +++ b/spec/models/import/source_user_placeholder_reference_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::SourceUserPlaceholderReference, feature_category: :importers do + describe 'associations' do + it { is_expected.to belong_to(:source_user).class_name('Import::SourceUser') } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:column) } + it { is_expected.to validate_presence_of(:model) } + it { is_expected.to validate_presence_of(:namespace_id) } + it { is_expected.to validate_presence_of(:source_user_id) } + it { is_expected.to validate_numericality_of(:find_by_id).only_integer.is_greater_than(0) } + it { expect(described_class).to validate_jsonb_schema(['find_by_composite']) } + it { is_expected.to allow_value({ id: 1 }).for(:find_by_composite) } + it { is_expected.to allow_value({ id: '1' }).for(:find_by_composite) } + it { is_expected.to allow_value({ foo: '1', bar: 2 }).for(:find_by_composite) } + it { is_expected.not_to allow_value({}).for(:find_by_composite) } + it { is_expected.not_to allow_value({ id: 'foo' }).for(:find_by_composite) } + it { is_expected.not_to allow_value(1).for(:find_by_composite) } + + describe '#validate_find_by_present' do + def validation_errors(...) + described_class.new(...).tap(&:validate) + .errors + .where(:base) + end + + it 'must have find_by_id or find_by_composite present' do + expect(validation_errors).to be_present + expect(validation_errors(find_by_id: 1)).to be_blank + expect(validation_errors(find_by_composite: { id: 1 })).to be_blank + expect(validation_errors(find_by_id: 1, find_by_composite: { id: 1 })).to be_present + end + end + end + + it 'is destroyed when source user is destroyed' do + reference = create(:import_source_user_placeholder_reference) + + expect { reference.source_user.destroy! }.to change { described_class.count }.by(-1) + end +end -- GitLab From ad33377401a058cfb6eb368c509175242e1e1564 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Tue, 18 Jun 2024 11:56:26 +1200 Subject: [PATCH 2/3] Add reviewer feedback --- app/models/import/source_user_placeholder_reference.rb | 2 +- ...2034702_create_import_source_user_placeholder_reference.rb | 2 +- db/structure.sql | 4 ++-- spec/factories/import_source_user_placeholder_references.rb | 2 +- spec/models/import/source_user_placeholder_reference_spec.rb | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/import/source_user_placeholder_reference.rb b/app/models/import/source_user_placeholder_reference.rb index 6977e6c63389a2..921356dffe0f23 100644 --- a/app/models/import/source_user_placeholder_reference.rb +++ b/app/models/import/source_user_placeholder_reference.rb @@ -7,7 +7,7 @@ class SourceUserPlaceholderReference < ApplicationRecord belongs_to :source_user, class_name: 'Import::SourceUser' belongs_to :namespace - validates :column, :model, :namespace_id, :source_user_id, presence: true + validates :model, :namespace_id, :source_user_id, :user_reference_column, presence: true validates :find_by_id, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true validate :validate_find_by_present validates :find_by_composite, diff --git a/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb b/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb index 38c04b2a25a68f..5cd3d2760b4e50 100644 --- a/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb +++ b/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb @@ -17,7 +17,7 @@ def change t.bigint :find_by_id, null: true t.timestamps_with_timezone null: false t.text :model, limit: 150, null: false - t.text :column, limit: 50, null: false + t.text :user_reference_column, limit: 50, null: false t.jsonb :find_by_composite, null: true end end diff --git a/db/structure.sql b/db/structure.sql index a353a657019d21..71a565b1893a00 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10587,9 +10587,9 @@ CREATE TABLE import_source_user_placeholder_references ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, model text NOT NULL, - "column" text NOT NULL, + user_reference_column text NOT NULL, find_by_composite jsonb, - CONSTRAINT check_11803c4c0f CHECK ((char_length("column") <= 50)), + CONSTRAINT check_782140eb9d CHECK ((char_length(user_reference_column) <= 50)), CONSTRAINT check_d17bd9dd4d CHECK ((char_length(model) <= 150)) ); diff --git a/spec/factories/import_source_user_placeholder_references.rb b/spec/factories/import_source_user_placeholder_references.rb index ace4149ab60ab0..affcd7d1dab53a 100644 --- a/spec/factories/import_source_user_placeholder_references.rb +++ b/spec/factories/import_source_user_placeholder_references.rb @@ -5,7 +5,7 @@ source_user factory: :import_source_user namespace model { 'Note' } - column { 'author_id' } + user_reference_column { 'author_id' } find_by_id { 1 } end end diff --git a/spec/models/import/source_user_placeholder_reference_spec.rb b/spec/models/import/source_user_placeholder_reference_spec.rb index aec395cfff7813..67cd33b83ef6fa 100644 --- a/spec/models/import/source_user_placeholder_reference_spec.rb +++ b/spec/models/import/source_user_placeholder_reference_spec.rb @@ -8,7 +8,7 @@ end describe 'validations' do - it { is_expected.to validate_presence_of(:column) } + it { is_expected.to validate_presence_of(:user_reference_column) } it { is_expected.to validate_presence_of(:model) } it { is_expected.to validate_presence_of(:namespace_id) } it { is_expected.to validate_presence_of(:source_user_id) } @@ -28,7 +28,7 @@ def validation_errors(...) .where(:base) end - it 'must have find_by_id or find_by_composite present' do + it 'must have find_by_id or find_by_composite present', :aggregate_failures do expect(validation_errors).to be_present expect(validation_errors(find_by_id: 1)).to be_blank expect(validation_errors(find_by_composite: { id: 1 })).to be_blank -- GitLab From 8650678de1b877a09e2ba4ca2ffb8e5777e23d24 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Wed, 19 Jun 2024 13:48:18 +1200 Subject: [PATCH 3/3] Add reviewer suggestions --- .../source_user_placeholder_reference.rb | 16 ++++++------ ..._placeholder_reference_composite_key.json} | 2 +- ...mport_source_user_placeholder_reference.rb | 6 ++--- db/structure.sql | 5 ++-- spec/db/schema_spec.rb | 1 - ...port_source_user_placeholder_references.rb | 2 +- .../source_user_placeholder_reference_spec.rb | 26 +++++++++---------- 7 files changed, 28 insertions(+), 30 deletions(-) rename app/validators/json_schemas/{import_source_user_placeholder_reference_find_by.json => import_source_user_placeholder_reference_composite_key.json} (73%) diff --git a/app/models/import/source_user_placeholder_reference.rb b/app/models/import/source_user_placeholder_reference.rb index 921356dffe0f23..66c6647e3d1386 100644 --- a/app/models/import/source_user_placeholder_reference.rb +++ b/app/models/import/source_user_placeholder_reference.rb @@ -8,20 +8,20 @@ class SourceUserPlaceholderReference < ApplicationRecord belongs_to :namespace validates :model, :namespace_id, :source_user_id, :user_reference_column, presence: true - validates :find_by_id, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true - validate :validate_find_by_present - validates :find_by_composite, - json_schema: { filename: 'import_source_user_placeholder_reference_find_by' }, + validates :numeric_key, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true + validates :composite_key, + json_schema: { filename: 'import_source_user_placeholder_reference_composite_key' }, allow_nil: true + validate :validate_numeric_or_composite_key_present - attribute :find_by_composite, :ind_jsonb + attribute :composite_key, :ind_jsonb private - def validate_find_by_present - return if find_by_id.present? ^ find_by_composite.present? + def validate_numeric_or_composite_key_present + return if numeric_key.present? ^ composite_key.present? - errors.add(:base, :blank, message: 'find_by_id or find_by_composite must be present') + errors.add(:base, :blank, message: 'numeric_key or composite_key must be present') end end end diff --git a/app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json b/app/validators/json_schemas/import_source_user_placeholder_reference_composite_key.json similarity index 73% rename from app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json rename to app/validators/json_schemas/import_source_user_placeholder_reference_composite_key.json index 6ae47a3b63ad93..ccfbe0ec527bc7 100644 --- a/app/validators/json_schemas/import_source_user_placeholder_reference_find_by.json +++ b/app/validators/json_schemas/import_source_user_placeholder_reference_composite_key.json @@ -1,6 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", - "description": "Stores composite find_by data for imported records that are mapped to placeholder users", + "description": "Stores composite_key data for imported records that are mapped to placeholder users", "type": "object", "minProperties": 1, "patternProperties": { diff --git a/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb b/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb index 5cd3d2760b4e50..0364d6322006ff 100644 --- a/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb +++ b/db/migrate/20240612034702_create_import_source_user_placeholder_reference.rb @@ -14,11 +14,11 @@ def change null: false, foreign_key: { to_table: :import_source_users, on_delete: :cascade } t.references :namespace, null: false, index: true, foreign_key: { on_delete: :cascade } - t.bigint :find_by_id, null: true - t.timestamps_with_timezone null: false + t.bigint :numeric_key, null: true + t.datetime_with_timezone :created_at, null: false t.text :model, limit: 150, null: false t.text :user_reference_column, limit: 50, null: false - t.jsonb :find_by_composite, null: true + t.jsonb :composite_key, null: true end end end diff --git a/db/structure.sql b/db/structure.sql index 71a565b1893a00..dcac771db947b9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10583,12 +10583,11 @@ CREATE TABLE import_source_user_placeholder_references ( id bigint NOT NULL, source_user_id bigint NOT NULL, namespace_id bigint NOT NULL, - find_by_id bigint, + numeric_key bigint, created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, model text NOT NULL, user_reference_column text NOT NULL, - find_by_composite jsonb, + composite_key jsonb, CONSTRAINT check_782140eb9d CHECK ((char_length(user_reference_column) <= 50)), CONSTRAINT check_d17bd9dd4d CHECK ((char_length(model) <= 150)) ); diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 1b134419d3ec23..4d23e268c8d01a 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -87,7 +87,6 @@ gitlab_subscription_histories: %w[gitlab_subscription_id hosted_plan_id namespace_id], identities: %w[user_id], import_failures: %w[project_id], - import_source_user_placeholder_references: %w[find_by_id], issues: %w[last_edited_by_id state_id], issue_emails: %w[email_message_id], jira_tracker_data: %w[jira_issue_transition_id], diff --git a/spec/factories/import_source_user_placeholder_references.rb b/spec/factories/import_source_user_placeholder_references.rb index affcd7d1dab53a..7aa19ad0a5a69e 100644 --- a/spec/factories/import_source_user_placeholder_references.rb +++ b/spec/factories/import_source_user_placeholder_references.rb @@ -6,6 +6,6 @@ namespace model { 'Note' } user_reference_column { 'author_id' } - find_by_id { 1 } + numeric_key { 1 } end end diff --git a/spec/models/import/source_user_placeholder_reference_spec.rb b/spec/models/import/source_user_placeholder_reference_spec.rb index 67cd33b83ef6fa..e441e065f59d4f 100644 --- a/spec/models/import/source_user_placeholder_reference_spec.rb +++ b/spec/models/import/source_user_placeholder_reference_spec.rb @@ -12,27 +12,27 @@ it { is_expected.to validate_presence_of(:model) } it { is_expected.to validate_presence_of(:namespace_id) } it { is_expected.to validate_presence_of(:source_user_id) } - it { is_expected.to validate_numericality_of(:find_by_id).only_integer.is_greater_than(0) } - it { expect(described_class).to validate_jsonb_schema(['find_by_composite']) } - it { is_expected.to allow_value({ id: 1 }).for(:find_by_composite) } - it { is_expected.to allow_value({ id: '1' }).for(:find_by_composite) } - it { is_expected.to allow_value({ foo: '1', bar: 2 }).for(:find_by_composite) } - it { is_expected.not_to allow_value({}).for(:find_by_composite) } - it { is_expected.not_to allow_value({ id: 'foo' }).for(:find_by_composite) } - it { is_expected.not_to allow_value(1).for(:find_by_composite) } + it { is_expected.to validate_numericality_of(:numeric_key).only_integer.is_greater_than(0) } + it { expect(described_class).to validate_jsonb_schema(['composite_key']) } + it { is_expected.to allow_value({ id: 1 }).for(:composite_key) } + it { is_expected.to allow_value({ id: '1' }).for(:composite_key) } + it { is_expected.to allow_value({ foo: '1', bar: 2 }).for(:composite_key) } + it { is_expected.not_to allow_value({}).for(:composite_key) } + it { is_expected.not_to allow_value({ id: 'foo' }).for(:composite_key) } + it { is_expected.not_to allow_value(1).for(:composite_key) } - describe '#validate_find_by_present' do + describe '#validate_numeric_or_composite_key_present' do def validation_errors(...) described_class.new(...).tap(&:validate) .errors .where(:base) end - it 'must have find_by_id or find_by_composite present', :aggregate_failures do + it 'must have numeric_key or composite_key present', :aggregate_failures do expect(validation_errors).to be_present - expect(validation_errors(find_by_id: 1)).to be_blank - expect(validation_errors(find_by_composite: { id: 1 })).to be_blank - expect(validation_errors(find_by_id: 1, find_by_composite: { id: 1 })).to be_present + expect(validation_errors(numeric_key: 1)).to be_blank + expect(validation_errors(composite_key: { id: 1 })).to be_blank + expect(validation_errors(numeric_key: 1, composite_key: { id: 1 })).to be_present end end end -- GitLab