Use BP_Groups_Group visibility and access properties.

[dcavins]

When checking visibility for the current user, use the is_visible property in BP_Groups_Group rather than calculating it again. Similarly, use the user_has_access property when checking access to a group. (is_visible was introduced in 2.9.)

[dcavins]

Use BP_Groups_Group properties in some cases.

[DJPaul]

If ->is_visible handles both private and hidden groups correctly for this change, go for it.

[DJPaul]

Yes it does.

[djpaul]

In 11761:

Groups: use class methods to check if user has visibility of a group.

Improves upon existing duplication of logic.

Fixes #7608

Props dcavins

[r-a-y]

Reopening because the changes to bp_group_is_visible() allows unauthenticated users to view a private group's homepage:
​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-templates/bp-legacy/buddypress/groups/single/home.php?marks=78#L75

@dcavins - Although it makes sense to use the 'is_visible' property in the bp_group_is_visible() function, we should probably revert due to backward compatibility issues. I don't have any bright ideas to fix this without introducing changes to our templates, which I don't think we want to do.

[dcavins]

@r-a-y
Thanks for your comments. I see what you mean-- the function bp_group_is_visible() wasn't actually checking visibility, it was checking to see if the user had access. Here's a list of places in BP where we use bp_group_is_visible() to enable access to something:

• groups_action_group_feed(): to display the feed or not (items posted to private groups are sensitive to the current user anyway, now)
• bp-legacy/buddypress/groups/single/cover-image-header.php and bp-legacy/buddypress/groups/single/group-header.php: Whether to display Admin and Moderator lists
• bp-legacy/buddypress/groups/single/home.php: Whether to load the bp_groups_front_template_part()
• bp-nouveau/buddypress/groups/single/parts/header-item-actions.php: Whether to display Admin and Moderator lists
• bp_nouveau_group_template_part(): Whether to display group no-access message.

Bummer. I suppose we should revert it, but can we, instead of putting the old logic back directly, have it wrap a call to bp_current_user_can( 'groups_access_group' ), and also add a comment about how the name got out of sync with the actual property it's checking. Also, before nouveau is released into the wild, can we just replace those checks with calls to the access_group check?

I'll attach a patch that includes the above.

Thanks again for catching this misunderstanding on my part.

[r-a-y]

I like revert.02.patch. Didn't even know of the new bp_groups_user_can_filter() function!

I think the changes to the Nouveau templates are good, but I believe the 'group_id' parameter needs to be passed in the bp_current_user_can() function?

Other than that, let's roll with your patch. Thanks @dcavins!

[dcavins]

Another revert option.

[dcavins]

Hi @r-a-y-

Thanks again for your feedback. The group_id is optional in those bp_current_user_can() calls because bp_get_current_group_id() is reliable in those two instances (and bp_groups_user_can_filter() uses it to get the current group if no group_id is passed). I'm leaving it off to keep it simpler (in those template uses), but we can add it to be complete if that makes more sense.

@boonebgorges, as the keeper of the history of the groups component, can you check my "historical note" in the attached patch to make sure I'm actually correct in my understanding?

Thanks!

[r-a-y]

I'm leaving it off to keep it simpler (in those template uses), but we can add it to be complete if that makes more sense.

I missed that! You're right, no need to add in the 'group_id' in this case.

[boonebgorges]

I believe that the "historical note" is correct.

[dcavins]

In 11896:

Revert bp_group_is_visible() to check access.

In r11761, bp_group_is_visible() was changed to rely on the BP_Groups_Group->is_visible property. However, bp_group_is_visible() has been used to check access, not visibility, making the new logic, even though it seems sensible, wrong.

This changeset also replaces bp_group_is_visible() in the BP Nouveau template parts, to start retiring this no-longer-correctly-named function.

Props r-a-y.

See #7608.