From d6c8eefb5d0298f0c733ac4880e1e64f2a37b24c Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Thu, 26 Mar 2015 19:13:49 -0700
Subject: [PATCH] Big refactoring of issues filters

* Squash project users selectbox and users selectbox into one class
* Move from API autocomplete to GitLab internal one
* Smarter filter for project/group/all issues
* Use selectbox with searchbox for assignee/author/milestone/label
* Switch to ajax filter for issue author/assignee
---
 app/assets/javascripts/api.js.coffee          |  67 ---------
 app/assets/javascripts/dispatcher.js.coffee   |   2 +-
 .../project_users_select.js.coffee            |  56 -------
 app/assets/stylesheets/generic/selects.scss   |   5 +
 app/assets/stylesheets/pages/issuable.scss    |   8 +
 app/assets/stylesheets/pages/issues.scss      |   5 +-
 app/helpers/application_helper.rb             |  10 +-
 app/helpers/labels_helper.rb                  |   5 +
 app/helpers/milestones_helper.rb              |  12 ++
 app/helpers/selects_helper.rb                 |  29 ++--
 app/views/projects/_issuable_form.html.haml   |   2 +-
 .../projects/issues/_issue_context.html.haml  |   4 +-
 app/views/projects/issues/index.html.haml     |   2 +-
 app/views/projects/issues/update.js.haml      |   2 +-
 .../merge_requests/_new_submit.html.haml      |   2 +-
 .../merge_requests/show/_context.html.haml    |   2 +-
 .../projects/merge_requests/update.js.haml    |   2 +-
 .../_new_project_member.html.haml             |   2 +-
 app/views/shared/_issuable_filter.html.haml   | 141 ++++++------------
 19 files changed, 114 insertions(+), 244 deletions(-)
 delete mode 100644 app/assets/javascripts/project_users_select.js.coffee

diff --git a/app/assets/javascripts/api.js.coffee b/app/assets/javascripts/api.js.coffee
index 27d04e7cac6..9e5d594c861 100644
--- a/app/assets/javascripts/api.js.coffee
+++ b/app/assets/javascripts/api.js.coffee
@@ -1,57 +1,7 @@
 @Api =
   groups_path: "/api/:version/groups.json"
   group_path: "/api/:version/groups/:id.json"
-  users_path: "/api/:version/users.json"
-  user_path: "/api/:version/users/:id.json"
-  notes_path: "/api/:version/projects/:id/notes.json"
   namespaces_path: "/api/:version/namespaces.json"
-  project_users_path: "/api/:version/projects/:id/users.json"
-
-  # Get 20 (depends on api) recent notes
-  # and sort the ascending from oldest to newest
-  notes: (project_id, callback) ->
-    url = Api.buildUrl(Api.notes_path)
-    url = url.replace(':id', project_id)
-
-    $.ajax(
-      url: url,
-      data:
-        private_token: gon.api_token
-        gfm: true
-        recent: true
-      dataType: "json"
-    ).done (notes) ->
-      notes.sort (a, b) ->
-        return a.id - b.id
-      callback(notes)
-
-  user: (user_id, callback) ->
-    url = Api.buildUrl(Api.user_path)
-    url = url.replace(':id', user_id)
-
-    $.ajax(
-      url: url
-      data:
-        private_token: gon.api_token
-      dataType: "json"
-    ).done (user) ->
-      callback(user)
-
-  # Return users list. Filtered by query
-  # Only active users retrieved
-  users: (query, callback) ->
-    url = Api.buildUrl(Api.users_path)
-
-    $.ajax(
-      url: url
-      data:
-        private_token: gon.api_token
-        search: query
-        per_page: 20
-        active: true
-      dataType: "json"
-    ).done (users) ->
-      callback(users)
 
   group: (group_id, callback) ->
     url = Api.buildUrl(Api.group_path)
@@ -80,23 +30,6 @@
     ).done (groups) ->
       callback(groups)
 
-  # Return project users list. Filtered by query
-  # Only active users retrieved
-  projectUsers: (project_id, query, callback) ->
-    url = Api.buildUrl(Api.project_users_path)
-    url = url.replace(':id', project_id)
-
-    $.ajax(
-      url: url
-      data:
-        private_token: gon.api_token
-        search: query
-        per_page: 20
-        active: true
-      dataType: "json"
-    ).done (users) ->
-      callback(users)
-
   # Return namespaces list. Filtered by query
   namespaces: (query, callback) ->
     url = Api.buildUrl(Api.namespaces_path)
diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee
index 3535d8c2cfc..821712f7512 100644
--- a/app/assets/javascripts/dispatcher.js.coffee
+++ b/app/assets/javascripts/dispatcher.js.coffee
@@ -127,7 +127,7 @@ class Dispatcher
           when 'show'
             new ProjectShow()
           when 'issues', 'merge_requests'
-            new ProjectUsersSelect()
+            new UsersSelect()
           when 'wikis'
             new Wikis()
             shortcut_handler = new ShortcutsNavigation()
diff --git a/app/assets/javascripts/project_users_select.js.coffee b/app/assets/javascripts/project_users_select.js.coffee
deleted file mode 100644
index 80ab1a61ab9..00000000000
--- a/app/assets/javascripts/project_users_select.js.coffee
+++ /dev/null
@@ -1,56 +0,0 @@
-class @ProjectUsersSelect
-  constructor: ->
-    $('.ajax-project-users-select').each (i, select) =>
-      project_id = $(select).data('project-id') || $('body').data('project-id')
-
-      $(select).select2
-        placeholder: $(select).data('placeholder') || "Search for a user"
-        multiple: $(select).hasClass('multiselect')
-        minimumInputLength: 0
-        query: (query) ->
-          Api.projectUsers project_id, query.term, (users) ->
-            data = { results: users }
-
-            if query.term.length == 0
-              nullUser = {
-                name: 'Unassigned',
-                avatar: null,
-                username: 'none',
-                id: -1
-              }
-
-              data.results.unshift(nullUser)
-
-            query.callback(data)
-
-        initSelection: (element, callback) ->
-          id = $(element).val()
-          if id != "" && id != "-1"
-            Api.user(id, callback)
-
-
-        formatResult: (args...) =>
-          @formatResult(args...)
-        formatSelection: (args...) =>
-          @formatSelection(args...)
-        dropdownCssClass: "ajax-project-users-dropdown"
-        dropdownAutoWidth: true
-        escapeMarkup: (m) -> # we do not want to escape markup since we are displaying html in results
-          m
-
-  formatResult: (user) ->
-    if user.avatar_url
-      avatar = user.avatar_url
-    else
-      avatar = gon.default_avatar_url
-
-    avatarMarkup = "<div class='user-image'><img class='avatar s24' src='#{avatar}'></div>"
-
-    "<div class='user-result'>
-       #{avatarMarkup}
-       <div class='user-name'>#{user.name}</div>
-       <div class='user-username'>#{user.username}</div>
-     </div>"
-
-  formatSelection: (user) ->
-    user.name
diff --git a/app/assets/stylesheets/generic/selects.scss b/app/assets/stylesheets/generic/selects.scss
index 7557f411111..69613608c82 100644
--- a/app/assets/stylesheets/generic/selects.scss
+++ b/app/assets/stylesheets/generic/selects.scss
@@ -28,6 +28,7 @@
 .select2-drop-active {
   border: 1px solid #BBB !important;
   margin-top: 4px;
+  font-size: 13px;
 
   &.select2-drop-above {
     margin-bottom: 8px;
@@ -106,3 +107,7 @@
     font-weight: bolder;
   }
 }
+
+.ajax-users-dropdown {
+  min-width: 225px !important;
+}
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index a640a4e2051..13e09d5596f 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -45,3 +45,11 @@
 
   .btn { font-size: 13px; }
 }
