Opened 13 years ago
Closed 13 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 |
|---|---|---|---|
| Priority: | normal | Milestone: | 2.0 |
| Component: | Core | Version: | 1.8.1 |
| Severity: | normal | 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
@
13 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
@
13 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
@
13 years ago
- Milestone Awaiting Review → 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.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
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?