User banned from public group can still reply to group activity items

[DJPaul]

A user banned from a public group can still reply to group activity items. This should be changed as, in the Default theme at least, the activity items are visually represented as belonging inside of a group -- so the ban should apply across all content in a group regardless of which underlying BP component it belongs to.

[rachelbaker]

blocking activity comments by banned users

[DJPaul]

Thanks for the patch, Rachel. Some initial feedback (I haven't tested the patch yet): we need to check that the Groups component is active before calling the function, and that the activity itself is related to a group.

It might also be more elegant to hook into (for example) bp_activity_can_comment from inside the Groups component and do the logic there; it helps to keep components tidy and unentangled. I know there's lots of parts of core that don't do this, but it might be a nice thing to aim for going forward.

[rachelbaker]

Paul,

Yeah on second look, you are right. I will rework the patch.

[johnjamesjacoby]

Any updates here?

[boonebgorges]

I just whipped up a patch along the lines of what rachelbaker and DJPaul have been suggesting here. See 4229.01.patch.

It works, but is not ready for primetime. In order to make it a truly general solution (one that works in the sitewide activity stream etc, as well as on a single group activity stream), you have to do this for each activity item:

groups_is_user_banned( bp_loggedin_user_id(), bp_get_activity_item_id() )

This results in an additional database hit for every single group-related activity item. The is_banned data is not cached at all. On activity pages, this could result in 20 extra queries per page, which is unacceptable.

We should be doing one or both of the following:

• Functions like groups_is_user_banned() should cache their results somehow
• When fetching the activity stream and building the $activities_template global, also slurp up whether the logged-in user is a member of each returned activity item.

There's no time for either in this cycle. Let's take a swing for the next cycle.

[imath]

hi,

When the user is not a group member, he can comment activity or reply to activity comment that "belongs" to the group. I think it can be confusing for group admins to see that non members can contribute to the group content (in this case activity). I wonder if it's a good idea to include a "groups_is_user_member()" check on your patch.. and add something to check for the membership in case the non member wants to reply to a comment. (made a gist to suits my need : ​https://gist.github.com/imath/4966936)

So in the same spirit, Boone, i think you should also filter 'bp_activity_can_comment_reply' to check for banned user, because i've noticed on my case that if a member of a group commented on a group activity, then a non member can reply to this comment.

[boonebgorges]

Thanks for the feedback, imath. Disallowing non-members from making comments on group activity items is a more radical change in behavior than what was originally being suggested, and I fear that it might alter the dynamics of certain sites. I guess I'd like to hear what other people think too.

Bumping this because it needs more work to reduce database overhead.

[boonebgorges]

The database considerations should be mitigated by #3856, which implements some pre-fetching for groups data at the beginning of the activity loop.

[boonebgorges]

There are still some performance issues that are causing bottlenecks here. (There is currently no good way to cache is_banned values, because of some confusion about whether the results of groups_get_extras belong in the cache.) See #5407 for more discussion.

[r-a-y]

The performance issues noted by Boone should no longer be a problem with BuddyPress 2.6.

Maybe time to revive this for v2.7?

[r-a-y]

I've added an updated patch that also filters 'bp_activity_can_comment_reply'.

The performance improvements for groups_is_user_banned() should be addressed since #6327.

[r-a-y]

In 11959:

Groups: Banned group members shouldn't be able to comment on activity items.

Previously, we did not check activity commenting capabilities against group
membership status.

Props rachelbaker, boonebgorges, imath, DJPaul, r-a-y.

Fixes #4429.

[r-a-y]

This one is finally fixed!

Thanks everyone for their contributions to this ticket.

[r-a-y]

In 12131:

Groups: Fix display of "Comment" button in the activity stream.

In r11959, the banned group members check is using the wrong conditional.
This caused legitimate group members to never see the "Comment" button in
the activity stream.

This commit reverses the check and adds some unit tests (which should have
been written beforehand!).

Anti-props r-a-y.

See #4429.

Fixes #7854 (3.x branch).