[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