From d00cb00d6b62c561da79b3fc0eab579364b3e91c Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Sun, 28 Jun 2015 16:12:32 -0400
Subject: [PATCH] Rename NoMilestone to Milestone::None

Also refactors IssuableFinder to avoid redundant title check.
---
 app/finders/issuable_finder.rb                      | 11 +++--------
 app/helpers/milestones_helper.rb                    |  2 +-
 app/models/milestone.rb                             |  4 ++++
 app/models/no_milestone.rb                          | 13 -------------
 spec/features/issues/filter_by_milestone_spec.rb    |  2 +-
 .../merge_requests/filter_by_milestone_spec.rb      |  2 +-
 6 files changed, 10 insertions(+), 24 deletions(-)
 delete mode 100644 app/models/no_milestone.rb

diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 872c63d82bd..ab89aa2c53a 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -10,7 +10,7 @@
 #     state: 'open' or 'closed' or 'all'
 #     group_id: integer
 #     project_id: integer
-#     milestone_id: integer
+#     milestone_title: string
 #     assignee_id: integer
 #     search: string
 #     label_name: string
@@ -76,7 +76,7 @@ class IssuableFinder
     return @milestones if defined?(@milestones)
 
     @milestones =
-      if milestones? && params[:milestone_title] != NoMilestone.title
+      if milestones? && params[:milestone_title] != Milestone::None.title
         Milestone.where(title: params[:milestone_title])
       else
         nil
@@ -183,12 +183,7 @@ class IssuableFinder
 
   def by_milestone(items)
     if milestones?
-      # `milestone_title` will still be present when "No Milestone" is selected
-      if params[:milestone_title] != NoMilestone.title
-        items = items.where(milestone_id: milestones.try(:pluck, :id))
-      else
-        items = items.where(milestone_id: NoMilestone.id)
-      end
+      items = items.where(milestone_id: milestones.try(:pluck, :id))
     end
 
     items
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 3dedc405365..132a893e532 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -29,7 +29,7 @@ module MilestonesHelper
       end.active
 
     grouped_milestones = Milestones::GroupService.new(milestones).execute
-    grouped_milestones.unshift(NoMilestone)
+    grouped_milestones.unshift(Milestone::None)
 
     options_from_collection_for_select(grouped_milestones, 'title', 'title', params[:milestone_title])
   end
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index e0c5fec97b7..d28f3c8d3f9 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -14,6 +14,10 @@
 #
 
 class Milestone < ActiveRecord::Base
+  # Represents a "No Milestone" state used for filtering Issues and Merge
+  # Requests that have no milestone assigned.
+  None = Struct.new(:title).new('No Milestone')
+
   include InternalId
   include Sortable
 
diff --git a/app/models/no_milestone.rb b/app/models/no_milestone.rb
deleted file mode 100644
index 0c7418b5e29..00000000000
--- a/app/models/no_milestone.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-# NoMilestone
-#
-# Represents a "No Milestone" state used for filtering Issues and Merge Requests
-# that have no milestone assigned.
-class NoMilestone
-  def self.id
-    nil
-  end
-
-  def self.title
-    'No Milestone'
-  end
-end
diff --git a/spec/features/issues/filter_by_milestone_spec.rb b/spec/features/issues/filter_by_milestone_spec.rb
index ad8adf4d372..284e38a1a09 100644
--- a/spec/features/issues/filter_by_milestone_spec.rb
+++ b/spec/features/issues/filter_by_milestone_spec.rb
@@ -13,7 +13,7 @@ feature 'Issue filtering by Milestone' do
     create(:issue, project: project)
 
     visit_issues
-    filter_by_milestone(NoMilestone.title)
+    filter_by_milestone(Milestone::None.title)
 
     expect(page).to have_css('.issue-title', count: 1)
   end
diff --git a/spec/features/merge_requests/filter_by_milestone_spec.rb b/spec/features/merge_requests/filter_by_milestone_spec.rb
index 56a9603f139..308d8101bdd 100644
--- a/spec/features/merge_requests/filter_by_milestone_spec.rb
+++ b/spec/features/merge_requests/filter_by_milestone_spec.rb
@@ -13,7 +13,7 @@ feature 'Merge Request filtering by Milestone' do
     create(:merge_request, :simple, source_project: project)
 
     visit_merge_requests
-    filter_by_milestone(NoMilestone.title)
+    filter_by_milestone(Milestone::None.title)
 
     expect(page).to have_css('.merge-request-title', count: 1)
   end