[buddypress-trac] [BuddyPress Trac] #8734: Introduce simple "Private Site" toggle

buddypress-trac noreply at wordpress.org
Sat Jul 22 11:26:48 UTC 2023


#8734: Introduce simple "Private Site" toggle
-----------------------------+-----------------------
 Reporter:  dcavins          |       Owner:  (none)
     Type:  feature request  |      Status:  assigned
 Priority:  normal           |   Milestone:  12.0.0
Component:  Core             |     Version:  10.4.0
 Severity:  normal           |  Resolution:
 Keywords:  has-patch early  |
-----------------------------+-----------------------

Comment (by imath):

 Thanks again @dcavins, I've tested the patch it's working very nicely,
 here are my feedbacks and why I think we shouldn't apply visibility to
 each component but for the whole community area for 12.0.

 Some details:

 1. The PHP Null Coalescing Operator is probably not supported by PHP 5.6
 (which BuddyPress currently support) as it's part of 7.0 new featurs. We
 should avoid to use it as long as we support PHP 5.6.
 https://www.php.net/manual/en/migration70.new-features.php

 2. Some `'bp-rewrites'` textdomains need to be replaced by `'buddypress'`.

 3. I had a PHP 8.2 deprecated notice in bp-core-community-visibility.php
 on line 145
 {{{
     Deprecated: Automatic conversion of false to array is deprecated in
 /path-to/buddypress/src/bp-core/bp-core-community-visibility.php on line
 145
 }}}

 4. When not logged-in: I had a PHP Warning in src/bp-core/classes/class-
 bp-component.php on line 1323
 {{{
     PHP Warning:  Undefined variable $posts in
 /Users/imath/forks/buddypress/src/bp-core/classes/class-bp-component.php
 on line 1323
 }}}

 PS: fixes for points 2, 3 & 4 are in 8734.02.feedback attached patch.

 More globally:

 I saw you haven't took the `bp_restricted` post status road in favor of a
 serialized option. Reading the code, I understood you prefer to be
 consistent accross components as some do not have a `buddypress` post type
 associtated (eg: xProfile, Notification, etc..). I personaly believe we
 should keep things about a specific component into its associated
 `buddypress` post type (post status or post meta). But I'm very open on
 the subject, and I'm fine with the serialized option road as it has some
 benefits over the post status: in WordPress, the menu block only allow
 `publish` posts to be listed (which is too restrictive imho).

 I've tested this setting:
 - Global: visible to anyone
 - **Activity: only visible to members**
 - Members: visible to anyone
 - Groups: visible to anyone

 The activity directory displays the log in form and hide the restricted
 content. But if I go in a single Members/Groups item, I do see their
 activities. This is making me say, we need more time to think about how we
 manage visibility granularity and its persistence in DB.

 After more thoughts & for 12.0, I think we should stick with your initial
 goal: a simple option we can toggle to restrict or not the entire
 community to its members. + We need to remove some code I added about the
 `bp_restricted` post status road -> I've added the needed changes into
 8734.02.feedback.patch.

 About the BP REST API, we shouldn't use filters imho, but directly edit
 the BP REST API.

 About organizing the code into a specific file: I really see the community
 visibility as a Core feature, I believe functions should be placed into
 the existing core files (functions, filters, caps, etc). For instance the
 code in `bp_community_visibility_user_can_filter()` should directly be in
 `bp_map_meta_caps()` with no filter as it's always possible to use
 existing filters.

 About the setting, I believe it should be in the "Main settings" section
 for 12.0 so that we can immediately see it.

 PS: thanks a lot for including unit tests 😍

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


More information about the buddypress-trac mailing list