+
+.filter-item {
+  margin-right: 15px;
+
+  > span {
+    margin-right: 4px;
+  }
+}
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 6c1dd4f7e9f..55e648a568f 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -60,6 +60,7 @@
 }
 
 @media (min-width: 800px)  {
+  .issues-filters,
   .issues_bulk_update {
     select, .select2-container {
       width: 120px !important;
@@ -69,14 +70,16 @@
 }
 
 @media (min-width: 1200px) {
+  .issues-filters,
   .issues_bulk_update {
     select, .select2-container {
-      width: 160px !important;
+      width: 140px !important;
       display: inline-block;
     }
   }
 }
 
+.issues-filters,
 .issues_bulk_update {
   .select2-container .select2-choice {
     color: #444 !important;
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 38b5fc4a011..3f3509bb18a 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -275,7 +275,9 @@ module ApplicationHelper
     'https://' + promo_host
   end
 
-  def page_filter_path(options={})
+  def page_filter_path(options = {})
+    without = options.delete(:without)
+
     exist_opts = {
       state: params[:state],
       scope: params[:scope],
@@ -288,6 +290,12 @@ module ApplicationHelper
 
     options = exist_opts.merge(options)
 
+    if without.present?
+      without.each do |key|
+        options.delete(key)
+      end
+    end
+
     path = request.path
     path << "?#{options.to_param}"
     path
diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 49063491abf..aa98ead43f1 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -47,4 +47,9 @@ module LabelsHelper
       "#FFF"
     end
   end
+
+  def project_labels_options(project)
+    options_for_select([['Any', nil]]) +
+      options_from_collection_for_select(project.labels, 'name', 'name', params[:label_name])
+  end
 end
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 59fdc0d49cc..e1dec3ec628 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -19,4 +19,16 @@ module MilestonesHelper
       content_tag :div, nil, options
     end
   end
+
+  def projects_milestones_options
+    milestones =
+      if @project
+        @project.milestones
+      else
+        Milestone.where(project_id: @projects)
+      end.active
+
+    options_for_select([['Any', nil]]) +
+      options_from_collection_for_select(milestones, 'id', 'title', params[:milestone_id])
+  end
 end
diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb
index 796d805f219..457cd3fa46b 100644
--- a/app/helpers/selects_helper.rb
+++ b/app/helpers/selects_helper.rb
@@ -4,18 +4,27 @@ module SelectsHelper
     css_class << "multiselect " if opts[:multiple]
     css_class << (opts[:class] || '')
     value = opts[:selected] || ''
+    placeholder = opts[:placeholder] || 'Search for a user'
 
-    hidden_field_tag(id, value, class: css_class)
-  end
+    null_user = opts[:null_user] || false
+    any_user = opts[:any_user] || false
 
-  def project_users_select_tag(id, opts = {})
-    css_class = "ajax-project-users-select "
-    css_class << "multiselect " if opts[:multiple]
-    css_class << (opts[:class] || '')
-    value = opts[:selected] || ''
-    placeholder = opts[:placeholder] || 'Select user'
-    project_id = opts[:project_id] || @project.id
-    hidden_field_tag(id, value, class: css_class, 'data-placeholder' => placeholder, 'data-project-id' => project_id)
+    html = {
+      class: css_class,
+      'data-placeholder' => placeholder,
+      'data-null-user' => null_user,
+      'data-any-user' => any_user,
+    }
+
+    unless opts[:scope] == :all
+      if @project
+        html['data-project-id'] = @project.id
+      elsif @group
+        html['data-group-id'] = @group.id
+      end
+    end
+
+    hidden_field_tag(id, value, html)
   end
 
   def groups_select_tag(id, opts = {})
diff --git a/app/views/projects/_issuable_form.html.haml b/app/views/projects/_issuable_form.html.haml
index 7fd5fe8a6e1..0cb1f913cbd 100644
--- a/app/views/projects/_issuable_form.html.haml
+++ b/app/views/projects/_issuable_form.html.haml
@@ -35,7 +35,7 @@
       %i.fa.fa-user
       Assign to
     .col-sm-10
-      = project_users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]",
+      = users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]",
           placeholder: 'Select a user', class: 'custom-form-control',
           selected: issuable.assignee_id)
       &nbsp;
diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml
index c3d6dc2e50b..52e38050419 100644
--- a/app/views/projects/issues/_issue_context.html.haml
+++ b/app/views/projects/issues/_issue_context.html.haml
@@ -8,7 +8,7 @@
       - else
         none
     - if can?(current_user, :modify_issue, @issue)
-      = project_users_select_tag('issue[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: @issue.assignee_id)
+      = users_select_tag('issue[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: @issue.assignee_id, null_user: true)
 
   %div.prepend-top-20.clearfix
     .issuable-context-title
@@ -44,5 +44,3 @@
 
 :coffeescript
   new Subscription("#{toggle_subscription_namespace_project_issue_path(@issue.project.namespace, @project, @issue)}")
-
-
diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml
index 54e3009cca2..210b77a6b15 100644
--- a/app/views/projects/issues/index.html.haml
+++ b/app/views/projects/issues/index.html.haml
@@ -19,7 +19,7 @@
     .issues_bulk_update.hide
       = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), method: :post  do
         = select_tag('update[state_event]', options_for_select([['Open', 'reopen'], ['Closed', 'close']]), prompt: "Status", class: 'form-control')
-        = project_users_select_tag('update[assignee_id]', placeholder: 'Assignee')
+        = users_select_tag('update[assignee_id]', placeholder: 'Assignee', null_user: true)
         = select_tag('update[milestone_id]', bulk_update_milestone_options, prompt: "Milestone")
         = hidden_field_tag 'update[issues_ids]', []
         = hidden_field_tag :state_event, params[:state_event]
diff --git a/app/views/projects/issues/update.js.haml b/app/views/projects/issues/update.js.haml
index 82c0e653759..1d38662bff8 100644
--- a/app/views/projects/issues/update.js.haml
+++ b/app/views/projects/issues/update.js.haml
@@ -13,5 +13,5 @@
 
 $('select.select2').select2({width: 'resolve', dropdownAutoWidth: true})
 $('.edit-issue.inline-update input[type="submit"]').hide();
-new ProjectUsersSelect();
+new UsersSelect()
 new Issue();
diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml
index 1d8eef4e8ce..d986ce67c0c 100644
--- a/app/views/projects/merge_requests/_new_submit.html.haml
+++ b/app/views/projects/merge_requests/_new_submit.html.haml
@@ -39,7 +39,7 @@
           %i.fa.fa-user
           Assign to
       .col-sm-10
-        = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select a user', class: 'custom-form-control', selected: @merge_request.assignee_id, project_id: @merge_request.target_project_id)
+        = users_select_tag('merge_request[assignee_id]', placeholder: 'Select a user', class: 'custom-form-control', selected: @merge_request.assignee_id, project_id: @merge_request.target_project_id)
         &nbsp;
         = link_to 'Assign to me', '#', class: 'btn assign-to-me-link'
     .form-group
diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml
index 80e5c223d60..105562fb05e 100644
--- a/app/views/projects/merge_requests/show/_context.html.haml
+++ b/app/views/projects/merge_requests/show/_context.html.haml
@@ -9,7 +9,7 @@
         none
     .issuable-context-selectbox
       - if can?(current_user, :modify_merge_request, @merge_request)
-        = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: @merge_request.assignee_id)
+        = users_select_tag('merge_request[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: @merge_request.assignee_id, null_user: true)
 
   %div.prepend-top-20.clearfix
     .issuable-context-title
diff --git a/app/views/projects/merge_requests/update.js.haml b/app/views/projects/merge_requests/update.js.haml
index f5cc98c7fa4..b4df1d20737 100644
--- a/app/views/projects/merge_requests/update.js.haml
+++ b/app/views/projects/merge_requests/update.js.haml
@@ -2,7 +2,7 @@
   $('.context').html("#{escape_javascript(render partial: 'projects/merge_requests/show/context', locals: { issue: @issue })}");
   $('.context').effect('highlight');
 
-  new ProjectUsersSelect();
+  new UsersSelect()
 
   $('select.select2').select2({width: 'resolve', dropdownAutoWidth: true});
   merge_request = new MergeRequest();
diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml
index 0f824bdabf8..5daae2708e6 100644
--- a/app/views/projects/project_members/_new_project_member.html.haml
+++ b/app/views/projects/project_members/_new_project_member.html.haml
@@ -1,7 +1,7 @@
 = form_for @project_member, as: :project_member, url: namespace_project_project_members_path(@project.namespace, @project), html: { class: 'form-horizontal users-project-form' } do |f|
   .form-group
     = f.label :user_ids, "People", class: 'control-label'
-    .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large')
+    .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large', scope: :all)
 
   .form-group
     = f.label :access_level, "Project Access", class: 'control-label'
diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/_issuable_filter.html.haml
index 5412b9ef0f4..686a3389bb4 100644
--- a/app/views/shared/_issuable_filter.html.haml
+++ b/app/views/shared/_issuable_filter.html.haml
@@ -15,105 +15,50 @@
           All
 
   %div
-    - if controller.controller_name == 'issues'
-      .check-all-holder
-        = check_box_tag "check_all_issues", nil, false,
-          class: "check_all_issues left",
-          disabled: !can?(current_user, :modify_issue, @project)
-    .issues-other-filters
-      .dropdown.inline.assignee-filter
-        %button.dropdown-toggle.btn{type: 'button', "data-toggle" => "dropdown"}
-          %i.fa.fa-user
-          %span.light assignee:
-          - if @assignee.present?
-            %strong= @assignee.name
-          - elsif params[:assignee_id] == "0"
-            Unassigned
-          - else
-            Any
-          %b.caret
-        %ul.dropdown-menu
-          %li
-            = link_to page_filter_path(assignee_id: nil) do
-              Any
-            = link_to page_filter_path(assignee_id: 0) do
-              Unassigned
-          - @assignees.sort_by(&:name).each do |user|
-            %li
-              = link_to page_filter_path(assignee_id: user.id) do
-                = image_tag avatar_icon(user.email), class: "avatar s16", alt: ''
-                = user.name
+    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_id, :label_name]), method: :get, class: 'filter-form' do
+      - if controller.controller_name == 'issues'
+        .check-all-holder
+          = check_box_tag "check_all_issues", nil, false,
+            class: "check_all_issues left",
+            disabled: !can?(current_user, :modify_issue, @project)
+      .issues-other-filters
+        .filter-item.inline
+          %span.light
+            %i.fa.fa-user
+            Assignee
+          %strong
+            = users_select_tag(:assignee_id, selected: params[:assignee_id],
+              placeholder: 'Any', class: 'trigger-submit', any_user: true, null_user: true)
 
-      .dropdown.inline.prepend-left-10.author-filter
-        %button.dropdown-toggle.btn{type: 'button', "data-toggle" => "dropdown"}
-          %i.fa.fa-user
-          %span.light author:
-          - if @author.present?
-            %strong= @author.name
-          - elsif params[:author_id] == "0"
-            Unassigned
-          - else
-            Any
-          %b.caret
-        %ul.dropdown-menu
-          %li
-            = link_to page_filter_path(author_id: nil) do
-              Any
-            = link_to page_filter_path(author_id: 0) do
-              Unassigned
-          - @authors.sort_by(&:name).each do |user|
-            %li
-              = link_to page_filter_path(author_id: user.id) do
-                = image_tag avatar_icon(user.email), class: "avatar s16", alt: ''
-                = user.name
+        .filter-item.inline
+          %span.light
+            %i.fa.fa-user
+            Author
+          %strong
+            = users_select_tag(:author_id, selected: params[:author_id],
+              placeholder: 'Any', class: 'trigger-submit', any_user: true)
 
-      .dropdown.inline.prepend-left-10.milestone-filter
-        %button.dropdown-toggle.btn{type: 'button', "data-toggle" => "dropdown"}
-          %i.fa.fa-clock-o
-          %span.light milestone:
-          - if @milestone.present?
-            %strong= @milestone.title
-          - elsif params[:milestone_id] == "0"
-            None (backlog)
-          - else
-            Any
-          %b.caret
-        %ul.dropdown-menu
-          %li
-            = link_to page_filter_path(milestone_id: nil) do
-              Any
-            = link_to page_filter_path(milestone_id: 0) do
-              None (backlog)
-          - @milestones.each do |milestone|
-            %li
-              = link_to page_filter_path(milestone_id: milestone.id) do
-                %strong= milestone.title
-                %small.light= milestone.expires_at
+        .filter-item.inline
+          %span.light
+            %i.fa.fa-clock-o
+            Milestone
+          %strong
+            = select_tag('milestone_id', projects_milestones_options, class: "select2 trigger-submit")
 
-      - if @project
-        .dropdown.inline.prepend-left-10.labels-filter
-          %button.dropdown-toggle.btn{type: 'button', "data-toggle" => "dropdown"}
-            %i.fa.fa-tags
-            %span.light label:
-            - if params[:label_name].present?
-              %strong= params[:label_name]
-            - else
-              Any
-            %b.caret
-          %ul.dropdown-menu
-            %li
-              = link_to page_filter_path(label_name: nil) do
-                Any
-            - if @project.labels.any?
-              - @project.labels.each do |label|
-                %li
-                  = link_to page_filter_path(label_name: label.name) do
-                    = render_colored_label(label)
-            - else
-              %li
-                = link_to generate_namespace_project_labels_path(@project.namespace, @project, redirect: request.original_url), method: :post do
-                  %i.fa.fa-plus-circle
-                  Create default labels
+        - if @project
+          .filter-item.inline
+            %span.light
+              %i.fa.fa-tag
+              Label
+            %strong
+              = select_tag('label_name', project_labels_options(@project), class: "select2 trigger-submit")
 
-      .pull-right
-        = render 'shared/sort_dropdown'
+        .pull-right
+          = render 'shared/sort_dropdown'
+
+:coffeescript
+  new UsersSelect()
+
+  $('form.filter-form').on 'submit', (event) ->
+    event.preventDefault()
+    Turbolinks.visit @.action + '&' + $(@).serialize()
-- 
GitLab