Merge branch 'added-milestone-upcoming' into 'master'

Dropdown bug fixes

- Added back upcoming to milestone filter

![Screen_Shot_2016-03-23_at_16.45.22](/uploads/8c31eff7e0984aad17881f6397ff17cd/Screen_Shot_2016-03-23_at_16.45.22.png)

- Fixed bug that would cause the currently selected value to disappear on filters
- Fixed bug that throw an error when filtering by upcoming when there is only a milestone in the past

Closes #14536

See merge request !3368
Signed-off-by: Rémy Coutable <remy@rymai.me>

CHANGELOG

@@ -9,7 +9,7 @@ v 8.6.2
   - Fix order of steps to prevent PostgreSQL errors when running migration. !3355
   - Fix bold text in issuable sidebar. !3358
   - Fix error with anonymous token in applications settings. !3362
-  - Fix the milestone 'upcoming' filter. !3364
+  - Fix the milestone 'upcoming' filter. !3364 + !3368
   - Fix comments on confidential issues showing up in activity feed to non-members. !3375
   - Fix `NoMethodError` when visiting CI root path at `/ci`. !3377
   - Add a tooltip to new branch button in issue page. !3380

app/assets/javascripts/gl_dropdown.js.coffee

@@ -143,10 +143,10 @@ class GitLabDropdown
         selector = ".dropdown-page-one .dropdown-content a"
 
       @dropdown.on "click", selector, (e) ->
-        self.rowClicked $(@)
+        selected = self.rowClicked $(@)
 
         if self.options.clicked
-          self.options.clicked()
+          self.options.clicked(selected)
 
   toggleLoading: ->
     $('.dropdown-menu', @dropdown).toggleClass LOADING_CLASS
@@ -258,17 +258,19 @@ class GitLabDropdown
 
   rowClicked: (el) ->
     fieldName = @options.fieldName
-    field = @dropdown.parent().find("input[name='#{fieldName}']")
+    selectedIndex = el.parent().index()
+    if @renderedData
+      selectedObject = @renderedData[selectedIndex]
+    value = if @options.id then @options.id(selectedObject, el) else selectedObject.id
+    field = @dropdown.parent().find("input[name='#{fieldName}'][value='#{value}']")
 
     if el.hasClass(ACTIVE_CLASS)
       field.remove()
-    else
-      fieldName = @options.fieldName
-      selectedIndex = el.parent().index()
-      if @renderedData
-        selectedObject = @renderedData[selectedIndex]
-      value = if @options.id then @options.id(selectedObject, el) else selectedObject.id
 
+      # Toggle the dropdown label
+      if @options.toggleLabel
+        $(@el).find(".dropdown-toggle-text").text @options.toggleLabel
+    else
       if !value?
         field.remove()
 
@@ -280,7 +282,7 @@ class GitLabDropdown
         @dropdown.find(".#{ACTIVE_CLASS}").removeClass ACTIVE_CLASS
 
       # Toggle active class for the tick mark
-      el.toggleClass "is-active"
+      el.addClass ACTIVE_CLASS
 
       # Toggle the dropdown label
       if @options.toggleLabel
@@ -289,10 +291,13 @@ class GitLabDropdown
       if value?
         if !field.length
           # Create hidden input for form
-          input = "<input type='hidden' name='#{fieldName}' />"
+          input = "<input type='hidden' name='#{fieldName}' value='#{value}' />"
+          if @options.inputId?
+            input = $(input)
+                      .attr('id', @options.inputId)
           @dropdown.before input
 
-        @dropdown.parent().find("input[name='#{fieldName}']").val value
+      return selectedObject
 
   selectFirstRow: ->
     selector = '.dropdown-content li:first-child a'

app/assets/javascripts/issues.js.coffee

 @Issues =
   init: ->
     Issues.initSearch()
-    Issues.initSelects()
     Issues.initChecks()
 
     $("body").on "ajax:success", ".close_issue, .reopen_issue", ->
@@ -17,18 +16,9 @@
           $(this).html totalIssues - 1
 
   reload: ->
-    Issues.initSelects()
     Issues.initChecks()
     $('#filter_issue_search').val($('#issue_search').val())
 
-  initSelects: ->
-    $("select#update_state_event").select2(width: 'resolve', dropdownAutoWidth: true)
-    $("select#update_assignee_id").select2(width: 'resolve', dropdownAutoWidth: true)
-    $("select#update_milestone_id").select2(width: 'resolve', dropdownAutoWidth: true)
-    $("select#label_name").select2(width: 'resolve', dropdownAutoWidth: true)
-    $("#milestone_id, #assignee_id, #label_name").on "change", ->
-      $(this).closest("form").submit()
-
   initChecks: ->
     $(".check_all_issues").click ->
       $(".selected_issue").prop("checked", @checked)

app/assets/javascripts/labels_select.js.coffee

@@ -144,12 +144,14 @@ class @LabelsSelect
             ''
           else
             label.title
