Fix outer join when filtering milestones

CHANGELOG

@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.9.0 (unreleased)
   - Redesign navigation for project pages
   - Use gitlab-shell v3.0.0
+  - Fix issues filter when ordering by milestone
 
 v 8.8.2 (unreleased)
   - Fix Error 500 when accessing application settings due to nil disabled OAuth sign-in sources

app/finders/issuable_finder.rb

@@ -250,12 +250,12 @@ class IssuableFinder
   def by_milestone(items)
     if milestones?
       if filter_by_no_milestone?
-        items = items.where(milestone_id: [-1, nil])
+        items = items.left_joins_milestones.where(milestone_id: [-1, nil])
       elsif filter_by_upcoming_milestone?
         upcoming_ids = Milestone.upcoming_ids_by_projects(projects)
         items = items.left_joins_milestones.where(milestone_id: upcoming_ids)
       else
-        items = items.left_joins_milestones.where(milestones: { title: params[:milestone_title] })
+        items = items.with_milestone(params[:milestone_title])
 
         if projects
           items = items.where(milestones: { project_id: projects })

app/models/concerns/issuable.rb

@@ -31,6 +31,7 @@ module Issuable
     scope :unassigned, -> { where("assignee_id IS NULL") }
     scope :of_projects, ->(ids) { where(project_id: ids) }
     scope :of_milestones, ->(ids) { where(milestone_id: ids) }
+    scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) }
     scope :opened, -> { with_state(:opened, :reopened) }
     scope :only_opened, -> { with_state(:opened) }
     scope :only_reopened, -> { with_state(:reopened) }
@@ -45,6 +46,7 @@ module Issuable
     scope :references_project, -> { references(:project) }
     scope :non_archived, -> { join_project.where(projects: { archived: false }) }
 
+
     delegate :name,
              :email,
              to: :author,

spec/features/issues/filter_by_milestone_spec.rb

@@ -15,14 +15,14 @@ feature 'Issue filtering by Milestone', feature: true do
   end
 
   context 'filters by upcoming milestone', js: true do
-    it 'should show issues with no expiry' do
+    it 'should not show issues with no expiry' do
       create(:issue, project: project)
       create(:issue, project: project, milestone: milestone)
 
       visit_issues(project)
       filter_by_milestone(Milestone::Upcoming.title)
 
-      expect(page).to have_css('.issue', count: 2)
+      expect(page).to have_css('.issue', count: 0)
     end
 
     it 'should show issues in future' do
@@ -36,7 +36,7 @@ feature 'Issue filtering by Milestone', feature: true do
       expect(page).to have_css('.issue', count: 1)
     end
 
-    it 'should show issues in past' do
+    it 'should not show issues in past' do
       milestone = create(:milestone, project: project, due_date: Date.yesterday)
       create(:issue, project: project)
       create(:issue, project: project, milestone: milestone)
@@ -44,7 +44,7 @@ feature 'Issue filtering by Milestone', feature: true do
       visit_issues(project)
       filter_by_milestone(Milestone::Upcoming.title)
 
-      expect(page).to have_css('.issue', count: 2)
+      expect(page).to have_css('.issue', count: 0)
     end
   end
 

spec/features/issues_spec.rb

@@ -185,12 +185,14 @@ describe 'Issues', feature: true do
       visit namespace_project_issues_path(project.namespace, project, sort: sort_value_recently_created)
 
       expect(first_issue).to include('baz')
+      expect(last_issue).to include('foo')
     end
 
     it 'sorts by oldest' do
       visit namespace_project_issues_path(project.namespace, project, sort: sort_value_oldest_created)
 
       expect(first_issue).to include('foo')
+      expect(last_issue).to include('baz')
     end
 
     it 'sorts by most recently updated' do

spec/features/merge_requests/filter_by_milestone_spec.rb

@@ -21,14 +21,14 @@ feature 'Merge Request filtering by Milestone', feature: true do
   end
 
   context 'filters by upcoming milestone', js: true do
-    it 'should show issues with no expiry' do
+    it 'should not show issues with no expiry' do
       create(:merge_request, :with_diffs, source_project: project)
       create(:merge_request, :simple, source_project: project, milestone: milestone)
 
       visit_merge_requests(project)
       filter_by_milestone(Milestone::Upcoming.title)
 
-      expect(page).to have_css('.merge-request', count: 2)
+      expect(page).to have_css('.merge-request', count: 0)
     end
 
     it 'should show issues in future' do
@@ -42,7 +42,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
       expect(page).to have_css('.merge-request', count: 1)
     end
 
-    it 'should show issues in past' do
+    it 'should not show issues in past' do
       milestone = create(:milestone, project: project, due_date: Date.yesterday)
       create(:merge_request, :with_diffs, source_project: project)
       create(:merge_request, :simple, source_project: project, milestone: milestone)
@@ -50,7 +50,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
       visit_merge_requests(project)
       filter_by_milestone(Milestone::Upcoming.title)
 
-      expect(page).to have_css('.merge-request', count: 2)
+      expect(page).to have_css('.merge-request', count: 0)
     end
   end