There is code in the UserSearch plugin that is attempting to make it possible to search for users using * wildcards:

   $keys = $this->keywords;
    // Replace wildcards with MySQL/PostgreSQL wildcards.
    $keys = preg_replace('!\*+!', '%', $keys);

However, this doesn't work (I've verified this in a manual test by searching for "jho*n" on a site with user "jhodgdon" defined), because later on the whole keyword query is surrounded by % so you are looking for a substring, and the % added above is escaped:

      $query->condition('name', '%' . $this->database->escapeLike($keys) . '%', 'LIKE')

D7 has the same problem [verified]. The code there is in user.module :: user_search_execute():

  $keys = preg_replace('!\*+!', '%', $keys);
...
    $query->condition('name', '%' . db_like($keys) . '%', 'LIKE')

D6 does not have the problem [verified], because there the code (in user_search() in user.module) is:

        $keys = preg_replace('!\*+!', '%', $keys);
...
         $result = pager_query("SELECT name, uid, mail FROM {users} WHERE LOWER(name) LIKE LOWER('%%%s%%') OR LOWER(mail) LIKE LOWER('%%%s%%')", 15, 0, NULL, $keys, $keys);

so there is no escaping of the % in D6.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Issue summary: View changes
jhodgdon’s picture

Assigned: Unassigned » jhodgdon
Issue summary: View changes

I just tested d6 and d7 and verified this problem exists in D7 but it works fine in D6. So this is a regression from Drupal 6. Should actually be easy to fix, and easy to add a test for.

jhodgdon’s picture

Was easy to fix. Here's a test-only patch that should have two failures on the new wildcard search lines, and a patch with also the code fix. Works locally...

Status: Needs review » Needs work

The last submitted patch, 3: 2251019-user-wildcard-search-TEST_ONLY_FAIL.patch, failed testing.

jhodgdon’s picture

Status: Needs work » Needs review

Test-only patch failed as intended, fix + test patch passed, let the reviews begin, please! :)

mgifford’s picture

I can conform that your patch works when searching users search/user?keys=ad*m

I hoped that it would also work when filtering on the admin side admin/people?user=ad*m&role=All&permission=All&status=All

I just searched on "ad*m" as it was always there and was easiest.

EDIT: Looks good to me!

jhodgdon’s picture

The admin page is a view. It's not using the UserSearch plugin, and has nothing to do with this patch. This patch fixes a regression in Search.

So so you think this is RTBC?

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the clarification about the admin page. And yes..

jhodgdon’s picture

Assigned: jhodgdon » Unassigned

Unassigning to avoid "jhodgdon is committing this" confusion in the RTBC queue.

alexpott’s picture

Version: 8.0.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 5385c22 and pushed to 8.0.x. Thanks!

  • alexpott committed 5385c22 on 8.0.x
    Issue #2251019 by jhodgdon: Fixed User wildcard search doesn't work.
    

  • alexpott committed 5385c22 on 8.1.x
    Issue #2251019 by jhodgdon: Fixed User wildcard search doesn't work.
    
pietmarcus’s picture

Assigned: Unassigned » pietmarcus

I'm not sure if backports are still accepted for 7.x, but I'll backport it anyway, if only for learning how to help with development of Drupal

jhodgdon’s picture

Backports for bug fixes are definitely still being accepted for Drupal 7, and in some cases, even feature releases. Thanks!

pietmarcus’s picture

Added a test-only patch (which will fail) and the patch with the test and fixed code.

The last submitted patch, 15: 2251019-user-wildcard-search-d7-TEST_ONLY_FAIL.patch, failed testing.

pietmarcus’s picture

Test only patch failed as intended, fix passed. Please review.

jhodgdon’s picture

Status: Needs review » Needs work

Thanks! This looks good.

The test-only patch had some weird unrelated failures, so I'm going to retest it after I submit this comment.

There's also a very small problem with the patch:

+++ b/modules/user/user.test
@@ -2229,6 +2229,13 @@ class UserUserSearchTestCase extends DrupalWebTestCase {
+    ¶

Extra space at the end of this line. You should be able to set up your text editor to either delete or highlight end-of-line spaces to avoid this.

So please make a new patch. You don't need to make a new test-only patch. Thanks!

The last submitted patch, 15: 2251019-user-wildcard-search-d7-TEST_ONLY_FAIL.patch, failed testing.

jhodgdon’s picture

Ugh. That retest of the test-only patch was even worse. I'll try again. I was not aware the testing platform was so unstable in D7!

The last submitted patch, 15: 2251019-user-wildcard-search-d7-TEST_ONLY_FAIL.patch, failed testing.

pietmarcus’s picture

Status: Needs work » Needs review
FileSize
2.56 KB

Ugh, I thought I had my IDE set up correctly. Sorry for that. Settings are fixed, and so is the patch. Thanks for the headsup!

pietmarcus’s picture

I'm on a roll here... Please disregard patch #22, it has some local settings to make sure PHP can finish on my machine. This is the correct one.

jhodgdon’s picture

Status: Needs review » Needs work

This looks good now, thanks!

However I just noticed that the test in your patch is not as comprehensive as the test in the D8 patch, which tested wildcards in email for admins (as yours does) but also wildcards in user name (which yours doesn't). Could you add that test too (and a new test-only patch)? Thanks!

pietmarcus’s picture

New test added. Passed locally (with patch).

The last submitted patch, 25: 2251019-user-wildcard-search-d7-TEST_ONLY_FAIL.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 25: 2251019-user-wildcard-search-with-tests-d7.patch, failed testing.

pietmarcus’s picture

Status: Needs work » Needs review
FileSize
2.38 KB

Failed tests have nothing to do with my fixes. Resubmitting for testing

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Yeah, the D7 testing system seems to be a bit unpredictable. I wouldn't bother to resubmit again if you see test failures. They will eventually sort themselves out, hopefully anyway.

Anyway, the latest patch looks good to me. Thanks for the quick work on this! Hopefully it will get committed to D7 sometime soon.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x - thanks!

Fixed on commit:

diff --git a/modules/user/user.test b/modules/user/user.test
index d05bb82..b9729c5 100644
--- a/modules/user/user.test
+++ b/modules/user/user.test
@@ -2230,19 +2230,19 @@ class UserUserSearchTestCase extends DrupalWebTestCase {
     $this->drupalPost('search/user/', $edit, t('Search'));
     $this->assertText($keys);
 
-    // Verify that wildcard search works
+    // Verify that wildcard search works.
     $keys = $user1->name;
     $keys = substr($keys, 0, 2) . '*' . substr($keys, 4, 2);
     $edit = array('keys' => $keys);
     $this->drupalPost('search/user/', $edit, t('Search'));
-    $this->assertText($user1->name, 'Search for username wildcard resulted in user name on page for administrative user');
+    $this->assertText($user1->name, 'Search for username wildcard resulted in user name on page for administrative user.');
 
-    // Verify that wildcard search works for email
+    // Verify that wildcard search works for email.
     $keys = $user1->mail;
     $keys = substr($keys, 0, 2) . '*' . substr($keys, 4, 2);
     $edit = array('keys' => $keys);
     $this->drupalPost('search/user/', $edit, t('Search'));
-    $this->assertText($user1->name, 'Search for email wildcard resulted in user name on page for administrative user');
+    $this->assertText($user1->name, 'Search for email wildcard resulted in user name on page for administrative user.');
 
     // Create a blocked user.
     $blocked_user = $this->drupalCreateUser();

  • David_Rothstein committed f0b44bd on 7.x
    Issue #2251019 by PietM, jhodgdon, mgifford: User wildcard search doesn'...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.