Merge branch 'add-reviewers-to-merge-requests' into 'master'

Add MergeRequestReviewer for dedicated Reviewers section

See merge request gitlab-org/gitlab!40358

app/models/merge_request.rb

@@ -80,6 +80,8 @@ class MergeRequest < ApplicationRecord
 
   has_many :merge_request_assignees
   has_many :assignees, class_name: "User", through: :merge_request_assignees
+  has_many :merge_request_reviewers
+  has_many :reviewers, class_name: "User", through: :merge_request_reviewers
   has_many :user_mentions, class_name: "MergeRequestUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
   has_many :deployment_merge_requests

app/models/merge_request_reviewer.rb

+# frozen_string_literal: true
+
+class MergeRequestReviewer < ApplicationRecord
+  belongs_to :merge_request
+  belongs_to :reviewer, class_name: "User", foreign_key: :user_id, inverse_of: :merge_request_assignees
+end

changelogs/unreleased/add-reviewers-to-merge-requests.yml

+---
+title: Add merge_request_reviewers table
+merge_request: 40358
+author:
+type: added

db/migrate/20200825071735_create_merge_request_reviewers.rb

+# frozen_string_literal: true
+
+class CreateMergeRequestReviewers < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    create_table :merge_request_reviewers do |t|
+      t.bigint :user_id, null: false
+      t.bigint :merge_request_id, null: false
+      t.datetime_with_timezone :created_at, null: false
+    end
+
+    add_index :merge_request_reviewers, [:merge_request_id, :user_id], unique: true
+    add_index :merge_request_reviewers, :user_id
+  end
+
+  def down
+    drop_table :merge_request_reviewers
+  end
+end

db/migrate/20200827060911_add_merge_request_foreign_key_to_merge_request_reviewers.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddMergeRequestForeignKeyToMergeRequestReviewers < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :merge_request_reviewers, :merge_requests, column: :merge_request_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :merge_request_reviewers, column: :merge_request_id
+    end
+  end
+end

db/migrate/20200827060932_add_user_foreign_key_to_merge_request_reviewers.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddUserForeignKeyToMergeRequestReviewers < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :merge_request_reviewers, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :merge_request_reviewers, column: :user_id
+    end
+  end
+end

db/schema_migrations/20200825071735

+cd8574318fae1f2bb021b53d4e453c6b64c763f0e7cc8836cdb8b12963ff0e18
\ No newline at end of file

db/schema_migrations/20200827060911

+124f6ba79f71e2de510741b22d3dd5cf15378b5476c759484bd814377a644256
\ No newline at end of file

db/schema_migrations/20200827060932

+ba2e32b4836062631308937023470d31b3f808b468999ba15374c3b953377402
\ No newline at end of file

db/structure.sql

@@ -13161,6 +13161,22 @@ CREATE SEQUENCE public.merge_request_metrics_id_seq
 
 ALTER SEQUENCE public.merge_request_metrics_id_seq OWNED BY public.merge_request_metrics.id;
 