-        clicked: ->
+        clicked: (label) ->
           page = $('body').data 'page'
           isIssueIndex = page is 'projects:issues:index'
           isMRIndex = page is page is 'projects:merge_requests:index'
 
           if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex)
+            selectedLabel = label.title
+
             Issues.filterResults $dropdown.closest('form')
           else if $dropdown.hasClass 'js-filter-submit'
             $dropdown.closest('form').submit()

app/assets/javascripts/milestone_select.js.coffee

@@ -7,6 +7,7 @@ class @MilestoneSelect
       selectedMilestone = $dropdown.data('selected')
       showNo = $dropdown.data('show-no')
       showAny = $dropdown.data('show-any')
+      showUpcoming = $dropdown.data('show-upcoming')
       useId = $dropdown.data('use-id')
       defaultLabel = $dropdown.data('default-label')
 
@@ -15,22 +16,32 @@ class @MilestoneSelect
           $.ajax(
             url: milestonesUrl
           ).done (data) ->
+            extraOptions = []
+            if showAny
+              extraOptions.push(
+                id: 0
+                name: ''
+                title: 'Any Milestone'
+              )
+
             if showNo
-              data.unshift(
-                id: '0'
+              extraOptions.push(
+                id: -1
+                name: 'No Milestone'
                 title: 'No Milestone'
               )
 
-            if showAny
-              data.unshift(
-                isAny: true
-                title: 'Any Milestone'
+            if showUpcoming
+              extraOptions.push(
+                id: -2
+                name: '#upcoming'
+                title: 'Upcoming'
               )
 
-            if data.length > 2
-              data.splice 2, 0, 'divider'
+            if extraOptions.length > 2
+              extraOptions.push 'divider'
 
-            callback(data)
+            callback(extraOptions.concat(data))
         filterable: true
         search:
           fields: ['title']
@@ -45,21 +56,49 @@ class @MilestoneSelect
           milestone.title
         id: (milestone) ->
           if !useId
-            if !milestone.isAny?
-              milestone.title
-            else
-              ''
+            milestone.name
           else
             milestone.id
         isSelected: (milestone) ->
-          milestone.title is selectedMilestone
-        clicked: ->
-          page = $('body').data 'page'
-          isIssueIndex = page is 'projects:issues:index'
-          isMRIndex = page is page is 'projects:merge_requests:index'
+          milestone.name is selectedMilestone
+        hidden: ->
+          $selectbox.hide()
+          $value.show()
+        clicked: (selected) ->
+          if $dropdown.hasClass 'js-filter-bulk-update'
+            return
 
-          if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex)
-            Issues.filterResults $dropdown.closest('form')
-          else if $dropdown.hasClass 'js-filter-submit'
-            $dropdown.closest('form').submit()
+          if $dropdown.hasClass('js-filter-submit')
+            if selected.name?
+              selectedMilestone = selected.name
+            else if selected.title?
+              selectedMilestone = selected.title
+            else
+              selectedMilestone = ''
+            $dropdown.parents('form').submit()
+          else
+            selected = $selectbox
+              .find('input[type="hidden"]')
+              .val()
+            data = {}
+            data[abilityName] = {}
+            data[abilityName].milestone_id = selected
+            $loading
+              .fadeIn()
+            $.ajax(
+              type: 'PUT'
+              url: issueUpdateURL
+              data: data
+            ).done (data) ->
+              $loading.fadeOut()
+              $selectbox.hide()
+              $milestoneLink = $value
+                      .show()
+                      .find('a')
+              if data.milestone?
+                data.milestone.namespace = _this.currentProject.namespace
+                data.milestone.path = _this.currentProject.path
+                $value.html(milestoneLinkTemplate(data.milestone))
+              else
+                $value.html(milestoneLinkNoneTemplate)
       )

app/assets/javascripts/users_select.js.coffee

@@ -62,12 +62,20 @@ class @UsersSelect
             selected.name
           else
             defaultLabel
-        clicked: ->
+
+        inputId: 'issue_assignee_id'
+
+        hidden: (e) ->
+          $selectbox.hide()
+          $value.show()
+
+        clicked: (user) ->
           page = $('body').data 'page'
           isIssueIndex = page is 'projects:issues:index'
           isMRIndex = page is page is 'projects:merge_requests:index'
 
           if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex)
+            selectedId = user.id
             Issues.filterResults $dropdown.closest('form')
           else if $dropdown.hasClass 'js-filter-submit'
             $dropdown.closest('form').submit()

app/controllers/projects/milestones_controller.rb

@@ -25,7 +25,7 @@ class Projects::MilestonesController < Projects::ApplicationController
         @milestones = @milestones.page(params[:page]).per(PER_PAGE)
       end
       format.json do
-        render json: @milestones
+        render json: @milestones.to_json(methods: :name)
       end
     end
   end

app/finders/issuable_finder.rb

@@ -243,7 +243,7 @@ class IssuableFinder
   end
 
   def filter_by_upcoming_milestone?
