Merge branch 'id-mr-list-when-filtered-by-approvers-only' into 'master'

Consider array params on rendering MR list on dashboard

See merge request gitlab-org/gitlab-ce!26623

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)