Merge branch 'revert-1403076d' into 'master'

Revert "Merge branch 'revert-253da9ae' into 'master'"

See merge request gitlab-org/gitlab!47074

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, :reviewers,
+                                    :target_project, :latest_merge_request_diff, :approvals, :approved_by_users,
                                     source_project: :route, head_pipeline: :project, target_project: :namespace
                                   ]
                                 end

app/finders/issuable_finder.rb

@@ -406,7 +406,7 @@ class IssuableFinder
     elsif params.filter_by_any_assignee?
       items.assigned
     elsif params.assignee
-      items_assigned_to(items, params.assignee)
+      items.assigned_to(params.assignee)
     elsif params.assignee_id? || params.assignee_username? # assignee not found
       items.none
     else
@@ -414,10 +414,6 @@ 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

@@ -164,13 +164,6 @@ class MergeRequestsFinder < IssuableFinder
   end
   # rubocop: enable CodeReuse/Finder
 
-  # rubocop: disable CodeReuse/ActiveRecord
-  def items_assigned_to(items, user)
-    assignee_or_reviewer = MergeRequest.from_union([super, items.reviewer_assigned_to(user)])
-    items.where(id: assignee_or_reviewer)
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-
   def by_deployments(items)
     env = params[:environment]
     before = params[:deployed_before]

app/models/merge_request.rb

@@ -303,19 +303,6 @@ class MergeRequest < ApplicationRecord
     includes(:metrics)
   end
 
-  scope :reviewer_assigned_to, ->(user) do
-    mr_reviewers_table = MergeRequestReviewer.arel_table
-
-    inner_sql = mr_reviewers_table
-                  .project(Arel::Nodes::True.new)
-                  .where(
-                    mr_reviewers_table[:merge_request_id].eq(MergeRequest.arel_table[:id])
-                      .and(mr_reviewers_table[:user_id].eq(user.id))
-                  ).exists
-
-    where(inner_sql)
-  end
-
   after_save :keep_around_commit, unless: :importing?
 
   alias_attribute :project, :target_project

ee/spec/requests/api/merge_requests_spec.rb

@@ -267,20 +267,12 @@ RSpec.describe API::MergeRequests do
     end
 
     context 'filter merge requests by approval IDs' do
-      let_it_be(:user3) { create(:user) }
-      let_it_be(:merge_request_with_approval) do
+      let!(:merge_request_with_approval) do
         create(:merge_request, author: user, source_project: project, target_project: project, source_branch: 'other-branch').tap do |mr|
           create(:approval, merge_request: mr, user: user2)
         end
       end
 
-      let_it_be(:merge_request_with_multiple_approvals) do
-        create(:merge_request, author: user, source_project: project, target_project: project, source_branch: 'another-branch').tap do |mr|
-          create(:approval, merge_request: mr, user: user2)
-          create(:approval, merge_request: mr, user: user3)
-        end
-      end
-
       before do
         get api('/merge_requests', user), params: { approved_by_ids: approvals_param, scope: :all }
       end
@@ -289,25 +281,7 @@ RSpec.describe API::MergeRequests do
         let(:approvals_param) { [user2.id] }
 
         it 'returns an array of merge requests which have specified the user as an approver' do
-          expect_response_contain_exactly(merge_request_with_approval.id, merge_request_with_multiple_approvals.id)
-        end
-      end
-
-      context 'with multiple specified approved_by ids' do
-        context 'when approved by all users' do
-          let(:approvals_param) { [user2.id, user3.id] }
-
-          it 'returns an array of merge requests which have specified the user as an approver' do
-            expect_response_contain_exactly(merge_request_with_multiple_approvals.id)
-          end
-        end
-
-        context 'when approved by only one user' do
-          let(:approvals_param) { [user.id, user2.id] }
-
-          it 'does not returns any merge request' do
-            expect_empty_array_response
-          end
+          expect_response_contain_exactly(merge_request_with_approval.id)
         end
       end
 
