Skip to:
Content

BuddyPress.org

Opened 11 years ago

Closed 11 years ago

#5114 closed enhancement (fixed)

BP User Query / Alphabetical sort - All Members tab count and spammers

Reported by: imath's profile imath Owned by: boonebgorges's profile boonebgorges
Milestone: 1.9 Priority: normal
Severity: normal Version: 1.8
Component: Members Keywords: has-patch
Cc: brajesh@…

Description

I've noticed that sorting members the alphabetical way was displaying all members even the ones marked as spammers. The result is that :

  • the All Members tab count can be not coherent with the members displayed.
  • if a non admin click on a user marked as spammer, he is redirected to a 404.

I've read that the use of 'last_activity' was problematic regarding query perdormance (#4060), so the draft diff i attached to this ticket might not be adapted, as it uses this usermeta to also manage the display in case of alphabetical sort order. But on the same time, i feel there must be something to do about this kind of sort order to make it more coherent with others...

Attachments (3)

5114.diff (2.2 KB) - added by imath 11 years ago.
5114.1.diff (854 bytes) - added by sbrajesh 11 years ago.
Fix the issue of showing all members on alphabetical sorting
5114.02.patch (3.2 KB) - added by boonebgorges 11 years ago.

Download all attachments as: .zip

Change History (15)

@imath
11 years ago

#1 follow-up: @boonebgorges
11 years ago

  • Milestone changed from Awaiting Review to 1.9

Good catch, imath. We should definitely fix this bug.

However, for the reasons you suggest, I'm still not keen on joining against usermeta - it's really horendous in terms of performance. (This was the main reason for developing BP_User_Query in the first place.)

If the problem is that spammed users are showing up, then we might think about excluding spam users by looking at the user_status and/or spam columns in the users table. I'm not sure why these checks weren't originally included in BP_User_Query. My guess is that it's because it wasn't present in the legacy queries, because *they* joined against last_activity.

In any case, I'm moving this to 1.9, as it's something that we should fix either way.

#2 in reply to: ↑ 1 @imath
11 years ago

Replying to boonebgorges:

Good catch, imath. We should definitely fix this bug.

Thanks Boone :) I haven't thought about user_status or spam, i see the interest as these fields are in $wpdb->users ;)

I think, another thing "obsessed" me and made me blind about using these fields. I'm working on a blog that already have a bunch of users before BuddyPress has been installed. And in this case, As i'm the only one who logged in after BuddyPress installed, All Members tab count is 1 and displayed users for online, active, newest is 1. But Alphabetical is displaying all users. So untill all other users logged in, Members tab count != count( alphabetical displayed users ) .

But i really like your idea. The above inconvenient will disappear as soon as users log in.

#3 @boonebgorges
11 years ago

  • Milestone changed from 1.9 to 2.0

I've taken a brief look here and I think that more detailed attention is going to be necessary, if only because there are a couple different kinds of alphabetical sorts (one with xprofile and one without).

#4 @sbrajesh
11 years ago

  • Cc brajesh@… added
  • Keywords has-patch added

Hi,
A little bit late here but faced the same issue. Here is a patch attached which does a sub query based on user_status.

I am checking for user_status!=0. There is a negative query using NOT IN and I am hoping that it should not be a performance problem.

Is it possible that we can include something in 1.9 ( My apologies if it is too late for this).

@sbrajesh
11 years ago

Fix the issue of showing all members on alphabetical sorting

#5 @johnjamesjacoby
11 years ago

When patching, please think of using bp_core_get_status_sql() which takes both single and multisite into account.

#6 @boonebgorges
11 years ago

  • Keywords needs-testing added

OK, since we're going to have to do another beta anyway, and since this issue is pretty annoying, let's go ahead and fix it.

sbrajesh, imath, I have attached a patch. Can I ask you to test it ASAP and report back? I would like verification that it's fixing the problem as expected before moving forward.

#7 @sbrajesh
11 years ago

Hi Boone,
Thank you for the updated patch. It works like a charm :)

johnjamesjacoby, that was an oversight on my part. Thank you for pointing :)

#8 @sbrajesh
11 years ago

  • Keywords reporter-feedback added; needs-testing removed

#9 @imath
11 years ago

Hi Boone,

Thanks for the patch :)

Just tested it in WordPress 3.8-beta1 - Multisite on. I've created a user in a child blog. I've logged in the main site where BuddyPress runs with the new user's credentials. Then as a Super Admin, i've went to front end user's profile to mark him as a spammer.

I've tested the patch, and i confirm a user marked as spam is no more displayed when alphabetical order is selected in the members directory.

But, i've noticed a strange behavior. I need to explore a bit more why it's happening and may submit a new ticket later.
In short :
if i mark a user as spammer from the front end (site.url/members/user/settings/capabilities/) and i unspam the user from the WordPress Network Administration screen for users using the bulk action then the user seems to be unspammed from this screen.
But logging in with the user is forbidden as he's still consider as a spammer. The SuperAdmin needs to go to user's profile on front end (site.url/members/user/settings/capabilities/) to unmark user as a spammer to finally let him log in.

When marking a user as spammer

  • from Front end : $wpdb->users fields 'user_status' and 'spam' are set to 1
  • from WordPress Network Admin users list : only the field 'spam' is set to 1 in $wpdb->users

When unmarking a user as spammer

  • from Front end : $wpdb->users fields 'user_status' and 'spam' are set to 0
  • from WordPress Network Admin users list : only the field 'spam' is set to 0 in $wpdb->users

i'll check with WordPress 3.7.1

#10 @imath
11 years ago

  • Keywords reporter-feedback removed

#11 @boonebgorges
11 years ago

  • Milestone changed from 2.0 to 1.9

Thanks, guys.

imath, it sounds like your bug is unrelated to the issue discussed in this ticket. Please do start a separate ticket once you've investigated a little bit more.

#12 @boonebgorges
11 years ago

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

In 7638:

Prevent BP_User_Query from fetching spam users on alphabetical sort

BP_User_Query's 'alphabetical' sort type, in contrast to the other values of
'type', does not look at users' last_activity usermeta value. As a result, it
cannot infer spam/deleted/activated status in the same way as the other 'type'
values. To prevent spammed/deleted/unactivated users from being returned on
alphabetical sorts, we do a subquery that limits results to user_status = 0.

Fixes #5114

Props sbrajesh, imath

Note: See TracTickets for help on using tickets.