Merge branch...

Merge branch 'ee-50626-searching-users-by-the-admin-panel-wipes-query-when-using-sort' into 'master'

Fix losing search query when sort users on admin users page

See merge request gitlab-org/gitlab-ee!8640

app/helpers/sorting_helper.rb

@@ -122,6 +122,18 @@ module SortingHelper
     }
   end
 
+  def users_sort_options_hash
+    {
+      sort_value_name => sort_title_name,
+      sort_value_recently_signin => sort_title_recently_signin,
+      sort_value_oldest_signin => sort_title_oldest_signin,
+      sort_value_recently_created => sort_title_recently_created,
+      sort_value_oldest_created => sort_title_oldest_created,
+      sort_value_recently_updated => sort_title_recently_updated,
+      sort_value_oldest_updated => sort_title_oldest_updated
+    }
+  end
+
   def sortable_item(item, path, sorted_by)
     link_to item, path, class: sorted_by == item ? 'is-active' : ''
   end

app/views/admin/users/index.html.haml

@@ -9,28 +9,20 @@
       .search-holder
         .search-field-holder
           = search_field_tag :search_query, params[:search_query], placeholder: 'Search by name, email or username', class: 'form-control search-text-input js-search-input', spellcheck: false
+          - if @sort.present?
+            = hidden_field_tag :sort, @sort
           = icon("search", class: "search-icon")
-        .dropdown
-          - toggle_text = if @sort.present? then sort_options_hash[@sort] else sort_title_name end
+          = button_tag 'Search users' if Rails.env.test?
+        .dropdown.user-sort-dropdown
+          - toggle_text = if @sort.present? then users_sort_options_hash[@sort] else sort_title_name end
           = dropdown_toggle(toggle_text, { toggle: 'dropdown' })
           %ul.dropdown-menu.dropdown-menu-right
             %li.dropdown-header
               Sort by
             %li
-              = link_to admin_users_path(sort: sort_value_name, filter: params[:filter]) do
-                = sort_title_name
-              = link_to admin_users_path(sort: sort_value_recently_signin, filter: params[:filter]) do
-                = sort_title_recently_signin
-              = link_to admin_users_path(sort: sort_value_oldest_signin, filter: params[:filter]) do
-                = sort_title_oldest_signin
-              = link_to admin_users_path(sort: sort_value_recently_created, filter: params[:filter]) do
-                = sort_title_recently_created
-              = link_to admin_users_path(sort: sort_value_oldest_created, filter: params[:filter]) do
-                = sort_title_oldest_created
-              = link_to admin_users_path(sort: sort_value_recently_updated, filter: params[:filter]) do
-                = sort_title_recently_updated
-              = link_to admin_users_path(sort: sort_value_oldest_updated, filter: params[:filter]) do
-                = sort_title_oldest_updated
+              - users_sort_options_hash.each do |value, title|
+                = link_to admin_users_path(sort: value, filter: params[:filter], search_query: params[:search_query]) do
+                  = title
         = link_to 'Send email to users', admin_email_path, class: 'btn'
         = link_to 'New user', new_admin_user_path, class: 'btn btn-success btn-search'
 

changelogs/unreleased/50626-searching-users-by-the-admin-panel-wipes-query-when-using-sort.yml

+---
+title: Allow search and sort users at same time on admin users page
+merge_request: 23439
+author:
+type: fixed

spec/features/admin/admin_users_spec.rb

 require 'spec_helper'
 
 describe "Admin::Users" do
+  include Spec::Support::Helpers::Features::ListRowsHelpers
+
   let!(:user) do
     create(:omniauth_user, provider: 'twitter', extern_uid: '123456')
   end
@@ -30,6 +32,51 @@ describe "Admin::Users" do
       expect(page).to have_button('Delete user and contributions')
     end
 
