Opened 11 years ago
Closed 11 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)
Change History (13)
#2
@
11 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
@
11 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..
#4
@
11 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.
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?