Commit 99e7b038 authored by David Kim's avatar David Kim

Revert "Load review requested MRs in assignee filter"

This reverts commit 550f2982.
parent 0745b989
...@@ -150,7 +150,7 @@ module IssuableCollections ...@@ -150,7 +150,7 @@ module IssuableCollections
common_attributes + [:project, project: :namespace] common_attributes + [:project, project: :namespace]
when 'MergeRequest' when 'MergeRequest'
common_attributes + [ 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 source_project: :route, head_pipeline: :project, target_project: :namespace
] ]
end end
......
...@@ -399,7 +399,7 @@ class IssuableFinder ...@@ -399,7 +399,7 @@ class IssuableFinder
elsif params.filter_by_any_assignee? elsif params.filter_by_any_assignee?
items.assigned items.assigned
elsif params.assignee elsif params.assignee
items_assigned_to(items, params.assignee) items.assigned_to(params.assignee)
elsif params.assignee_id? || params.assignee_username? # assignee not found elsif params.assignee_id? || params.assignee_username? # assignee not found
items.none items.none
else else
...@@ -407,10 +407,6 @@ class IssuableFinder ...@@ -407,10 +407,6 @@ class IssuableFinder
end end
end end
def items_assigned_to(items, user)
items.assigned_to(user)
end
def by_negated_assignee(items) def by_negated_assignee(items)
# We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB" # We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB"
if not_params.assignees.present? if not_params.assignees.present?
......
...@@ -164,10 +164,6 @@ class MergeRequestsFinder < IssuableFinder ...@@ -164,10 +164,6 @@ class MergeRequestsFinder < IssuableFinder
end end
# rubocop: enable CodeReuse/Finder # rubocop: enable CodeReuse/Finder
def items_assigned_to(items, user)
MergeRequest.from_union([super, items.reviewer_assigned_to(user)])
end
def by_deployments(items) def by_deployments(items)
# Until this feature flag is enabled permanently, we retain the old # Until this feature flag is enabled permanently, we retain the old
# filtering behaviour/code. # filtering behaviour/code.
......
...@@ -302,10 +302,6 @@ class MergeRequest < ApplicationRecord ...@@ -302,10 +302,6 @@ class MergeRequest < ApplicationRecord
includes(:metrics) includes(:metrics)
end 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? after_save :keep_around_commit, unless: :importing?
alias_attribute :project, :target_project alias_attribute :project, :target_project
......
...@@ -52,29 +52,20 @@ RSpec.describe 'Dashboard Merge Requests' do ...@@ -52,29 +52,20 @@ RSpec.describe 'Dashboard Merge Requests' do
end end
context 'merge requests exist' do context 'merge requests exist' do
let_it_be(:author_user) { create(:user) }
let(:label) { create(:label) } let(:label) { create(:label) }
let!(:assigned_merge_request) do let!(:assigned_merge_request) do
create(:merge_request, create(:merge_request,
assignees: [current_user], assignees: [current_user],
source_project: project, source_project: project,
author: author_user) author: create(:user))
end
let!(:review_requested_merge_request) do
create(:merge_request,
reviewers: [current_user],
source_branch: 'review',
source_project: project,
author: author_user)
end end
let!(:assigned_merge_request_from_fork) do let!(:assigned_merge_request_from_fork) do
create(:merge_request, create(:merge_request,
source_branch: 'markdown', assignees: [current_user], source_branch: 'markdown', assignees: [current_user],
target_project: public_project, source_project: forked_project, target_project: public_project, source_project: forked_project,
author: author_user) author: create(:user))
end end
let!(:authored_merge_request) do let!(:authored_merge_request) do
...@@ -103,7 +94,7 @@ RSpec.describe 'Dashboard Merge Requests' do ...@@ -103,7 +94,7 @@ RSpec.describe 'Dashboard Merge Requests' do
create(:merge_request, create(:merge_request,
source_branch: 'fix', source_branch: 'fix',
source_project: project, source_project: project,
author: author_user) author: create(:user))
end end
before do before do
...@@ -120,10 +111,6 @@ RSpec.describe 'Dashboard Merge Requests' do ...@@ -120,10 +111,6 @@ RSpec.describe 'Dashboard Merge Requests' do
expect(page).not_to have_content(labeled_merge_request.title) expect(page).not_to have_content(labeled_merge_request.title)
end 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 it 'shows authored merge requests', :js do
reset_filters reset_filters
input_filtered_search("author:=#{current_user.to_reference}") input_filtered_search("author:=#{current_user.to_reference}")
......
...@@ -333,8 +333,6 @@ RSpec.describe MergeRequestsFinder do ...@@ -333,8 +333,6 @@ RSpec.describe MergeRequestsFinder do
end end
context 'assignee filtering' do context 'assignee filtering' do
let_it_be(:user3) { create(:user) }
let(:issuables) { described_class.new(user, params).execute } let(:issuables) { described_class.new(user, params).execute }
it_behaves_like 'assignee ID filter' do it_behaves_like 'assignee ID filter' do
...@@ -353,6 +351,7 @@ RSpec.describe MergeRequestsFinder do ...@@ -353,6 +351,7 @@ RSpec.describe MergeRequestsFinder do
merge_request3.assignees = [user2, user3] merge_request3.assignees = [user2, user3]
end end
let_it_be(:user3) { create(:user) }
let(:params) { { assignee_username: [user2.username, user3.username] } } let(:params) { { assignee_username: [user2.username, user3.username] } }
let(:expected_issuables) { [merge_request3] } let(:expected_issuables) { [merge_request3] }
end end
...@@ -367,6 +366,7 @@ RSpec.describe MergeRequestsFinder do ...@@ -367,6 +366,7 @@ RSpec.describe MergeRequestsFinder do
end end
it_behaves_like 'no assignee filter' do it_behaves_like 'no assignee filter' do
let_it_be(:user3) { create(:user) }
let(:expected_issuables) { [merge_request4, merge_request5] } let(:expected_issuables) { [merge_request4, merge_request5] }
end end
...@@ -374,54 +374,30 @@ RSpec.describe MergeRequestsFinder do ...@@ -374,54 +374,30 @@ RSpec.describe MergeRequestsFinder do
let(:expected_issuables) { [merge_request1, merge_request2, merge_request3] } let(:expected_issuables) { [merge_request1, merge_request2, merge_request3] }
end end
context 'with just reviewers' do context 'filtering by group milestone' do
it_behaves_like 'assignee username filter' do let(:group_milestone) { create(:milestone, group: group) }
before do
merge_request4.reviewers = [user3]
merge_request4.assignees = []
end
let(:params) { { assignee_username: [user3.username] } }
let(:expected_issuables) { [merge_request4] }
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
let(:params) { { assignee_username: [user3.username] } } before do
let(:expected_issuables) { [merge_request3, merge_request4] } merge_request1.update!(milestone: group_milestone)
merge_request2.update!(milestone: group_milestone)
end end
end
end
context 'filtering by group milestone' do it 'returns merge requests assigned to that group milestone' do
let(:group_milestone) { create(:milestone, group: group) } params = { milestone_title: group_milestone.title }
before do merge_requests = described_class.new(user, params).execute
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
expect(merge_requests).to contain_exactly(merge_request1, merge_request2) expect(merge_requests).to contain_exactly(merge_request1, merge_request2)
end end
context 'using NOT' do context 'using NOT' do
let(:params) { { not: { milestone_title: group_milestone.title } } } let(:params) { { not: { milestone_title: group_milestone.title } } }
it 'returns MRs not assigned to that group milestone' do it 'returns MRs not assigned to that group milestone' do
merge_requests = described_class.new(user, params).execute 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 end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment