Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 6 years ago

#5155 closed defect (bug) (fixed)

Messaging: ajax autocomplete does not work if users want to send an email to users login name

Reported by: quan_flo Owned by: boonebgorges
Milestone: 2.0 Priority: normal
Severity: normal Version: 1.8.1
Component: Core Keywords: needs-refresh needs-unit-tests
Cc:

Description

Many sites use the login name and do not display the real name of a user.

If you want to write a message to another user, autocompleter will not work with login-name.

I'd love to see this feature included in buddypress, so I modified the code of bp-core-classes.php

Line 1101 and 1102 were modified:

		$total_users_sql = apply_filters( 'bp_core_search_users_count_sql', "SELECT COUNT(DISTINCT u.ID) as id FROM {$wpdb->users} u LEFT JOIN {$bp->profile->table_name_data} pd ON u.ID = pd.user_id WHERE {$status_sql} AND (pd.value LIKE '%%{$search_terms}%%' OR u.user_login LIKE '%%{$search_terms}%%') ORDER BY pd.value ASC", $search_terms );
		$paged_users_sql = apply_filters( 'bp_core_search_users_sql',       "SELECT DISTINCT u.ID as id, u.user_registered, u.user_nicename, u.user_login, u.user_email FROM {$wpdb->users} u LEFT JOIN {$bp->profile->table_name_data} pd ON u.ID = pd.user_id WHERE {$status_sql} AND (pd.value LIKE '%%{$search_terms}%%' OR u.user_login LIKE '%%{$search_terms}%%') ORDER BY pd.value ASC{$pag_sql}", $search_terms, $pag_sql );

user_login was added to the query.

If you have concerns if everybody wants to have this, you could user a define that can be set in bp-custom.php for example.

Attachments (1)

5155.diff (7.6 KB) - added by imath 7 years ago.

Download all attachments as: .zip

Change History (13)

#1 @DJPaul
7 years ago

  • Keywords dev-feedback added

Not sure if we want to do this, so asking for other core team feedback.

@quan_flo, is this the only thing stopping auto-complete working with the login-name? You didn't have to change anything else to get it to work?

#2 @boonebgorges
7 years ago

Autocomplete ajax appears to be the only place in BP where we use BP_Core_User::search_users(). Most search passes through BP_User_Query, and that'll take significant refactoring to search by $wpdb->users fields as well, as they're not currently joined (and, ideally, will not be joined).

Since we're already doing the join in BP_Core_User::search_users(), the damage is already being done there, and I don't have a particular problem adding a search by user_nicename or user_login fields. However, it's going to create a disparity between this search and the main search in BP_User_Query. (You might further argue that we should refactor the autocomplete to use BP_User_Query, for less reduplication of code.)

On balance, I guess my preference would be not to add more enhancements to BP_Core_User::search_users(). Instead, I'd suggest that we add functionality to search against $wpdb->users in BP_User_Query, and then switch the ajax over. More work, but a better end product.

#3 @imath
7 years ago

I've run some tests, in a particular config. I imagined an admin deactivated the xprofile component. In this particular case : member search is not working at all as BP_User_Query do not search if xprofile is not active. So i suggest to change a little this behavior at the same time the enhancement suggested by @quan_flo is applied.

Now still under this particular config and deactivating the friends component, if the admin sets define( 'BP_MESSAGES_AUTOCOMPLETE_ALL', true ); in his wp-config.php then the message component is not usable at all as $bp->profile->table_name_data is not defined. A notice error is thrown and there's no way to select a user.

In 5155.diff i've also added a bp_is_active( 'xprofile' ) in BP_Core_User::search_users() as some admin can eventually use the bp_use_legacy_user_query filter.

I'm not using friends_search_friends() anymore in the patch, but i think it's best to leave it if some plugins/themes are using it..

@imath
7 years ago

#4 @boonebgorges
7 years ago

  • Milestone changed from Awaiting Review to 1.9

Thanks very much, imath. This is getting close to what I was envisioning.

However, if we're going to fix the problem originally reported in this ticket - which was not directly connected to the xprofile component - we should *really* fix it. Instead of this logic:

if ( bp_is_active( 'xprofile' ) ) {
    search the profile fields
} else {
    search the user fields
}

I was thinking we'd do something more comprehensive, like this:

$found_user_ids = select from wp_users ...

if ( bp_is_active( 'xprofile' ) ) {
    $profile_found_user_ids = select from xprofile ...
    $found_user_ids = array_unique( array_merge( $profile_found_user_ids, $found_user_ids ) );
}

That way, it'll be possible to match against the user table fields regardless of whether xprofile is enabled.

In 5155.diff i've also added a bp_is_active( 'xprofile' ) in BP_Core_User::search_users() as some admin can eventually use the bp_use_legacy_user_query filter.

Yeah, it looks like this will fix a separate bug, which is that the search_users() method will fail if xprofile is disabled. We'll handle it in a different commit (it's fine to keep it in this ticket).

I'm not using friends_search_friends() anymore in the patch, but i think it's best to leave it if some plugins/themes are using it..

Sounds good.

I would also like to have unit tests for the new search logic in BP_User_Query before making the proposed changes.

So, we're close enough that I'll move this to 1.9. Thanks for your work so far, imath.

#5 @boonebgorges
7 years ago

  • Keywords needs-refresh needs-unit-tests added; dev-feedback removed

#6 @imath
7 years ago

You're welcome Boone, and i agree about the code you wrote : much better ;)

#7 @imath
7 years ago

Side note : i think fixing this ticket might also fix #3511

#8 @boonebgorges
7 years ago

Thanks, imath. I've closed that one as a duplicate.

#9 @boonebgorges
7 years ago

  • Milestone changed from 1.9 to 2.0

#10 @DJPaul
6 years ago

  • Keywords has-patch removed

#11 @boonebgorges
6 years ago

In 8087:

BP_User_Query search_terms should match against user_login and user_nicename in addition to xprofile fields

This changeset changes the search_terms logic in BP_User_Query so that it
matches user_login and user_nicename in the core users table. Then, via filter,
the xprofile component amends the WHERE clause so that search_terms are matched
in the users table OR the xprofile data table.

Clauses have been refactored to subqueries rather than storing the matched
user IDs in PHP, for improved performance.

See #5155

#12 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 8088:

Match user_nicename and user_login in addition to xprofile fields in messages autocomplete

The AJAX handler bp_legacy_theme_ajax_messages_autocomplete_results() now uses
BP_User_Query and its search_terms parameter for matching users.

Fixes #5155

Props imath for an initial patch

Note: See TracTickets for help on using tickets.