From 2d154e54b68b8a2e78ea5a6de8851f1b1f9240e3 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 10 May 2016 19:20:27 +0000 Subject: Merge branch '15529-fix=missing-from-clause-for-table-issues' into 'master' Fix an issue when filtering merge requests with more than one label Fixes #15529. See merge request !3886 --- CHANGELOG | 3 + app/models/concerns/issuable.rb | 4 +- .../user_lists_merge_requests_spec.rb | 137 +++++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 spec/features/merge_requests/user_lists_merge_requests_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 27728fca728..2c882f09d57 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,9 @@ v 8.7.4 - Running rake gitlab:db:drop_tables now drops tables with cascade !4020 - Running rake gitlab:db:drop_tables uses "IF EXISTS" as a precaution !4100 - Use a case-insensitive comparison in sanitizing URI schemes + - 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 d5166e81474..e2f534d2483 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -123,8 +123,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 -- cgit v1.2.1