Fix an issue when filtering merge requests with more than one label

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 140 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1c21ad36b69..499febdfd08 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -43,6 +43,7 @@ v 8.8.0 (unreleased)
 v 8.7.4
   - Fix always showing build notification message when switching between merge requests
   - Links for Redmine issue references are generated correctly again (Benedikt Huss)
+  - Fix an issue when filtering merge requests with more than one label. !3886
 
 v 8.7.3
   - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 7a3db742030..c1248b53031 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -124,8 +124,8 @@ module Issuable
     end
 
     def with_label(title)
-      if title.is_a?(Array) && title.count > 1
-        joins(:labels).where(labels: { title: title }).group('issues.id').having("count(distinct labels.title) = #{title.count}")
+      if title.is_a?(Array) && title.size > 1
+        joins(:labels).where(labels: { title: title }).group(arel_table[:id]).having("COUNT(DISTINCT labels.title) = #{title.size}")
       else
         joins(:labels).where(labels: { title: title })
       end
diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb
new file mode 100644
index 00000000000..cc7f78e7325
--- /dev/null
+++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb
@@ -0,0 +1,137 @@
+require 'spec_helper'
+
+describe 'Projects > Merge requests > User lists merge requests', feature: true do
+  include SortingHelper
+
+  let(:project) { create(:project, :public) }
+  let(:user) { create(:user) }
+
+  before do
+    @fix = create(:merge_request,
+                  title: 'fix',
+                  source_project: project,
+                  source_branch: 'fix',
+                  assignee: user,
+                  milestone: create(:milestone, due_date: '2013-12-11'),
+                  created_at: 1.minute.ago,
+                  updated_at: 1.minute.ago)
+    create(:merge_request,
+           title: 'markdown',
+           source_project: project,
+           source_branch: 'markdown',
+           assignee: user,
+           milestone: create(:milestone, due_date: '2013-12-12'),
+           created_at: 2.minutes.ago,
+           updated_at: 2.minutes.ago)
+    create(:merge_request,
+           title: 'lfs',
+           source_project: project,
+           source_branch: 'lfs',
+           created_at: 3.minutes.ago,
+           updated_at: 10.seconds.ago)
+  end
+
+  it 'filters on no assignee' do
+    visit_merge_requests(project, assignee_id: IssuableFinder::NONE)
+
+    expect(current_path).to eq(namespace_project_merge_requests_path(project.namespace, project))
+    expect(page).to have_content 'lfs'
+    expect(page).not_to have_content 'fix'
+    expect(page).not_to have_content 'markdown'
+  end
+
+  it 'filters on a specific assignee' do
+    visit_merge_requests(project, assignee_id: user.id)
+
+    expect(page).not_to have_content 'lfs'
+    expect(page).to have_content 'fix'
+    expect(page).to have_content 'markdown'
+  end
+
+  it 'sorts by newest' do
+    visit_merge_requests(project, sort: sort_value_recently_created)
+
+    expect(first_merge_request).to include('lfs')
+    expect(last_merge_request).to include('fix')
+  end
+
+  it 'sorts by oldest' do
+    visit_merge_requests(project, sort: sort_value_oldest_created)
+
+    expect(first_merge_request).to include('fix')
+    expect(last_merge_request).to include('lfs')
+  end
+
+  it 'sorts by last updated' do
+    visit_merge_requests(project, sort: sort_value_recently_updated)
+
+    expect(first_merge_request).to include('lfs')
+  end
+
+  it 'sorts by oldest updated' do
+    visit_merge_requests(project, sort: sort_value_oldest_updated)
+
+    expect(first_merge_request).to include('markdown')
+  end
+
+  it 'sorts by milestone due soon' do
+    visit_merge_requests(project, sort: sort_value_milestone_soon)
+
+    expect(first_merge_request).to include('fix')
+  end
+
+  it 'sorts by milestone due later' do
+    visit_merge_requests(project, sort: sort_value_milestone_later)
+
+    expect(first_merge_request).to include('markdown')
+  end
+
+  it 'filters on one label and sorts by due soon' do
+    label = create(:label, project: project)
+    create(:label_link, label: label, target: @fix)
+
+    visit_merge_requests(project, label_name: [label.name],
+                                  sort: sort_value_due_date_soon)
+
+    expect(first_merge_request).to include('fix')
+  end
+
+  context 'while filtering on two labels' do
+    let(:label) { create(:label, project: project) }
+    let(:label2) { create(:label, project: project) }
+
+    before do
+      create(:label_link, label: label, target: @fix)
+      create(:label_link, label: label2, target: @fix)
+    end
+
+    it 'sorts by due soon' do
+      visit_merge_requests(project, label_name: [label.name, label2.name],
+                                    sort: sort_value_due_date_soon)
+
+      expect(first_merge_request).to include('fix')
+    end
+
+    context 'filter on assignee and' do
+      it 'sorts by due soon' do
+        visit_merge_requests(project, label_name: [label.name, label2.name],
+                                      assignee_id: user.id,
+                                      sort: sort_value_due_date_soon)
+
+        expect(first_merge_request).to include('fix')
+      end
+    end
+  end
+
+  def visit_merge_requests(project, opts = {})
+    visit namespace_project_merge_requests_path(project.namespace, project, opts)
+  end
+
+  def first_merge_request
+    page.all('ul.mr-list > li').first.text
+  end
+
+  def last_merge_request
+    page.all('ul.mr-list > li').last.text
+  end
+end