Merge branch 'issue_15557' into 'master'

Fix error 500 when sorting issues by milestone due date and filtering by labels

fixes #15557 

See merge request !4327

5 files changed, 22 insertions, 6 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1429ee40bb7..9b2a9d298b3 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -263,7 +263,7 @@ class ApplicationController < ActionController::Base
       # internal repos where you are not a member. Enable this filter
       # or improve current implementation to filter only issues you
       # created or assigned or mentioned
-      #@filter_params[:authorized_only] = true
+      # @filter_params[:authorized_only] = true
     end
 
     @filter_params
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 17ce8e2ad20..d54284d7b20 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -205,7 +205,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def branch_from
-    #This is always source
+    # This is always source
     @source_project = @merge_request.nil? ? @project : @merge_request.source_project
     @commit = @repository.commit(params[:ref]) if params[:ref].present?
     render layout: false
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..9aaaf787372 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -126,13 +126,29 @@ 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(*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 grouping_columns(sort)
+      grouping_columns = [arel_table[:id]]
+
+      if ["milestone_due_desc", "milestone_due_asc"].include?(sort)
+        milestone_table = Milestone.arel_table
+        grouping_columns << milestone_table[:id]
+        grouping_columns << milestone_table[:due_date]
+      end
+
+      grouping_columns
+    end
   end
 
   def today?
diff --git a/app/models/key.rb b/app/models/key.rb
index d52afda67d1..0532e84f47d 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -26,7 +26,7 @@ class Key < ActiveRecord::Base
   end
 
   def publishable_key
-    #Removes anything beyond the keytype and key itself
+    # Removes anything beyond the keytype and key itself
     self.key.split[0..1].join(' ')
   end