[buddypress-trac] [BuddyPress Trac] #4785: Decouple "visibility" and "access" properties

[buddypress-trac]

#4785: Decouple "visibility" and "access" properties
-------------------------+------------------
 Reporter:  smninja      |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  2.1
Component:  Groups       |     Version:  1.7
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |
-------------------------+------------------
Changes (by dcavins):

 * cc: david.cavins@… (added)
 * keywords:  needs-patch => has-patch


Comment:

 Thanks for doing a whole bunch of work on this. I was able to address a
 few issues, and this seems to be working as expected for me. The weird
 404s you were getting were a result of `user_has_access()` being a
 protected method; the filter was unable to find/use it so returned false
 (I didn't know this->
 http://codex.wordpress.org/Function_Reference/add_filter "The function
 returns true if the attempted function hook succeeds or false if not.")

 The setup still seems a little over-complicated, though. In order to
 display pages from a private group to the public, your plugin must set all
 three of
 {{{
 'visibility'        => 'public',
 'enable_nav_item'   => true,
 'access'            => array(
                                         'public'  => 'anyone',
                                         'private' => 'anyone',
                                         'hidden'  => 'members',
                                 )
 }}}

 Could we condense these options to two:
 1. Should the plugin be active for this group? (replacing
 `enable_nav_item`, as it appears that it currently means "show the tab,"
 but if it's false, you can't reach the plugin's display screen even if the
 plugin is active, and that value is used for `user_has_access`, too:
 https://buddypress.trac.wordpress.org/browser/tags/1.9.2/bp-groups/bp-
 groups-classes.php#L3032)
 2. Who should have access to the plugin's display screen? (the new access
 array)

 (I'm trying to imagine a situation in which you'd want to display a
 navigation item for a page the user can't visit. The flip side is a page
 that the user can visit by URL but that doesn't have a navigation item.
 Since those cases seem unusual, I think replacing `enable_nav_item` with
 `enable_plugin` makes sense, and we could use the `enable_nav_item`
 setting for that setting if it doesn't exist to provide backwards
 compatibility. I didn't do this in this patch, though, since it seems like
 a pretty big departure.)

 I also used the new filter in `setup_display_hooks()` to help determine if
 the subnav item should be created and also what value `user_has_access`
 should have. Finally, I made some updates to the theme file
 groups/single/home.php because it was also doing access checks based on
 `bp_group_is_visible()`.

 This would be a pretty exciting change for me, so please offer comments
 and guidance and I'll be happy to help in any way I can. I'll be even more
 thrilled if we can arrive at a simpler setup that would work and be
 backwards compatible.

 On a side note, what would setting a plugin's visibility to "private"
 actually do? https://buddypress.trac.wordpress.org/browser/tags/1.9.2/bp-
 groups/bp-groups-classes.php#L2800 Who could visit the plugin's display
 screen then?

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/4785#comment:15>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac