Skip to:
Content

Opened 20 months ago

Last modified 3 weeks ago

#4429 assigned defect (bug)

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

Reported by: DJPaul Owned by: rachelbaker
Milestone: 2.1 Priority: normal
Severity: normal Version:
Component: Activity Keywords: reporter-feedback
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 (2)

4429.patch (720 bytes) - added by rachelbaker 20 months ago.
blocking activity comments by banned users
4229.01.patch (2.2 KB) - added by boonebgorges 16 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 rachelbaker20 months ago

  • Owner set to rachelbaker
  • Status changed from new to assigned

rachelbaker20 months ago

blocking activity comments by banned users

comment:2 rachelbaker20 months ago

  • Keywords has-patch needs-testing added

comment:3 DJPaul20 months 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.

comment:4 rachelbaker20 months ago

Paul,

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

comment:5 rachelbaker20 months ago

  • Keywords needs-patch added; has-patch needs-testing removed

comment:6 johnjamesjacoby16 months ago

  • Keywords needs-refresh reporter-feedback added; needs-patch removed

Any updates here?

comment:7 boonebgorges16 months 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.

comment:8 boonebgorges16 months ago

  • Keywords 1.8-early removed
  • Milestone changed from 1.7 to 1.8

boonebgorges16 months ago

comment:9 imath14 months 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.

comment:10 boonebgorges11 months ago

  • Milestone changed from 1.8 to 1.9

comment:11 boonebgorges5 months 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.

comment:12 boonebgorges6 weeks ago

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

comment:13 boonebgorges3 weeks ago

  • Milestone changed from 2.0 to 2.1
Note: See TracTickets for help on using tickets.