summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIgor Drozdov <idrozdov@gitlab.com>2019-03-27 00:16:48 +0300
committerIgor Drozdov <idrozdov@gitlab.com>2019-04-03 17:29:56 +0300
commitb3f5413afad7e44b2d85eb02d3ed8ba3294a153a (patch)
treee26536859e34ff7bec87a22324057694617598c3
parentdb12e729a7f0615ccb2fd8cd3209bddc3f7bfe39 (diff)
downloadgitlab-ce-id-mr-list-when-filtered-by-approvers-only.tar.gz
Consider array params on rendering MR list on dashboardid-mr-list-when-filtered-by-approvers-only
This fixes the bug, when approver filter is provided, but dashboard asks to enter any filter
-rw-r--r--app/controllers/dashboard_controller.rb5
-rw-r--r--app/finders/issuable_finder.rb1
-rw-r--r--spec/controllers/concerns/issuable_collections_spec.rb4
-rw-r--r--spec/controllers/dashboard_controller_spec.rb33
-rw-r--r--spec/features/dashboard/merge_requests_spec.rb25
5 files changed, 64 insertions, 4 deletions
diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index 75329b05a6f..1a97b39d3ae 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -46,7 +46,10 @@ class DashboardController < Dashboard::ApplicationController
end
def check_filters_presence!
- @no_filters_set = finder_type.scalar_params.none? { |k| params.key?(k) }
+ no_scalar_filters_set = finder_type.scalar_params.none? { |k| params.key?(k) }
+ no_array_filters_set = finder_type.array_params.none? { |k, _| params.key?(k) }
+
+ @no_filters_set = no_scalar_filters_set && no_array_filters_set
return unless @no_filters_set
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 6eab8c5ee51..b6be2895d85 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -53,7 +53,6 @@ class IssuableFinder
assignee_username
author_id
author_username
- label_name
milestone_title
my_reaction_emoji
search
diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb
index 8580900215c..a82b66361ca 100644
--- a/spec/controllers/concerns/issuable_collections_spec.rb
+++ b/spec/controllers/concerns/issuable_collections_spec.rb
@@ -117,7 +117,7 @@ describe IssuableCollections do
due_date: '2017-01-01',
group_id: '3',
iids: '4',
- label_name: 'foo',
+ label_name: ['foo'],
milestone_title: 'bar',
my_reaction_emoji: 'thumbsup',
non_archived: 'true',
@@ -142,7 +142,7 @@ describe IssuableCollections do
'author_id' => '2',
'author_username' => 'user2',
'confidential' => true,
- 'label_name' => 'foo',
+ 'label_name' => ['foo'],
'milestone_title' => 'bar',
'my_reaction_emoji' => 'thumbsup',
'due_date' => '2017-01-01',
diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb
index c857a78d5e8..b039ec2906c 100644
--- a/spec/controllers/dashboard_controller_spec.rb
+++ b/spec/controllers/dashboard_controller_spec.rb
@@ -23,4 +23,37 @@ describe DashboardController do
it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first
it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics
+
+ describe "#check_filters_presence!" do
+ let(:user) { create(:user) }
+
+ before do
+ sign_in(user)
+ get :merge_requests, params: params
+ end
+
+ context "no filters" do
+ let(:params) { {} }
+
+ it 'sets @no_filters_set to false' do
+ expect(assigns[:no_filters_set]).to eq(true)
+ end
+ end
+
+ context "scalar filters" do
+ let(:params) { { author_id: user.id } }
+
+ it 'sets @no_filters_set to false' do
+ expect(assigns[:no_filters_set]).to eq(false)
+ end
+ end
+
+ context "array filters" do
+ let(:params) { { label_name: ['bug'] } }
+
+ it 'sets @no_filters_set to false' do
+ expect(assigns[:no_filters_set]).to eq(false)
+ end
+ end
+ end
end
diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb
index 9ffa75aee47..4965770605a 100644
--- a/spec/features/dashboard/merge_requests_spec.rb
+++ b/spec/features/dashboard/merge_requests_spec.rb
@@ -44,6 +44,8 @@ describe 'Dashboard Merge Requests' do
end
context 'merge requests exist' do
+ let(:label) { create(:label) }
+
let!(:assigned_merge_request) do
create(:merge_request,
assignee: current_user,
@@ -72,6 +74,14 @@ describe 'Dashboard Merge Requests' do
target_project: public_project, source_project: forked_project)
end
+ let!(:labeled_merge_request) do
+ create(:labeled_merge_request,
+ source_branch: 'labeled',
+ labels: [label],
+ author: current_user,
+ source_project: project)
+ end
+
let!(:other_merge_request) do
create(:merge_request,
source_branch: 'fix',
@@ -90,6 +100,7 @@ describe 'Dashboard Merge Requests' do
expect(page).not_to have_content(authored_merge_request.title)
expect(page).not_to have_content(authored_merge_request_from_fork.title)
expect(page).not_to have_content(other_merge_request.title)
+ expect(page).not_to have_content(labeled_merge_request.title)
end
it 'shows authored merge requests', :js do
@@ -98,7 +109,21 @@ describe 'Dashboard Merge Requests' do
expect(page).to have_content(authored_merge_request.title)
expect(page).to have_content(authored_merge_request_from_fork.title)
+ expect(page).to have_content(labeled_merge_request.title)
+
+ expect(page).not_to have_content(assigned_merge_request.title)
+ expect(page).not_to have_content(assigned_merge_request_from_fork.title)
+ expect(page).not_to have_content(other_merge_request.title)
+ end
+
+ it 'shows labeled merge requests', :js do
+ reset_filters
+ input_filtered_search("label:#{label.name}")
+ expect(page).to have_content(labeled_merge_request.title)
+
+ expect(page).not_to have_content(authored_merge_request.title)
+ expect(page).not_to have_content(authored_merge_request_from_fork.title)
expect(page).not_to have_content(assigned_merge_request.title)
expect(page).not_to have_content(assigned_merge_request_from_fork.title)
expect(page).not_to have_content(other_merge_request.title)