#4429 closed defect (bug) (fixed)
User banned from public group can still reply to group activity items
Reported by: | DJPaul | Owned by: | rachelbaker |
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | 1.2 |
Component: | Activity | Keywords: | has-patch |
Cc: |
Description
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.
Attachments (3)
Change History (23)
#3
@
12 years ago
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.
#6
@
12 years ago
- Keywords needs-refresh reporter-feedback added; needs-patch removed
Any updates here?
#7
@
12 years ago
- Keywords 1.8-early added; needs-refresh removed
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.
#9
@
12 years ago
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.
#11
@
11 years ago
- Milestone changed from 1.9 to 2.0
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.
#12
@
11 years ago
The database considerations should be mitigated by #3856, which implements some pre-fetching for groups data at the beginning of the activity loop.
#14
@
11 years ago
- Keywords reporter-feedback removed
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.
#16
@
8 years ago
The performance issues noted by Boone should no longer be a problem with BuddyPress 2.6.
Maybe time to revive this for v2.7?
#17
@
7 years ago
- Keywords has-patch added
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.
#19
@
7 years ago
- Milestone changed from Awaiting Contributions to 3.0
- Version set to 1.2
This one is finally fixed!
Thanks everyone for their contributionsn to this ticket.
blocking activity comments by banned users