-    params[:milestone_title] == '#upcoming'
+    params[:milestone_title] == Milestone::Upcoming.name
   end
 
   def by_milestone(items)
@@ -252,7 +252,7 @@ class IssuableFinder
         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.title })
+        items = items.joins(:milestone).where(milestones: { title: upcoming.try(:title) })
       else
         items = items.joins(:milestone).where(milestones: { title: params[:milestone_title] })
 

app/helpers/issuables_helper.rb

@@ -37,6 +37,14 @@ module IssuablesHelper
     end
   end
 
+  def milestone_dropdown_label(milestone_title, default_label = "Milestone")
+    if milestone_title == Milestone::Upcoming.name
+      milestone_title = Milestone::Upcoming.title
+    end
+
+    h(milestone_title.presence || default_label)
+  end
+
   private
 
   def sidebar_gutter_collapsed?

app/models/global_milestone.rb

@@ -14,6 +14,7 @@ class GlobalMilestone
 
   def initialize(title, milestones)
     @title = title
+    @name = title
     @milestones = milestones
   end
 

app/views/shared/issuable/_milestone_dropdown.html.haml

 - if params[:milestone_title]
   = hidden_field_tag(:milestone_title, params[:milestone_title])
-= dropdown_tag(h(params[:milestone_title].presence || "Milestone"), options: { title: "Filter by milestone", toggle_class: 'js-milestone-select js-filter-submit', filter: true, dropdown_class: "dropdown-menu-selectable",
-  placeholder: "Search milestones", footer_content: @project.present?, data: { show_no: true, show_any: true, field_name: "milestone_title", selected: params[:milestone_title], project_id: @project.try(:id), milestones: milestones_filter_dropdown_path, default_label: "Milestone" } }) do
+= dropdown_tag(milestone_dropdown_label(params[:milestone_title]), options: { title: "Filter by milestone", toggle_class: 'js-milestone-select js-filter-submit', filter: true, dropdown_class: "dropdown-menu-selectable",
+  placeholder: "Search milestones", footer_content: @project.present?, data: { show_no: true, show_any: true, show_upcoming: true, field_name: "milestone_title", selected: params[:milestone_title], project_id: @project.try(:id), milestones: milestones_filter_dropdown_path, default_label: "Milestone" } }) do
   - if @project
     %ul.dropdown-footer-list
       - if can? current_user, :admin_milestone, @project

spec/features/issues/filter_by_milestone_spec.rb

@@ -11,7 +11,41 @@ feature 'Issue filtering by Milestone', feature: true do
     visit_issues(project)
     filter_by_milestone(Milestone::None.title)
 
-    expect(page).to have_css('.issue .title', count: 1)
+    expect(page).to have_css('.issue', count: 1)
+  end
+
+  context 'filters by upcoming milestone', js: true 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: 0)
+    end
+
+    it 'should show issues in future' do
+      milestone = create(:milestone, project: project, due_date: Date.tomorrow)
+      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: 1)
+    end
+
+    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)
+
+      visit_issues(project)
+      filter_by_milestone(Milestone::Upcoming.title)
+
+      expect(page).to have_css('.issue', count: 0)
+    end
   end
 
   scenario 'filters by a specific Milestone', js: true do
@@ -21,7 +55,7 @@ feature 'Issue filtering by Milestone', feature: true do
     visit_issues(project)
     filter_by_milestone(milestone.title)
 
-    expect(page).to have_css('.issue .title', count: 1)
+    expect(page).to have_css('.issue', count: 1)
   end
 
   def visit_issues(project)
@@ -30,8 +64,6 @@ feature 'Issue filtering by Milestone', feature: true do
 
   def filter_by_milestone(title)
     find(".js-milestone-select").click
-    sleep 0.5
-    find(".milestone-filter a", text: title).click
-    sleep 1
+    find(".milestone-filter .dropdown-content a", text: title).click
   end
 end

spec/features/merge_requests/filter_by_milestone_spec.rb

@@ -11,7 +11,41 @@ feature 'Merge Request filtering by Milestone', feature: true do
     visit_merge_requests(project)
     filter_by_milestone(Milestone::None.title)
 
-    expect(page).to have_css('.merge-request-title', count: 1)
+    expect(page).to have_css('.merge-request', count: 1)
+  end
+
+  context 'filters by upcoming milestone', js: true 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: 0)
+    end
+
+    it 'should show issues in future' do
+      milestone = create(:milestone, project: project, due_date: Date.tomorrow)
+      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: 1)
+    end
+
+    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)
+
+      visit_merge_requests(project)
+      filter_by_milestone(Milestone::Upcoming.title)
+
+      expect(page).to have_css('.merge-request', count: 0)
+    end
   end
 
   scenario 'filters by a specific Milestone', js: true do
@@ -21,7 +55,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
     visit_merge_requests(project)
     filter_by_milestone(milestone.title)
 
-    expect(page).to have_css('.merge-request-title', count: 1)
+    expect(page).to have_css('.merge-request', count: 1)
   end
 
   def visit_merge_requests(project)