Merge branch...

Merge branch '250800-create-new-experimentsubject-model-with-user_id-namespace_id-or-project_id' into 'master'

Create new ExperimentSubject model

See merge request gitlab-org/gitlab!47042

app/models/experiment.rb

@@ -2,6 +2,7 @@
 
 class Experiment < ApplicationRecord
   has_many :experiment_users
+  has_many :experiment_subjects, inverse_of: :experiment
 
   validates :name, presence: true, uniqueness: true, length: { maximum: 255 }
 

app/models/experiment_subject.rb

+# frozen_string_literal: true
+
+class ExperimentSubject < ApplicationRecord
+  include ::Gitlab::Experimentation::GroupTypes
+
+  belongs_to :experiment, inverse_of: :experiment_subjects
+  belongs_to :user
+  belongs_to :group
+  belongs_to :project
+
+  validates :experiment, presence: true
+  validates :variant, presence: true
+  validate :must_have_one_subject_present
+
+  enum variant: { GROUP_CONTROL => 0, GROUP_EXPERIMENTAL => 1 }
+
+  private
+
+  def must_have_one_subject_present
+    if non_nil_subjects.length != 1
+      errors.add(:base, s_("ExperimentSubject|Must have exactly one of User, Group, or Project."))
+    end
+  end
+
+  def non_nil_subjects
+    @non_nil_subjects ||= [user, group, project].reject(&:blank?)
+  end
+end

changelogs/unreleased/250800-create-new-experimentsubject-model-with-user_id-namespace_id-or-pr.yml

+---
+title: Create a new `ExperimentSubject` model, associated to the `Experiment` model, and related database migrations.
+merge_request: 47042
+author:
+type: added

db/migrate/20201110221400_create_experiment_subjects.rb

+# frozen_string_literal: true
+
+class CreateExperimentSubjects < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    create_table :experiment_subjects do |t|
+      t.references :experiment, index: true, foreign_key: { on_delete: :cascade }, null: false
+      t.bigint :user_id, index: true
+      t.bigint :group_id, index: true
+      t.bigint :project_id, index: true
+      t.integer :variant, limit: 2, null: false, default: 0
+      t.timestamps_with_timezone null: false
+    end
+
+    # Require exactly one of user_id, group_id, or project_id to be NOT NULL
+    execute <<-SQL
+      ALTER TABLE experiment_subjects ADD CONSTRAINT chk_has_one_subject CHECK (num_nonnulls(user_id, group_id, project_id) = 1);
+    SQL
+  end
+
+  def down
+    drop_table :experiment_subjects
+  end
+end

db/migrate/20201111051655_add_foreign_key_to_experiment_subjects_on_user.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToExperimentSubjectsOnUser < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_concurrent_foreign_key :experiment_subjects, :users, column: :user_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :experiment_subjects, column: :user_id
+    end
+  end
+end

db/migrate/20201111051847_add_foreign_key_to_experiment_subjects_on_group.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToExperimentSubjectsOnGroup < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_concurrent_foreign_key :experiment_subjects, :namespaces, column: :group_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :experiment_subjects, column: :group_id
+    end
+  end
+end

db/migrate/20201111051904_add_foreign_key_to_experiment_subjects_on_project.rb

+# frozen_string_literal: true
+
+class AddForeignKeyToExperimentSubjectsOnProject < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_concurrent_foreign_key :experiment_subjects, :projects, column: :project_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :experiment_subjects, column: :project_id
+    end
+  end
+end

db/schema_migrations/20201110221400

+9fba60d8805915fcf6af7812e2c752007ac17bb92c8a02c942c0c790d2997441
\ No newline at end of file

db/schema_migrations/20201111051655

+4340d0f6d3b660b336fdc3166a4960865c79e90f505b1173bab4e0d11c1199b3
\ No newline at end of file

db/schema_migrations/20201111051847

+8180908c5e577757b3f518d312cbf0ba77c65b39fa55dde487036541f49114a1
\ No newline at end of file

db/schema_migrations/20201111051904

+c228aa5c16e63af7520dd1bd90cefb1f74ec2371af3b0e839938d8c628f70e8a
\ No newline at end of file

db/structure.sql

@@ -12138,6 +12138,27 @@ CREATE SEQUENCE evidences_id_seq
 
 ALTER SEQUENCE evidences_id_seq OWNED BY evidences.id;
 
