From 3b2223f11676831f9e995095de13926e1a39b462 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Thu, 5 May 2016 16:23:51 -0300
Subject: [PATCH] improve ordering sql for milestones

---
 app/models/concerns/issuable.rb       |  4 +--
 spec/models/concerns/issuable_spec.rb | 49 +++++++++++----------------
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 4b21cf02919..ce73fe7035e 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -37,8 +37,8 @@ module Issuable
     scope :closed, -> { with_state(:closed) }
 
     scope :left_joins_milestones,    -> { joins("LEFT OUTER JOIN milestones ON #{table_name}.milestone_id = milestones.id") }
-    scope :order_milestone_due_desc, -> { left_joins_milestones.reorder('CASE WHEN milestones.due_date IS NULL then 0 ELSE 1 END DESC, CASE WHEN milestones.id IS NULL then 0 ELSE 1 END DESC, milestones.due_date DESC') }
-    scope :order_milestone_due_asc,  -> { left_joins_milestones.reorder('CASE WHEN milestones.due_date IS NULL then 1 ELSE 0 END ASC, CASE WHEN milestones.id IS NULL then 1 ELSE 0 END ASC, milestones.due_date ASC') }
+    scope :order_milestone_due_desc, -> { left_joins_milestones.reorder('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date DESC') }
+    scope :order_milestone_due_asc,  -> { left_joins_milestones.reorder('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date ASC') }
 
     scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) }
     scope :join_project, -> { joins(:project) }
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 2bb770df4fd..56a4313e8af 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -115,41 +115,30 @@ describe Issue, "Issuable" do
   end
 
   describe "#sort" do
-    #Correct order is:
-    #Issues/MRs with milestones ordered by date
-    #Issues/MRs with milestones without dates
-    #Issues/MRs without milestones
+    let(:project) { build_stubbed(:empty_project) }
 
-    let!(:issue) { create(:issue) }
-    let(:project) { issue.project }
-    let!(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) }
-    let!(:late_milestone) { create(:milestone, project: project, due_date: 30.days.from_now) }
-    let!(:issue1) { create(:issue, project: project, milestone: early_milestone) }
-    let!(:issue2) { create(:issue, project: project, milestone: late_milestone) }
-    let!(:issue3) { create(:issue, project: project) }
+    context "by milestone due date" do
+      #Correct order is:
+      #Issues/MRs with milestones ordered by date
+      #Issues/MRs with milestones without dates
+      #Issues/MRs without milestones
+
+      let!(:issue) { create(:issue, project: project) }
+      let!(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) }
+      let!(:late_milestone) { create(:milestone, project: project, due_date: 30.days.from_now) }
+      let!(:issue1) { create(:issue, project: project, milestone: early_milestone) }
+      let!(:issue2) { create(:issue, project: project, milestone: late_milestone) }
+      let!(:issue3) { create(:issue, project: project) }
 
 
-    context "milestone due later" do
-      subject { Issue.where(project_id: project.id).order_milestone_due_desc }
-      before  { @issues = subject }
-
-      it "puts issues with nil values at the end of collection" do
-        expect(@issues.first).to eq(issue2)
-        expect(@issues.second).to eq(issue1)
-        expect(@issues.third).to eq(issue)
-        expect(@issues.fourth).to eq(issue3)
+      it "sorts desc" do
+        issues = project.issues.sort('milestone_due_desc')
+        expect(issues).to match_array([issue2, issue1, issue, issue3])
       end
-    end
 
-    context "milestone due soon" do
-      subject { Issue.where(project_id: project.id).order_milestone_due_asc }
-      before  { @issues = subject }
-
-      it "puts issues with nil values at the end of collection" do
-        expect(@issues.first).to eq(issue1)
-        expect(@issues.second).to eq(issue2)
-        expect(@issues.third).to eq(issue)
-        expect(@issues.fourth).to eq(issue3)
+      it "sorts asc" do
+        issues = project.issues.sort('milestone_due_asc')
+        expect(issues).to match_array([issue1, issue2, issue, issue3])
       end
     end
   end