@@ -323,7 +297,7 @@ RSpec.describe API::MergeRequests do
         let(:approvals_param) { 'Any' }
 
         it 'returns an array of merge requests with any approver' do
-          expect_response_contain_exactly(merge_request_with_approval.id, merge_request_with_multiple_approvals.id)
+          expect_response_contain_exactly(merge_request_with_approval.id)
         end
       end
 

spec/features/dashboard/merge_requests_spec.rb

@@ -52,29 +52,20 @@ 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: author_user)
-    end
-
-    let!(:review_requested_merge_request) do
-      create(:merge_request,
-        reviewers: [current_user],
-        source_branch: 'review',
-        source_project: project,
-        author: author_user)
+        author: create(: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: author_user)
+              author: create(:user))
     end
 
     let!(:authored_merge_request) do
@@ -103,7 +94,7 @@ RSpec.describe 'Dashboard Merge Requests' do
       create(:merge_request,
               source_branch: 'fix',
               source_project: project,
-              author: author_user)
+              author: create(:user))
     end
 
     before do
@@ -120,10 +111,6 @@ 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,8 +333,6 @@ 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
@@ -353,6 +351,7 @@ 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
@@ -367,6 +366,7 @@ 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,54 +374,30 @@ RSpec.describe MergeRequestsFinder do
           let(:expected_issuables) { [merge_request1, merge_request2, merge_request3] }
         end
 
-        context 'with just reviewers' do
-          it_behaves_like 'assignee username filter' do
-            before do
-              merge_request4.reviewers = [user3]
-              merge_request4.assignees = []
-            end
+        context 'filtering by group milestone' do
+          let(:group_milestone) { create(:milestone, group: group) }
 
-            let(:params) { { assignee_username: [user3.username] } }
-            let(:expected_issuables) { [merge_request4] }
+          before do
+            merge_request1.update!(milestone: group_milestone)
+            merge_request2.update!(milestone: group_milestone)
           end
-        end
 
-        context 'with an additional reviewer' do
-          it_behaves_like 'assignee username filter' do
-            before do
-              merge_request3.assignees = [user3]
-              merge_request4.reviewers = [user3]
-            end
+          it 'returns merge requests assigned to that group milestone' do
+            params = { milestone_title: group_milestone.title }
 
-            let(:params) { { assignee_username: [user3.username] } }
-            let(:expected_issuables) { [merge_request3, merge_request4] }
-          end
-        end
-      end
-
-      context 'filtering by group milestone' do
-        let(:group_milestone) { create(:milestone, group: group) }
-
-        before do
-          merge_request1.update!(milestone: group_milestone)
-          merge_request2.update!(milestone: group_milestone)
-        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
+            merge_requests = described_class.new(user, params).execute
 
-          expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
-        end
+            expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
+          end
 
-        context 'using NOT' do
-          let(:params) { { not: { milestone_title: group_milestone.title } } }
+          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
+            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)
+              expect(merge_requests).to contain_exactly(merge_request3, merge_request4, merge_request5)
+            end
           end
         end
       end
@@ -587,27 +563,6 @@ RSpec.describe MergeRequestsFinder do
           expect(mrs).to eq([mr2])
         end
       end
-
-      it 'does not raise any exception with complex filters' do
-        # available filters from MergeRequest dashboard UI
-        params = {
-          project_id: project1.id,
-          scope: 'authored',
-          state: 'opened',
-          author_username: user.username,
-          assignee_username: user.username,
-          approver_usernames: [user.username],
-          approved_by_usernames: [user.username],
-          milestone_title: 'none',
-          release_tag: 'none',
-          label_names: 'none',
-          my_reaction_emoji: 'none',
-          draft: 'no'
-        }
-
-        merge_requests = described_class.new(user, params).execute
-        expect { merge_requests.load }.not_to raise_error
-      end
     end
 
     describe '#row_count', :request_store do