Merge branch 'issue-filter-name-options' into 'master'

Issuable filtering improvements

This improves the filtering of issues and merge requests by creating a single file that encapsulates all the filtering. Previously this was done with a file for issues and a file for merge requests.

Created the ability for the text search to be done alongside other filterables. Previously because this was outside the filterable form, this wasn't possible and would instead do either filter dropdown or text filter - not both. Fixes #4252 

Fixed issue with not being able to filter and sort issues without refreshing the page. Fixes #15269 

See merge request !3699

11 files changed, 203 insertions, 71 deletions

diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee
index 12c143fdf76..a3185f87640 100644
--- a/app/assets/javascripts/dispatcher.js.coffee
+++ b/app/assets/javascripts/dispatcher.js.coffee
@@ -16,7 +16,6 @@ class Dispatcher
     shortcut_handler = null
     switch page
       when 'projects:issues:index'
-        Issues.init()
         Issuable.init()
         shortcut_handler = new ShortcutsNavigation()
       when 'projects:issues:show'
diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee
index afffed63ac5..6504e481102 100644
--- a/app/assets/javascripts/issuable.js.coffee
+++ b/app/assets/javascripts/issuable.js.coffee
@@ -1,7 +1,11 @@
+issuable_created = false
 @Issuable =
   init: ->
-    Issuable.initTemplates()
-    Issuable.initSearch()
+    unless issuable_created
+      issuable_created = true
+      Issuable.initTemplates()
+      Issuable.initSearch()
+      Issuable.initChecks()
 
   initTemplates: ->
     Issuable.labelRow = _.template(
@@ -19,7 +23,16 @@
       .on 'keyup', ->
         clearTimeout(@timer)
         @timer = setTimeout( ->
-          Issuable.filterResults $('#issue_search_form')
+          $search = $('#issue_search')
+          $form = $('.js-filter-form')
+          $input = $("input[name='#{$search.attr('name')}']", $form)
+
+          if $input.length is 0
+            $form.append "<input type='hidden' name='#{$search.attr('name')}' value='#{_.escape($search.val())}'/>"
+          else
+            $input.val $search.val()
+
+          Issuable.filterResults $form
         , 500)
 
   toggleLabelFilters: ->
@@ -59,15 +72,22 @@
       dataType: "json"
 
   reload: ->
-    if Issues.created
-      Issues.initChecks()
+    if Issuable.created
+      Issuable.initChecks()
 
     $('#filter_issue_search').val($('#issue_search').val())
 
+  initChecks: ->
+    $('.check_all_issues').on 'click', ->
+      $('.selected_issue').prop('checked', @checked)
+      Issuable.checkChanged()
+
+    $('.selected_issue').on 'change', Issuable.checkChanged
+
   updateStateFilters: ->
-    stateFilters =  $('.issues-state-filters')
+    stateFilters =  $('.issues-state-filters, .dropdown-menu-sort')
     newParams = {}
-    paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search']
+    paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search', 'issue_search']
 
     for paramKey in paramKeys
       newParams[paramKey] = gl.utils.getParameterValues(paramKey)[0] or ''
@@ -82,3 +102,17 @@
         else
           newUrl = gl.utils.mergeUrlParams(newParams, initialUrl)
         $(this).attr 'href', newUrl
+
+  checkChanged: ->
+    checked_issues = $('.selected_issue:checked')
+    if checked_issues.length > 0
+      ids = $.map checked_issues, (value) ->
+        $(value).data('id')
+
+      $('#update_issues_ids').val ids
+      $('.issues-other-filters').hide()
+      $('.issues_bulk_update').show()
+    else
+      $('#update_issues_ids').val []
+      $('.issues_bulk_update').hide()
+      $('.issues-other-filters').show()
diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee
deleted file mode 100644
index 3330e6c68ad..00000000000
--- a/app/assets/javascripts/issues.js.coffee
+++ /dev/null
@@ -1,38 +0,0 @@
-@Issues =
-  init: ->
-    Issues.created = true
-    Issues.initChecks()
-
-    $("body").on "ajax:success", ".close_issue, .reopen_issue", ->
-      t = $(this)
-      totalIssues = undefined
-      reopen = t.hasClass("reopen_issue")
-      $(".issue_counter").each ->
-        issue = $(this)
-        totalIssues = parseInt($(this).html(), 10)
-        if reopen and issue.closest(".main_menu").length
-          $(this).html totalIssues + 1
-        else
-          $(this).html totalIssues - 1
-
-  initChecks: ->
-    $(".check_all_issues").click ->
-      $(".selected_issue").prop("checked", @checked)
-      Issues.checkChanged()
-
-    $(".selected_issue").bind "change", Issues.checkChanged
-
-  checkChanged: ->
-    checked_issues = $(".selected_issue:checked")
-    if checked_issues.length > 0
-      ids = []
-      $.each checked_issues, (index, value) ->
-        ids.push $(value).attr("data-id")
-
-      $("#update_issues_ids").val ids
-      $(".issues-other-filters").hide()
-      $(".issues_bulk_update").show()
-    else
-      $("#update_issues_ids").val []
-      $(".issues_bulk_update").hide()
-      $(".issues-other-filters").show()
diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee
index 6a00932c028..e8085e1c2e4 100644
--- a/app/assets/javascripts/lib/url_utility.js.coffee
+++ b/app/assets/javascripts/lib/url_utility.js.coffee
@@ -26,10 +26,19 @@
     newUrl = decodeURIComponent(url)
     for paramName, paramValue of params
       pattern = new RegExp "\\b(#{paramName}=).*?(&|$)"
-      if url.search(pattern) >= 0
+      if not paramValue?
+        newUrl = newUrl.replace pattern, ''
+      else if url.search(pattern) isnt -1
         newUrl = newUrl.replace pattern, "$1#{paramValue}$2"
       else
         newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}"
