Merge branch...

Merge branch '237922-show-merge-requests-where-user-is-designated-as-reviewer-on-the-mr-list-page' into 'master'

Allow merge_request dashboard filter to load review requested MRs

See merge request gitlab-org/gitlab!43497

app/controllers/concerns/issuable_collections.rb

@@ -150,7 +150,7 @@ module IssuableCollections
                                   common_attributes + [:project, project: :namespace]
                                 when 'MergeRequest'
                                   common_attributes + [
-                                    :target_project, :latest_merge_request_diff, :approvals, :approved_by_users,
+                                    :target_project, :latest_merge_request_diff, :approvals, :approved_by_users, :reviewers,
                                     source_project: :route, head_pipeline: :project, target_project: :namespace
                                   ]
                                 end

app/finders/issuable_finder.rb

@@ -397,7 +397,7 @@ class IssuableFinder
     elsif params.filter_by_any_assignee?
       items.assigned
     elsif params.assignee
-      items.assigned_to(params.assignee)
+      items_assigned_to(items, params.assignee)
     elsif params.assignee_id? || params.assignee_username? # assignee not found
       items.none
     else
@@ -405,6 +405,10 @@ class IssuableFinder
     end
   end
 
+  def items_assigned_to(items, user)
+    items.assigned_to(user)
+  end
+
   def by_negated_assignee(items)
     # We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB"
     if not_params.assignees.present?

app/finders/merge_requests_finder.rb

@@ -145,6 +145,10 @@ class MergeRequestsFinder < IssuableFinder
       .execute(items)
   end
   # rubocop: enable CodeReuse/Finder
+
+  def items_assigned_to(items, user)
+    MergeRequest.from_union([super, items.reviewer_assigned_to(user)])
+  end
 end
 
 MergeRequestsFinder.prepend_if_ee('EE::MergeRequestsFinder')

app/models/merge_request.rb

@@ -302,6 +302,10 @@ class MergeRequest < ApplicationRecord
     includes(:metrics)
   end
 
+  scope :reviewer_assigned_to, ->(user) do
+    where("EXISTS (SELECT TRUE FROM merge_request_reviewers WHERE user_id = ? AND merge_request_id = merge_requests.id)", user.id)
+  end
+
   after_save :keep_around_commit, unless: :importing?
 
   alias_attribute :project, :target_project

spec/features/dashboard/merge_requests_spec.rb

@@ -52,20 +52,29 @@ RSpec.describe 'Dashboard Merge Requests' do
   end
 
   context 'merge requests exist' do
+    let_it_be(:author_user) { create(:user) }
     let(:label) { create(:label) }
 
     let!(:assigned_merge_request) do
       create(:merge_request,
         assignees: [current_user],
         source_project: project,
-        author: create(:user))
+        author: author_user)
+    end
+
+    let!(:review_requested_merge_request) do
+      create(:merge_request,
+        reviewers: [current_user],
+        source_branch: 'review',
+        source_project: project,
+        author: author_user)
     end
 
     let!(:assigned_merge_request_from_fork) do
       create(:merge_request,
               source_branch: 'markdown', assignees: [current_user],
               target_project: public_project, source_project: forked_project,
-              author: create(:user))
+              author: author_user)
     end
 
     let!(:authored_merge_request) do
@@ -94,7 +103,7 @@ RSpec.describe 'Dashboard Merge Requests' do
       create(:merge_request,
               source_branch: 'fix',
               source_project: project,
-              author: create(:user))
+              author: author_user)
     end
 
     before do
@@ -111,6 +120,10 @@ RSpec.describe 'Dashboard Merge Requests' do
       expect(page).not_to have_content(labeled_merge_request.title)
     end
 
+    it 'shows review requested merge requests' do
+      expect(page).to have_content(review_requested_merge_request.title)
+    end
+
     it 'shows authored merge requests', :js do
       reset_filters
       input_filtered_search("author:=#{current_user.to_reference}")

spec/finders/merge_requests_finder_spec.rb

@@ -333,6 +333,8 @@ RSpec.describe MergeRequestsFinder do
       end
 
       context 'assignee filtering' do
+        let_it_be(:user3) { create(:user) }
+
         let(:issuables) { described_class.new(user, params).execute }
 
         it_behaves_like 'assignee ID filter' do
@@ -351,7 +353,6 @@ RSpec.describe MergeRequestsFinder do
             merge_request3.assignees = [user2, user3]
           end
 
-          let_it_be(:user3) { create(:user) }
           let(:params) { { assignee_username: [user2.username, user3.username] } }
           let(:expected_issuables) { [merge_request3] }
         end
@@ -366,7 +367,6 @@ RSpec.describe MergeRequestsFinder do
         end
 
         it_behaves_like 'no assignee filter' do
-          let_it_be(:user3) { create(:user) }
           let(:expected_issuables) { [merge_request4, merge_request5] }
         end
 
@@ -374,30 +374,54 @@ RSpec.describe MergeRequestsFinder do
           let(:expected_issuables) { [merge_request1, merge_request2, merge_request3] }
         end
 
-        context 'filtering by group milestone' do
-          let(:group_milestone) { create(:milestone, group: group) }
+        context 'with just reviewers' do
+          it_behaves_like 'assignee username filter' do
+            before do
+              merge_request4.reviewers = [user3]
+              merge_request4.assignees = []
+            end
 
-          before do
-            merge_request1.update!(milestone: group_milestone)
-            merge_request2.update!(milestone: group_milestone)
+            let(:params) { { assignee_username: [user3.username] } }
+            let(:expected_issuables) { [merge_request4] }
           end
+        end
 
-          it 'returns merge requests assigned to that group milestone' do
-            params = { milestone_title: group_milestone.title }
-
-            merge_requests = described_class.new(user, params).execute
+        context 'with an additional reviewer' do
+          it_behaves_like 'assignee username filter' do
+            before do
+              merge_request3.assignees = [user3]
+              merge_request4.reviewers = [user3]
+            end
 
-            expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
+            let(:params) { { assignee_username: [user3.username] } }
+            let(:expected_issuables) { [merge_request3, merge_request4] }
           end
+        end
+      end
 
-          context 'using NOT' do
-            let(:params) { { not: { milestone_title: group_milestone.title } } }
+      context 'filtering by group milestone' do
+        let(:group_milestone) { create(:milestone, group: group) }
 
-            it 'returns MRs not assigned to that group milestone' do
-              merge_requests = described_class.new(user, params).execute
+        before do
+          merge_request1.update!(milestone: group_milestone)
+          merge_request2.update!(milestone: group_milestone)
+        end
 
-              expect(merge_requests).to contain_exactly(merge_request3, merge_request4, merge_request5)
-            end
+        it 'returns merge requests assigned to that group milestone' do
+          params = { milestone_title: group_milestone.title }
+
+          merge_requests = described_class.new(user, params).execute
+
+          expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
+        end
+
+        context 'using NOT' do
+          let(:params) { { not: { milestone_title: group_milestone.title } } }
+
+          it 'returns MRs not assigned to that group milestone' do
+            merge_requests = described_class.new(user, params).execute
+
+            expect(merge_requests).to contain_exactly(merge_request3, merge_request4, merge_request5)
           end
         end
       end