Make upcoming milestone work across projects

Before: we took the next milestone due across all projects in the
search and found issues whose milestone title matched that
one. Problems:

1. The milestone could be closed.
2. Different projects have milestones with different schedules.
3. Different projects have milestones with different titles.
4. Different projects can have milestones with different schedules, but
   the _same_ title. That means we could show issues from a past
   milestone, or one that's far in the future.

After: gather the ID of the next milestone on each project we're looking
at, and find issues with those milestone IDs. Problems:

1. For a lot of projects, this can return a lot of IDs.
2. The SQL query has to be different between Postgres and MySQL, because
   MySQL is much more lenient with HAVING: as well as the columns
   appearing in GROUP BY or in aggregate clauses, MySQL allows them to
   appear in the SELECT list (un-aggregated).

CHANGELOG

@@ -15,6 +15,7 @@ v 8.8.0 (unreleased)
   - Make build status canceled if any of the jobs was canceled and none failed
   - Upgrade Sidekiq to 4.1.2
   - Added /health_check endpoint for checking service status
+  - Make 'upcoming' filter for milestones work better across projects
   - Sanitize repo paths in new project error message
   - Bump mail_room to 0.7.0 to fix stuck IDLE connections
   - Remove future dates from contribution calendar graph.

app/finders/issuable_finder.rb

@@ -252,8 +252,8 @@ class IssuableFinder
       if filter_by_no_milestone?
         items = items.where(milestone_id: [-1, nil])
       elsif filter_by_upcoming_milestone?
-        upcoming = Milestone.where(project_id: projects).upcoming
-        items = items.joins(:milestone).where(milestones: { title: upcoming.try(:title) })
+        upcoming_ids = Milestone.upcoming_ids_by_projects(projects)
+        items = items.joins(:milestone).where(milestone_id: upcoming_ids)
       else
         items = items.joins(:milestone).where(milestones: { title: params[:milestone_title] })
 

app/models/milestone.rb

@@ -67,8 +67,18 @@ class Milestone < ActiveRecord::Base
     @link_reference_pattern ||= super("milestones", /(?<milestone>\d+)/)
   end
 
-  def self.upcoming
-    self.where('due_date > ?', Time.now).reorder(due_date: :asc).first
+  def self.upcoming_ids_by_projects(projects)
+    rel = unscoped.of_projects(projects).active.where('due_date > ?', Time.now)
+
+    if Gitlab::Database.postgresql?
+      rel.order(:project_id, :due_date).pluck('DISTINCT ON (project_id) id')
+    else
+      rel.
+        group(:project_id).
+        having('due_date = MIN(due_date)').
+        pluck(:id, :project_id, :due_date).
+        map(&:first)
+    end
   end
 
   def to_reference(from_project = nil)

spec/finders/issues_finder_spec.rb

@@ -66,6 +66,40 @@ describe IssuesFinder do
         end
       end
 
+      context 'filtering by upcoming milestone' do
+        let(:params) { { milestone_title: Milestone::Upcoming.name } }
+
+        let(:project_no_upcoming_milestones) { create(:empty_project, :public) }
+        let(:project_next_1_1) { create(:empty_project, :public) }
+        let(:project_next_8_8) { create(:empty_project, :public) }
+
+        let(:yesterday) { Date.today - 1.day }
+        let(:tomorrow) { Date.today + 1.day }
+        let(:two_days_from_now) { Date.today + 2.days }
+        let(:ten_days_from_now) { Date.today + 10.days }
+
+        let(:milestones) do
+          [
+            create(:milestone, :closed, project: project_no_upcoming_milestones),
+            create(:milestone, project: project_next_1_1, title: '1.1', due_date: two_days_from_now),
+            create(:milestone, project: project_next_1_1, title: '8.8', due_date: ten_days_from_now),
+            create(:milestone, project: project_next_8_8, title: '1.1', due_date: yesterday),
+            create(:milestone, project: project_next_8_8, title: '8.8', due_date: tomorrow)
+          ]
+        end
+
+        before do
+          milestones.each do |milestone|
+            create(:issue, project: milestone.project, milestone: milestone, author: user, assignee: user)
+          end
+        end
+
+        it 'returns issues in the upcoming milestone for each project' do
+          expect(issues.map { |issue| issue.milestone.title }).to contain_exactly('1.1', '8.8')
+          expect(issues.map { |issue| issue.milestone.due_date }).to contain_exactly(tomorrow, two_days_from_now)
+        end
+      end
+
       context 'filtering by label' do
         let(:params) { { label_name: label.title } }
 

spec/models/milestone_spec.rb

@@ -204,4 +204,35 @@ describe Milestone, models: true do
         to eq([milestone])
     end
   end
+
+  describe '.upcoming_ids_by_projects' do
+    let(:project_1) { create(:empty_project) }
+    let(:project_2) { create(:empty_project) }
+    let(:project_3) { create(:empty_project) }
+    let(:projects) { [project_1, project_2, project_3] }
+
+    let!(:past_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now - 1.day) }
+    let!(:current_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 1.day) }
+    let!(:future_milestone_project_1) { create(:milestone, project: project_1, due_date: Time.now + 2.days) }
+
+    let!(:past_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now - 1.day) }
+    let!(:closed_milestone_project_2) { create(:milestone, :closed, project: project_2, due_date: Time.now + 1.day) }
+    let!(:current_milestone_project_2) { create(:milestone, project: project_2, due_date: Time.now + 2.days) }
+
+    let!(:past_milestone_project_3) { create(:milestone, project: project_3, due_date: Time.now - 1.day) }
+
+    let(:milestone_ids) { Milestone.upcoming_ids_by_projects(projects) }
+
+    it 'returns the next upcoming open milestone ID for each project' do
+      expect(milestone_ids).to contain_exactly(current_milestone_project_1.id, current_milestone_project_2.id)
+    end
+
+    context 'when the projects have no open upcoming milestones' do
+      let(:projects) { [project_3] }
+
+      it 'returns no results' do
+        expect(milestone_ids).to be_empty
+      end
+    end
+  end
 end