+
+    # Remove a trailing ampersand
+    lastChar = newUrl[newUrl.length - 1]
+
+    if lastChar is '&'
+        newUrl = newUrl.slice 0, -1
+
     newUrl
 
   # removes parameter query string from url. returns the modified url
diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss
index cb4ddac179e..2ae6c61d524 100644
--- a/app/assets/stylesheets/framework/nav.scss
+++ b/app/assets/stylesheets/framework/nav.scss
@@ -119,7 +119,7 @@
     }
 
     input {
-      height: 34px;
+      height: 35px;
       display: inline-block;
       position: relative;
       top: 2px;
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index fc9db97132d..59f72cd8b69 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -40,11 +40,6 @@
   }
 }
 
-.issue-search-form {
-  margin: 0;
-  height: 24px;
-}
-
 form.edit-issue {
   margin: 0;
 }
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index e6e2546b92f..439b015b3b8 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -262,6 +262,8 @@ module ApplicationHelper
       assignee_id: params[:assignee_id],
       author_id: params[:author_id],
       sort: params[:sort],
+      issue_search: params[:issue_search],
+      label_name: params[:label_name]
     }
 
     options = exist_opts.merge(options)
@@ -272,16 +274,11 @@ module ApplicationHelper
       end
     end
 
-    path = request.path
-    path << "?#{options.to_param}"
-    if add_label
-      if params[:label_name].present? and params[:label_name].respond_to?('any?')
-        params[:label_name].each do |label|
-          path << "&label_name[]=#{label}"
-        end
-      end
-    end
-    path
+    params = options.compact
+
+    params.delete(:label_name) unless add_label
+
+    "#{request.path}?#{params.to_param}"
   end
 
   def outdated_browser?
diff --git a/app/views/shared/_sort_dropdown.html.haml b/app/views/shared/_sort_dropdown.html.haml
index d327bd0a96f..1e0f075b303 100644
--- a/app/views/shared/_sort_dropdown.html.haml
+++ b/app/views/shared/_sort_dropdown.html.haml
@@ -6,7 +6,7 @@
     - else
       = sort_title_recently_created
     %b.caret
-  %ul.dropdown-menu.dropdown-menu-align-right
+  %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-sort
     %li
       = link_to page_filter_path(sort: sort_value_recently_created) do
         = sort_title_recently_created
diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml
index 9474462cbd1..323d563cd4a 100644
--- a/app/views/shared/issuable/_filter.html.haml
+++ b/app/views/shared/issuable/_filter.html.haml
@@ -1,6 +1,8 @@
 .issues-filters
   .issues-details-filters.row-content-block.second-block
-    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name]), method: :get, class: 'filter-form' do
+    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :issue_search]), method: :get, class: 'filter-form js-filter-form' do
+      - if params[:issue_search].present?
+        = hidden_field_tag :issue_search, params[:issue_search]
       - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project)
         .check-all-holder
           = check_box_tag "check_all_issues", nil, false,