+    describe 'search and sort' do
+      before do
+        create(:user, name: 'Foo Bar')
+        create(:user, name: 'Foo Baz')
+        create(:user, name: 'Dmitriy')
+      end
+
+      it 'searches users by name' do
+        visit admin_users_path(search_query: 'Foo')
+
+        expect(page).to have_content('Foo Bar')
+        expect(page).to have_content('Foo Baz')
+        expect(page).not_to have_content('Dmitriy')
+      end
+
+      it 'sorts users by name' do
+        visit admin_users_path
+
+        sort_by('Name')
+
+        expect(first_row.text).to include('Dmitriy')
+        expect(second_row.text).to include('Foo Bar')
+      end
+
+      it 'sorts search results only' do
+        visit admin_users_path(search_query: 'Foo')
+
+        sort_by('Name')
+
+        expect(page).not_to have_content('Dmitriy')
+        expect(first_row.text).to include('Foo Bar')
+        expect(second_row.text).to include('Foo Baz')
+      end
+
+      it 'searches with respect of sorting' do
+        visit admin_users_path(sort: 'Name')
+
+        fill_in :search_query, with: 'Foo'
+        click_button('Search users')
+
+        expect(first_row.text).to include('Foo Bar')
+        expect(second_row.text).to include('Foo Baz')
+      end
+    end
+
     describe 'Two-factor Authentication filters' do
       it 'counts users who have enabled 2FA' do
         create(:user, :two_factor)
@@ -628,4 +675,10 @@ describe "Admin::Users" do
   def check_breadcrumb(content)
     expect(find('.breadcrumbs-sub-title')).to have_content(content)
   end
+
+  def sort_by(text)
+    page.within('.user-sort-dropdown') do
+      click_link text
+    end
+  end
 end

spec/features/groups/members/list_members_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe 'Groups > Members > List members' do
   include Select2Helper
+  include Spec::Support::Helpers::Features::ListRowsHelpers
 
   let(:user1) { create(:user, name: 'John Doe') }
   let(:user2) { create(:user, name: 'Mary Jane') }
@@ -43,12 +44,4 @@ describe 'Groups > Members > List members' do
       let(:user_with_status) { user2 }
     end
   end
-
-  def first_row
-    page.all('ul.content-list > li')[0]
-  end
-
-  def second_row
-    page.all('ul.content-list > li')[1]
-  end
 end

spec/features/groups/members/manage_members_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe 'Groups > Members > Manage members' do
   include Select2Helper
+  include Spec::Support::Helpers::Features::ListRowsHelpers
 
   let(:user1) { create(:user, name: 'John Doe') }
   let(:user2) { create(:user, name: 'Mary Jane') }
@@ -119,14 +120,6 @@ describe 'Groups > Members > Manage members' do
     end
   end
 
-  def first_row
-    page.all('ul.content-list > li')[0]
-  end
-
-  def second_row
-    page.all('ul.content-list > li')[1]
-  end
-
   def add_user(id, role)
     page.within ".users-group-form" do
       select2(id, from: "#user_ids", multiple: true)

spec/features/projects/members/list_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe 'Project members list' do
   include Select2Helper
+  include Spec::Support::Helpers::Features::ListRowsHelpers
 
   let(:user1) { create(:user, name: 'John Doe') }
   let(:user2) { create(:user, name: 'Mary Jane') }
@@ -83,14 +84,6 @@ describe 'Project members list' do
     end
   end
 
-  def first_row
-    page.all('ul.content-list > li')[0]
-  end
-
-  def second_row
-    page.all('ul.content-list > li')[1]
-  end
-
   def add_user(id, role)
     page.within ".users-project-form" do
       select2(id, from: "#user_ids", multiple: true)

spec/support/helpers/features/list_rows_helpers.rb

+# frozen_string_literal: true
+# These helpers allow you to access rows in the list
+#
+# Usage:
+#   describe "..." do
+#   include Spec::Support::Helpers::Features::ListRowsHelpers
+#     ...
+#
+#     expect(first_row.text).to include("John Doe")
+#     expect(second_row.text).to include("John Smith")
+#
+module Spec
+  module Support
+    module Helpers
+      module Features
+        module ListRowsHelpers
+          def first_row
+            page.all('ul.content-list > li')[0]
+          end
+
+          def second_row
+            page.all('ul.content-list > li')[1]
+          end
+        end
+      end
+    end
+  end
+end