Merge pull request #9234 from dsander/group-milestones-by-title

Group milestones by title in the dashboard and all other issue views

7 files changed, 11 insertions, 33 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 08a75eb8b36..75e7cfe2566 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,7 @@ v 7.11.0 (unreleased)
   - Don't crash when an MR from a fork has a cross-reference comment from the target project on of its commits.
   - Include commit comments in MR from a forked project.
   - Fix adding new group members from admin area
+  - Group milestones by title in the dashboard and all other issue views.
   - Add default project and snippet visibility settings to the admin web UI.
   - Show incompatible projects in Google Code import status (Stan Hu)
   - Fix bug where commit data would not appear in some subdirectories (Stan Hu)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index c9b34eac4b0..eee10d6c22a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -287,40 +287,15 @@ class ApplicationController < ActionController::Base
     @filter_params
   end
 
-  def set_filter_values(collection)
-    assignee_id = @filter_params[:assignee_id]
-    author_id = @filter_params[:author_id]
-    milestone_id = @filter_params[:milestone_id]
-
-    @sort = @filter_params[:sort]
-    @assignees = User.where(id: collection.pluck(:assignee_id))
-    @authors = User.where(id: collection.pluck(:author_id))
-    @milestones = Milestone.where(id: collection.pluck(:milestone_id))
-
-    if assignee_id.present? && !assignee_id.to_i.zero?
-      @assignee = @assignees.find_by(id: assignee_id)
-    end
-
-    if author_id.present? && !author_id.to_i.zero?
-      @author = @authors.find_by(id: author_id)
-    end
-
-    if milestone_id.present? && !milestone_id.to_i.zero?
-      @milestone = @milestones.find_by(id: milestone_id)
-    end
-  end
-
   def get_issues_collection
     set_filters_params
     issues = IssuesFinder.new.execute(current_user, @filter_params)
-    set_filter_values(issues)
     issues
   end
 
   def get_merge_requests_collection
     set_filters_params
     merge_requests = MergeRequestsFinder.new.execute(current_user, @filter_params)
-    set_filter_values(merge_requests)
     merge_requests
   end
 
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 2c0702073d4..b8f367c6339 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -113,8 +113,9 @@ class IssuableFinder
   end
 
   def by_milestone(items)
-    if params[:milestone_id].present?
-      items = items.where(milestone_id: (params[:milestone_id] == NONE ? nil : params[:milestone_id]))
+    if params[:milestone_title].present?
+      milestone_ids = (params[:milestone_title] == NONE ? nil : Milestone.where(title: params[:milestone_title]).pluck(:id))
+      items = items.where(milestone_id: milestone_ids)
     end
 
     items
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 282bdf744d2..93e33ebefd8 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -28,6 +28,7 @@ module MilestonesHelper
         Milestone.where(project_id: @projects)
       end.active
 
-    options_from_collection_for_select(milestones, 'id', 'title', params[:milestone_id])
+    grouped_milestones = Milestones::GroupService.new(milestones).execute
+    options_from_collection_for_select(grouped_milestones, 'title', 'title', params[:milestone_title])
   end
 end
diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/_issuable_filter.html.haml
index f9eb2dcfa28..fa8b4eae314 100644
--- a/app/views/shared/_issuable_filter.html.haml
+++ b/app/views/shared/_issuable_filter.html.haml
@@ -15,7 +15,7 @@
           #{state_filters_text_for(:all, @project)}
 
   .issues-details-filters
-    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_id, :label_name]), method: :get, class: 'filter-form' do
+    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name]), method: :get, class: 'filter-form' do
       - if controller.controller_name == 'issues'
         .check-all-holder
           = check_box_tag "check_all_issues", nil, false,
@@ -31,7 +31,7 @@
             placeholder: 'Author', class: 'trigger-submit', any_user: true, first_user: true)
 
         .filter-item.inline.milestone-filter
-          = select_tag('milestone_id', projects_milestones_options, class: "select2 trigger-submit", prompt: 'Milestone')
+          = select_tag('milestone_title', projects_milestones_options, class: "select2 trigger-submit", prompt: 'Milestone')
 
         - if @project
           .filter-item.inline.labels-filter
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index e217f0739d2..66d73b2505c 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -95,7 +95,7 @@ describe 'Issues', feature: true do
     let(:issue) { @issue }
 
     it 'should allow filtering by issues with no specified milestone' do
-      visit namespace_project_issues_path(project.namespace, project, milestone_id: IssuableFinder::NONE)
+      visit namespace_project_issues_path(project.namespace, project, milestone_title: IssuableFinder::NONE)
 
       expect(page).not_to have_content 'foobar'
       expect(page).to have_content 'barbaz'
@@ -103,7 +103,7 @@ describe 'Issues', feature: true do
     end
 
     it 'should allow filtering by a specified milestone' do
-      visit namespace_project_issues_path(project.namespace, project, milestone_id: issue.milestone.id)
+      visit namespace_project_issues_path(project.namespace, project, milestone_title: issue.milestone.title)
 
       expect(page).to have_content 'foobar'
       expect(page).not_to have_content 'barbaz'
diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb
index 479fa950387..69bac387d20 100644
--- a/spec/finders/issues_finder_spec.rb
+++ b/spec/finders/issues_finder_spec.rb
@@ -43,7 +43,7 @@ describe IssuesFinder do
       end
 
       it 'should filter by milestone id' do
-        params = { scope: "all", milestone_id: milestone.id, state: 'opened' }
+        params = { scope: "all", milestone_title: milestone.title, state: 'opened' }
         issues = IssuesFinder.new.execute(user, params)
         expect(issues).to eq([issue1])
       end