Merge branch 'revert-2e7f4bbb' into 'master'

Revert milestone filter behavior for "any"  (CE-backport)

See merge request gitlab-org/gitlab-ce!31583

7 files changed, 23 insertions, 51 deletions

diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js
index 01ed27c49fb..43949d5cc86 100644
--- a/app/assets/javascripts/milestone_select.js
+++ b/app/assets/javascripts/milestone_select.js
@@ -55,7 +55,7 @@ export default class MilestoneSelect {
       const $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon');
       const $value = $block.find('.value');
       const $loading = $block.find('.block-loading').fadeOut();
-      selectedMilestoneDefault = showAny ? __('Any Milestone') : null;
+      selectedMilestoneDefault = showAny ? '' : null;
       selectedMilestoneDefault =
         showNo && defaultNo ? __('No Milestone') : selectedMilestoneDefault;
       selectedMilestone = $dropdown.data('selected') || selectedMilestoneDefault;
@@ -74,16 +74,14 @@ export default class MilestoneSelect {
             if (showAny) {
               extraOptions.push({
                 id: null,
-                // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
-                name: 'Any',
+                name: null,
                 title: __('Any Milestone'),
               });
             }
             if (showNo) {
               extraOptions.push({
                 id: -1,
-                // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
-                name: 'None',
+                name: __('No Milestone'),
                 title: __('No Milestone'),
               });
             }
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 86970ae3219..1773ac2d508 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -484,19 +484,22 @@ class IssuableFinder
 
   # rubocop: disable CodeReuse/ActiveRecord
   def by_milestone(items)
-    return items unless milestones?
-    return items if filter_by_any_milestone?
-
-    if filter_by_no_milestone?
-      items.left_joins_milestones.where(milestone_id: nil)
-    elsif filter_by_upcoming_milestone?
-      upcoming_ids = Milestone.upcoming_ids(projects, related_groups)
-      items.left_joins_milestones.where(milestone_id: upcoming_ids)
-    elsif filter_by_started_milestone?
-      items.left_joins_milestones.merge(Milestone.started)
-    else
-      items.with_milestone(params[:milestone_title])
+    if milestones?
+      if filter_by_no_milestone?
+        items = items.left_joins_milestones.where(milestone_id: [-1, nil])
+      elsif filter_by_any_milestone?
+        items = items.any_milestone
+      elsif filter_by_upcoming_milestone?
+        upcoming_ids = Milestone.upcoming_ids(projects, related_groups)
+        items = items.left_joins_milestones.where(milestone_id: upcoming_ids)
+      elsif filter_by_started_milestone?
+        items = items.left_joins_milestones.merge(Milestone.started)
+      else
+        items = items.with_milestone(params[:milestone_title])
+      end
     end
+
+    items
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 60266992ee1..2ad2838111e 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -4,8 +4,8 @@ class Milestone < ApplicationRecord
   # Represents a "No Milestone" state used for filtering Issues and Merge
   # Requests that have no milestone assigned.
   MilestoneStruct = Struct.new(:title, :name, :id)
-  None = MilestoneStruct.new('No Milestone', 'No Milestone', -1)
-  Any = MilestoneStruct.new('Any Milestone', '', nil)
+  None = MilestoneStruct.new('No Milestone', 'No Milestone', 0)
+  Any = MilestoneStruct.new('Any Milestone', '', -1)
   Upcoming = MilestoneStruct.new('Upcoming', '#upcoming', -2)
   Started = MilestoneStruct.new('Started', '#started', -3)
 
diff --git a/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml b/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml
deleted file mode 100644
index d50c59bf607..00000000000
--- a/changelogs/unreleased/4221-board-milestone-should-persist-any-none-properly.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: For milestone filters, treat Any as No Filter (using null). Use -1 for No Milestone
-merge_request:
-author:
-type: changed
diff --git a/db/migrate/20190703001120_default_milestone_to_nil.rb b/db/migrate/20190703001120_default_milestone_to_nil.rb
deleted file mode 100644
index 6a1c3603d9d..00000000000
--- a/db/migrate/20190703001120_default_milestone_to_nil.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-# frozen_string_literal: true
-
-class DefaultMilestoneToNil < ActiveRecord::Migration[5.1]
-  DOWNTIME = false
-
-  def up
-    execute(update_board_milestones_query)
-  end
-
-  def down
-    # no-op
-  end
-
-  private
-
-  # Only 105 records to update, as of 2019/07/18
-  def update_board_milestones_query
-    <<~HEREDOC
-   UPDATE boards
-     SET milestone_id = NULL
-   WHERE boards.milestone_id = -1
-    HEREDOC
-  end
-end
diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb
index bcde730c40b..879ff01f294 100644
--- a/spec/finders/issues_finder_spec.rb
+++ b/spec/finders/issues_finder_spec.rb
@@ -113,13 +113,13 @@ describe IssuesFinder do
         let(:params) { { milestone_title: 'Any' } }
 
         it 'returns issues with any assigned milestone' do
-          expect(issues).to contain_exactly(issue1, issue2, issue3, issue4)
+          expect(issues).to contain_exactly(issue1)
         end
 
         it 'returns issues with any assigned milestone (deprecated)' do
           params[:milestone_title] = Milestone::Any.title
 
-          expect(issues).to contain_exactly(issue1, issue2, issue3, issue4)
+          expect(issues).to contain_exactly(issue1)
         end
       end
 
diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb
index f11d8259d4a..f7ca6fd1e0a 100644
--- a/spec/requests/api/issues/get_project_issues_spec.rb
+++ b/spec/requests/api/issues/get_project_issues_spec.rb
@@ -389,7 +389,7 @@ describe API::Issues do
     it 'returns an array of issues with any milestone' do
       get api("#{base_url}/issues", user), params: { milestone: any_milestone_title }
 
-      expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue.id])
+      expect_paginated_array_response([issue.id, closed_issue.id])
     end
 
     context 'without sort params' do