+CREATE TABLE public.merge_request_reviewers (
+    id bigint NOT NULL,
+    user_id bigint NOT NULL,
+    merge_request_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL
+);
+
+CREATE SEQUENCE public.merge_request_reviewers_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.merge_request_reviewers_id_seq OWNED BY public.merge_request_reviewers.id;
+
 CREATE TABLE public.merge_request_user_mentions (
     id bigint NOT NULL,
     merge_request_id integer NOT NULL,
@@ -17144,6 +17160,8 @@ ALTER TABLE ONLY public.merge_request_diffs ALTER COLUMN id SET DEFAULT nextval(
 
 ALTER TABLE ONLY public.merge_request_metrics ALTER COLUMN id SET DEFAULT nextval('public.merge_request_metrics_id_seq'::regclass);
 
+ALTER TABLE ONLY public.merge_request_reviewers ALTER COLUMN id SET DEFAULT nextval('public.merge_request_reviewers_id_seq'::regclass);
+
 ALTER TABLE ONLY public.merge_request_user_mentions ALTER COLUMN id SET DEFAULT nextval('public.merge_request_user_mentions_id_seq'::regclass);
 
 ALTER TABLE ONLY public.merge_requests ALTER COLUMN id SET DEFAULT nextval('public.merge_requests_id_seq'::regclass);
@@ -18262,6 +18280,9 @@ ALTER TABLE ONLY public.merge_request_diffs
 ALTER TABLE ONLY public.merge_request_metrics
     ADD CONSTRAINT merge_request_metrics_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.merge_request_reviewers
+    ADD CONSTRAINT merge_request_reviewers_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.merge_request_user_mentions
     ADD CONSTRAINT merge_request_user_mentions_pkey PRIMARY KEY (id);
 
@@ -20093,6 +20114,10 @@ CREATE INDEX index_merge_request_metrics_on_target_project_id ON public.merge_re
 
 CREATE INDEX index_merge_request_metrics_on_target_project_id_merged_at ON public.merge_request_metrics USING btree (target_project_id, merged_at);
 
+CREATE UNIQUE INDEX index_merge_request_reviewers_on_merge_request_id_and_user_id ON public.merge_request_reviewers USING btree (merge_request_id, user_id);
+
+CREATE INDEX index_merge_request_reviewers_on_user_id ON public.merge_request_reviewers USING btree (user_id);
+
 CREATE UNIQUE INDEX index_merge_request_user_mentions_on_note_id ON public.merge_request_user_mentions USING btree (note_id) WHERE (note_id IS NOT NULL);
 
 CREATE INDEX index_merge_requests_closing_issues_on_issue_id ON public.merge_requests_closing_issues USING btree (issue_id);
@@ -22282,6 +22307,9 @@ ALTER TABLE ONLY public.board_labels
 ALTER TABLE ONLY public.merge_request_blocks
     ADD CONSTRAINT fk_rails_364d4bea8b FOREIGN KEY (blocked_merge_request_id) REFERENCES public.merge_requests(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.merge_request_reviewers
+    ADD CONSTRAINT fk_rails_3704a66140 FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.analytics_cycle_analytics_project_stages
     ADD CONSTRAINT fk_rails_3829e49b66 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
@@ -23056,6 +23084,9 @@ ALTER TABLE ONLY public.alert_management_alert_assignees
 ALTER TABLE ONLY public.geo_hashed_storage_attachments_events
     ADD CONSTRAINT fk_rails_d496b088e9 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.merge_request_reviewers
+    ADD CONSTRAINT fk_rails_d9fec24b9d FOREIGN KEY (merge_request_id) REFERENCES public.merge_requests(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.jira_imports
     ADD CONSTRAINT fk_rails_da617096ce FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE SET NULL;
 

spec/lib/gitlab/import_export/all_models.yml

@@ -120,6 +120,7 @@ merge_requests:
 - award_emoji
 - author
 - assignee
+- reviewers
 - updated_by
 - milestone
 - iteration
@@ -147,6 +148,7 @@ merge_requests:
 - latest_merge_request_diff
 - pipelines_for_merge_request
 - merge_request_assignees
+- merge_request_reviewers
 - suggestions
 - diff_note_positions
 - unresolved_notes

spec/models/merge_request_reviewer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequestReviewer do
+  let(:merge_request) { create(:merge_request) }
+
+  subject { merge_request.merge_request_reviewers.build(reviewer: create(:user)) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') }
+    it { is_expected.to belong_to(:reviewer).class_name('User') }
+  end
+end

spec/models/merge_request_spec.rb

@@ -18,6 +18,7 @@ RSpec.describe MergeRequest do
     it { is_expected.to belong_to(:source_project).class_name('Project') }
     it { is_expected.to belong_to(:merge_user).class_name("User") }
     it { is_expected.to have_many(:assignees).through(:merge_request_assignees) }
+    it { is_expected.to have_many(:reviewers).through(:merge_request_reviewers) }
     it { is_expected.to have_many(:merge_request_diffs) }
     it { is_expected.to have_many(:user_mentions).class_name("MergeRequestUserMention") }
     it { is_expected.to belong_to(:milestone) }