diff options
author | Felipe Artur <felipefac@gmail.com> | 2016-05-27 16:53:20 -0300 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2016-05-31 10:35:57 -0300 |
commit | 30e61ed79c89de4c31a4d2057f710ad2b0704389 (patch) | |
tree | f4941eda8679c0e82bea0a4b0ee05f191d93696b | |
parent | f9bb9151b595fdc1afc1742bb51c816965908f53 (diff) | |
download | gitlab-ce-30e61ed79c89de4c31a4d2057f710ad2b0704389.tar.gz |
Fix error 500 when sorting issues by milestone due date and filtering by labels
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 21 | ||||
-rw-r--r-- | spec/features/merge_requests/user_lists_merge_requests_spec.rb | 9 |
4 files changed, 30 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index a0194a1ba25..a885e44f38b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,7 @@ v 8.9.0 (unreleased) - Changed the Slack build message to use the singular duration if necessary (Aran Koning) - Fix issues filter when ordering by milestone - Todos will display target state if issuable target is 'Closed' or 'Merged' + - Fix bug when sorting issues by milestone due date and filtering by two or more labels - Remove 'main language' feature - Measure queue duration between gitlab-workhorse and Rails diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 8ed3ccf1c02..7d8c56f4c22 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -271,7 +271,7 @@ class IssuableFinder if filter_by_no_label? items = items.without_label else - items = items.with_label(label_names) + items = items.with_label(label_names, params[:sort]) if projects items = items.where(labels: { project_id: projects }) end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 91315b3459f..117d66131ae 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -126,13 +126,30 @@ module Issuable joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC") end - def with_label(title) + def with_label(title, sort = nil) 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}") + joins(:labels).where(labels: { title: title }).group(*get_grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}") else joins(:labels).where(labels: { title: title }) end end + + # Includes table keys in group by clause when sorting + # preventing errors in postgres + # + # Returns an array of arel columns + + def get_grouping_columns(sort) + default_columns = [arel_table[:id]] + + if ["milestone_due_desc", "milestone_due_asc"].include?(sort) + milestone_table = Milestone.arel_table + default_columns << milestone_table[:id] + default_columns << milestone_table[:due_date] + end + + default_columns + end end def today? diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index 2c7e1c748ad..1c130057c56 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -131,6 +131,15 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(first_merge_request).to include('fix') expect(count_merge_requests).to eq(1) end + + it 'sorts by recently due milestone' do + visit namespace_project_merge_requests_path(project.namespace, project, + label_name: [label.name, label2.name], + assignee_id: user.id, + sort: sort_value_milestone_soon) + + expect(first_merge_request).to include('fix') + end end end |