+CREATE TABLE experiment_subjects (
+    id bigint NOT NULL,
+    experiment_id bigint NOT NULL,
+    user_id bigint,
+    group_id bigint,
+    project_id bigint,
+    variant smallint DEFAULT 0 NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    CONSTRAINT chk_has_one_subject CHECK ((num_nonnulls(user_id, group_id, project_id) = 1))
+);
+
+CREATE SEQUENCE experiment_subjects_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE experiment_subjects_id_seq OWNED BY experiment_subjects.id;
+
 CREATE TABLE experiment_users (
     id bigint NOT NULL,
     experiment_id bigint NOT NULL,
@@ -18187,6 +18208,8 @@ ALTER TABLE ONLY events ALTER COLUMN id SET DEFAULT nextval('events_id_seq'::reg
 
 ALTER TABLE ONLY evidences ALTER COLUMN id SET DEFAULT nextval('evidences_id_seq'::regclass);
 
+ALTER TABLE ONLY experiment_subjects ALTER COLUMN id SET DEFAULT nextval('experiment_subjects_id_seq'::regclass);
+
 ALTER TABLE ONLY experiment_users ALTER COLUMN id SET DEFAULT nextval('experiment_users_id_seq'::regclass);
 
 ALTER TABLE ONLY experiments ALTER COLUMN id SET DEFAULT nextval('experiments_id_seq'::regclass);
@@ -19353,6 +19376,9 @@ ALTER TABLE ONLY events
 ALTER TABLE ONLY evidences
     ADD CONSTRAINT evidences_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY experiment_subjects
+    ADD CONSTRAINT experiment_subjects_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY experiment_users
     ADD CONSTRAINT experiment_users_pkey PRIMARY KEY (id);
 
@@ -21233,6 +21259,14 @@ CREATE UNIQUE INDEX index_events_on_target_type_and_target_id_and_fingerprint ON
 
 CREATE INDEX index_evidences_on_release_id ON evidences USING btree (release_id);
 
+CREATE INDEX index_experiment_subjects_on_experiment_id ON experiment_subjects USING btree (experiment_id);
+
+CREATE INDEX index_experiment_subjects_on_group_id ON experiment_subjects USING btree (group_id);
+
+CREATE INDEX index_experiment_subjects_on_project_id ON experiment_subjects USING btree (project_id);
+
+CREATE INDEX index_experiment_subjects_on_user_id ON experiment_subjects USING btree (user_id);
+
 CREATE INDEX index_experiment_users_on_experiment_id ON experiment_users USING btree (experiment_id);
 
 CREATE INDEX index_experiment_users_on_user_id ON experiment_users USING btree (user_id);
@@ -23424,6 +23458,9 @@ ALTER TABLE ONLY packages_package_files
 ALTER TABLE ONLY ci_builds
     ADD CONSTRAINT fk_87f4cefcda FOREIGN KEY (upstream_pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY experiment_subjects
+    ADD CONSTRAINT fk_88489af1b1 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY vulnerabilities
     ADD CONSTRAINT fk_88b4d546ef FOREIGN KEY (start_date_sourcing_milestone_id) REFERENCES milestones(id) ON DELETE SET NULL;
 
@@ -23610,6 +23647,9 @@ ALTER TABLE ONLY issues
 ALTER TABLE ONLY issue_links
     ADD CONSTRAINT fk_c900194ff2 FOREIGN KEY (source_id) REFERENCES issues(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY experiment_subjects
+    ADD CONSTRAINT fk_ccc28f8ceb FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY todos
     ADD CONSTRAINT fk_ccf0373936 FOREIGN KEY (author_id) REFERENCES users(id) ON DELETE CASCADE;
 
@@ -23670,6 +23710,9 @@ ALTER TABLE ONLY analytics_devops_adoption_segment_selections
 ALTER TABLE ONLY issues
     ADD CONSTRAINT fk_df75a7c8b8 FOREIGN KEY (promoted_to_epic_id) REFERENCES epics(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY experiment_subjects
+    ADD CONSTRAINT fk_dfc3e211d4 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY ci_resources
     ADD CONSTRAINT fk_e169a8e3d5 FOREIGN KEY (build_id) REFERENCES ci_builds(id) ON DELETE SET NULL;
 
@@ -25032,6 +25075,9 @@ ALTER TABLE ONLY snippet_statistics
 ALTER TABLE ONLY project_security_settings
     ADD CONSTRAINT fk_rails_ed4abe1338 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY experiment_subjects
+    ADD CONSTRAINT fk_rails_ede5754774 FOREIGN KEY (experiment_id) REFERENCES experiments(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY ci_daily_build_group_report_results
     ADD CONSTRAINT fk_rails_ee072d13b3 FOREIGN KEY (last_pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
 

locale/gitlab.pot

@@ -11319,6 +11319,9 @@ msgstr ""
 msgid "Experienced"
 msgstr ""
 
+msgid "ExperimentSubject|Must have exactly one of User, Group, or Project."
+msgstr ""
+
 msgid "Expiration"
 msgstr ""
 

spec/factories/experiment_subjects.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :experiment_subject do
+    experiment
+    user
+    variant { :control }
+  end
+end

spec/models/experiment_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe Experiment do
 
   describe 'associations' do
     it { is_expected.to have_many(:experiment_users) }
+    it { is_expected.to have_many(:experiment_subjects) }
   end
 
   describe 'validations' do

spec/models/experiment_subject_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ExperimentSubject, type: :model do
+  describe 'associations' do
+    it { is_expected.to belong_to(:experiment) }
+    it { is_expected.to belong_to(:user) }
+    it { is_expected.to belong_to(:group) }
+    it { is_expected.to belong_to(:project) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:experiment) }
+
+    describe 'must_have_one_subject_present' do
+      let(:experiment_subject) { build(:experiment_subject, user: nil, group: nil, project: nil) }
+      let(:error_message) { 'Must have exactly one of User, Group, or Project.' }
+
+      it 'fails when no subject is present' do
+        expect(experiment_subject).not_to be_valid
+        expect(experiment_subject.errors[:base]).to include(error_message)
+      end
+
+      it 'passes when user subject is present' do
+        experiment_subject.user = build(:user)
+        expect(experiment_subject).to be_valid
+      end
+
+      it 'passes when group subject is present' do
+        experiment_subject.group = build(:group)
+        expect(experiment_subject).to be_valid
+      end
+
+      it 'passes when project subject is present' do
+        experiment_subject.project = build(:project)
+        expect(experiment_subject).to be_valid
+      end
+
+      it 'fails when more than one subject is present', :aggregate_failures do
+        # two subjects
+        experiment_subject.user = build(:user)
+        experiment_subject.group = build(:group)
+        expect(experiment_subject).not_to be_valid
+        expect(experiment_subject.errors[:base]).to include(error_message)
+
+        # three subjects
+        experiment_subject.project = build(:project)
+        expect(experiment_subject).not_to be_valid
+        expect(experiment_subject.errors[:base]).to include(error_message)
+      end
+    end
+  end
+end