diff --git a/app/views/shared/issuable/_search_form.html.haml b/app/views/shared/issuable/_search_form.html.haml
index afad48499b7..186963b32b8 100644
--- a/app/views/shared/issuable/_search_form.html.haml
+++ b/app/views/shared/issuable/_search_form.html.haml
@@ -1,8 +1,2 @@
 = form_tag(path, method: :get, id: "issue_search_form", class: 'issue-search-form') do
   = search_field_tag :issue_search, params[:issue_search], { placeholder: 'Filter by name ...', class: 'form-control issue_search search-text-input input-short', spellcheck: false }
-  = hidden_field_tag :state, params['state']
-  = hidden_field_tag :scope, params['scope']
-  = hidden_field_tag :assignee_id, params['assignee_id']
-  = hidden_field_tag :author_id, params['author_id']
-  = hidden_field_tag :milestone_id, params['milestone_id']
-  = hidden_field_tag :label_id, params['label_id']
diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb
index 192e3619375..bfbd06a29e2 100644
--- a/spec/features/issues/filter_issues_spec.rb
+++ b/spec/features/issues/filter_issues_spec.rb
@@ -154,4 +154,144 @@ describe 'Filter issues', feature: true do
       end
     end
   end
+
+  describe 'filter issues by text' do
+    before do
+      create(:issue, title: "Bug", project: project)
+
+      bug_label = create(:label, project: project, title: 'bug')
+      milestone = create(:milestone, title: "8", project: project)
+
+      issue = create(:issue,
+        title: "Bug 2",
+        project: project,
+        milestone: milestone,
+        author: user,
+        assignee: user)
+      issue.labels << bug_label
+
+      visit namespace_project_issues_path(project.namespace, project)
+    end
+
+    context 'only text', js: true do
+      it 'should filter issues by searched text' do
+        fill_in 'issue_search', with: 'Bug'
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 2)
+        end
+      end
+
+      it 'should not show any issues' do
+        fill_in 'issue_search', with: 'testing'
+
+        page.within '.issues-list' do
+          expect(page).to_not have_selector('.issue')
+        end
+      end
+    end
+
+    context 'text and dropdown options', js: true do
+      it 'should filter by text and label' do
+        fill_in 'issue_search', with: 'Bug'
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 2)
+        end
+
+        click_button 'Label'
+        page.within '.labels-filter' do
+          click_link 'bug'
+        end
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 1)
+        end
+      end
+
+      it 'should filter by text and milestone' do
+        fill_in 'issue_search', with: 'Bug'
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 2)
+        end
+
+        click_button 'Milestone'
+        page.within '.milestone-filter' do
+          click_link '8'
+        end
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 1)
+        end
+      end
+
+      it 'should filter by text and assignee' do
+        fill_in 'issue_search', with: 'Bug'
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 2)
+        end
+
+        click_button 'Assignee'
+        page.within '.dropdown-menu-assignee' do
+          click_link user.name
+        end
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 1)
+        end
+      end
+
+      it 'should filter by text and author' do
+        fill_in 'issue_search', with: 'Bug'
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 2)
+        end
+
+        click_button 'Author'
+        page.within '.dropdown-menu-author' do
+          click_link user.name
+        end
+
+        page.within '.issues-list' do
+          expect(page).to have_selector('.issue', count: 1)
+        end
+      end
+    end
+  end
+
+  describe 'filter issues and sort', js: true do
+    before do
+      bug_label = create(:label, project: project, title: 'bug')
+      bug_one = create(:issue, title: "Frontend", project: project)
+      bug_two = create(:issue, title: "Bug 2", project: project)
+
+      bug_one.labels << bug_label
+      bug_two.labels << bug_label
+
+      visit namespace_project_issues_path(project.namespace, project)
+    end
+
+    it 'should be able to filter and sort issues' do
+      click_button 'Label'
+      page.within '.labels-filter' do
+        click_link 'bug'
+      end
+
+      page.within '.issues-list' do
+        expect(page).to have_selector('.issue', count: 2)
+      end
+
+      click_button 'Last created'
+      page.within '.dropdown-menu-sort' do
+        click_link 'Oldest created'
+      end
+
+      page.within '.issues-list' do
+        expect(first('.issue')).to have_content('Frontend')
+      end
+    end
+  end
 end