Rename NoMilestone to Milestone::None

Also refactors IssuableFinder to avoid redundant title check.

6 files changed, 10 insertions, 24 deletions

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