Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 14 months ago

Last modified 13 months ago

#8734 closed feature request (fixed)

Introduce simple "Private Site" toggle

Reported by: dcavins's profile dcavins Owned by: dcavins's profile dcavins
Milestone: 12.0.0 Priority: normal
Severity: normal Version: 10.4.0
Component: Core Keywords: has-patch commit
Cc:

Description

Explore adding a very simple "private site" feature like
[] Require user to be logged in to visit BuddyPress components
that would easily create a private site where content is only available to logged-in users.

These items would need to be considered:
Page viewing
REST access
RSS Feeds/other programmatic access?

Attachments (7)

8734.1.patch (6.7 KB) - added by dcavins 2 years ago.
Enable simple site privacy. Prevent access to BP pages, RSS feeds, REST API. Allow access to Registration and Activation screeens.
enable-private-site-option.png (36.7 KB) - added by dcavins 2 years ago.
Location of option input in BP settings.
8734.02.patch (23.5 KB) - added by dcavins 14 months ago.
Beta site privacy patch, now calles "community visibility".
community-visibility-settings.png (29.8 KB) - added by dcavins 14 months ago.
Community visibility settings pane.
8734.02.feedback.patch (7.8 KB) - added by imath 14 months ago.
8734.03.patch (20.2 KB) - added by dcavins 14 months ago.
Return to simple option for community visibility.
8734.03.feedbacks.patch (6.1 KB) - added by imath 14 months ago.

Download all attachments as: .zip

Change History (71)

#1 @shawfactor
2 years ago

There are several reasons why this should not be added

Firstly there plugins that do this already, so I don’t think it is necessary.

Secondly privacy is not binary, there are various levels of privacy that may be appropriate (all of which can be achieved by plugins

Thirdly I don’t think we need more options, remover decisions not options

#2 @imath
2 years ago

  • Milestone changed from Awaiting Review to 11.0.0

Thanks a lot for opening this ticket. Building a private community is a request users are often making about BuddyPress. Here are some examples:

I believe including a very straightforward way to use BuddyPress to add to his WordPress site a private community is something we should consider.

@shawfactor your first and second points are somehow the same "plugins can do that". Sure they can and it's fine if they can offer the various levels of privacy to extend a very basic level one provided by BuddyPress.

Problem is, a new user don't know about these plugins (me neither by the way!). He tries to see how it can be done from settings, don't find, uninstall BuddyPress and then share a bad review looking like "how come such a basic feature is not included into BuddyPress???"

The goal here is to include this very basic way of making a small private community as a start, if people need more granular control over privacy, then yes I trust you that there are specific plugins to help them satisfy it.

Let's talk about it during 11.0.0 development cycle. I'd really love to have more feedbacks about it and not only from developers like us but from users.

#3 @shawfactor
2 years ago

i reiterate that this is a terrible idea

You are right that buddypress is not useful by most people without additional plugins .

This is how it exactly should be. Social networking means many different things and it is highly unlikely that basic buddypress will match the goal you have when you first start. This is no different to WordPress itself, no one uses WordPress without plugins.

Indeed considering how complex social networking is trying to cater to the needs of even the majority of users in core is impossible. Arguable you’d need to add forums, attachments, anti spam, tagging, a buddypress theme, and privacy. That is just off the top of my head,

It gets ugly fast and sets a bad precedent both in terms of bloat (this is already a problem, abd I’ve raised two trac items about two legacy features that are a pain point) and it undermines the vital plugin developer ecosystem that exists around buddypress (I’m one).

Instead we should keep building the apis that are needed. The two urgent ones are the attachments api and integrating with full site editing.

The lesson we should take from the two reviews you linked too is the need for better documentation, help forums, and onboarding. I understand this is already happening

#4 @imath
2 years ago

Thanks for your 2nd comments about this, I understood your points. Are we all ok with:

« BuddyPress is not useful by most people without additional plugins » and that’s the way it should stay?

I’m not. We should at least provide all the basics plugins can extend.

Why having private or hidden groups but no native way to build a private community?

Again, I’d really like to read some other feedbacks and user ones if possible 🙂

#5 @shawfactor
2 years ago

You’ve mischaracterised my point

Buddypress is not particularly useful without additional plugins and that will remain the case even if a privacy option is added.

Sure we could add the various features over many years but it would be a massive amount of work and a huge backwards step as

  1. It would create a huge technical debt
  2. It would come at a huge opportunity cost to the real work that should be done
  3. It would be a massive pain to people who want to do those features differently (eg privacy is a spectrum that a simple option cannot capture
  4. It will alienate the small group of plugin developers who would be cannibalised

Like it or not buddypress like woocommerce is a platform and we should concentrate on the apis that others can build on top of.

#6 @imath
2 years ago

After your 3 comments I confirm I understood you don’t want us to include such a basic feature. Thanks a lot for your interest in the project directions. I want to read other people points of view, you clearly made your point but I don’t understand why a basic private community feature would do so much arm to the plugins ecosystem or would transform BuddyPress all of a sudden to something completely different than it is today.

I’d like to stress the fact that without users, there’s no BuddyPress or BuddyPress plugins. That’s more concerning imho than an option into the dashboard. BuddyPress plugin authors should be more involved into contributing to BuddyPress, I’m glad you often share your opinion, but I’m concerned not many others are at least beta testing the BuddyPress beta release. You’re talking about API’s but I never saw a patch from you to improve existing ones. Show the way! Build patches & contribute to code/tests and documentation, we’ll be very happy to be able to benefit from your work.

There would be things to think about on the BuddyPress plugins topic : how come each time I read from a user about it, it’s negative like « broke » or « no more maintained », etc.

We are clearly observing a huge decrease on active installs stats. This basic feature might not be the thing to inverse the trend, but it would at least show a basic private community can be easily built for users who only need a logged in user only community area. It doesn’t prevent plugins to build something more granular. So to me we can still consider it for 11.0.0.

Instead of saying « no » with the plugins perspective only in mind, let’s open our eyes on users who won’t search for plugins once they figured out there’s not even a basic switch to private pages like it exists in WordPress.

#7 @shawfactor
2 years ago

A basic privacy option won’t change much at all in itself but it sets a precedent.

Re patches from me, basically the main barrier is GitHub. I need to familiarise myself with it. I’m a self taught programmer I know it is hard to believe but I and have barely used it and would have no idea how to patch things (although I will learn). I’d also say that in terms of contributing buddypress will be better served by me publishing the rest of my plugins (I’ve about 50 I’ve been meaning to put in the repository)

Re installations falling. How dramatic? The statistics in the repository show a decline over 2-3 years. I’d speculate that is inevitable for several reasons. The first is the main competitor is Facebook so the overall market is declining but conversely I’d say the sites that do remain are higher value (more users and more complex). Secondly I’d say that buddyboss cannibalise buddypress so overall installs of both may be growing. Anyway I’d be interested in more data

Finally I’d suggest we’d be better served documenting, shepherding, and assisting new users of buddypress. Making it easier to add plugins, maybe having canonical plugins etc (see Matt Mullenwegs recent comments).

I realise this has all got a bit meta so if there is a different place to continue this thread let me know.

#8 @imath
2 years ago

  • Keywords needs-patch added

Ok, thanks for this new comment, in short : I’m ready to take the responsibility of setting this precedent. If we need to talk more about competitors and project management improvements (energy/time to put on documentation, support etc..). I’m totally open to this and I’ll add a specific point about it to the agenda of our next dev-chat (next Wednesday at 19:30 UTC).

So let’s move on to the « how ».

The more I think of it the more I believe as we use regular WordPress pages to set components root pages that we should use the corresponding page status to decide about the privacy of component’s content. This means we don’t need an option and can use the pages post statuses instead.

This means we’d need to take in account the protected by a password status and decide how we should handle it.

#9 @shawfactor
2 years ago

That makes perfect sense.

Using the post status makes perfect sense and is a low bloat logical approach, that actually allows a high level of flexibility.

My only concern thoughts are

  1. Is this future proof, ie is the plan to keep using pages to proxy buddypress components? I don’t have an opinion but I always believed that eventually buddypress would move away from doing that
  1. Would it work with custom post statuses? The would be a great approach btw. I’ve used your wp statuses library and the two could play nicely together, especially if the buddypress components moved to custom post types.

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


2 years ago

#11 @epgb101
2 years ago

Hi all..
I will add my 2-cents and probably be shot down :) I managed to make a private community using a plugin but it was a bit of a nightmare to get it right and I had t try a dozen plugins and it took me ages to get it working right with tones of testing and retesting - several plugins did not work properly or partially. While I was doing this I was really wishing it was built in and to my mind is a no-brainer - but then again I am not the one having to code it and I REALLY appreciate what you people do. So perhaps it should be built in - or not - but that's what happened to me.

E.

#12 @imath
2 years ago

Hi @epgb101

Thanks a lot for sharing these inputs with us. You're very welcome to help us identify the potential blocks we'll find on our road, feel safe & free to contribute: we might woof hard but we don't bite 😅

@dcavins
2 years ago

Enable simple site privacy. Prevent access to BP pages, RSS feeds, REST API. Allow access to Registration and Activation screeens.

@dcavins
2 years ago

Location of option input in BP settings.

#13 @dcavins
2 years ago

I've added a first patch that adds simple privacy with filters so that plugin authors can include their screens in the scheme or so that users can allow specific items to be visited.

The REST API disabling seems pretty sad; it's an all-or-nothing routine. Hopefully someone who is more familiar with the REST API can suggest a better approach that would generally disable the content but allow for customization of what is available.

#14 @johnjamesjacoby
2 years ago

A few thoughts:

  • Try not to think about a future setting as "making a site private" but rather a "default community visibility"
  • Use bp_current_user_can() with some unique and new capability key
  • "Enable Private Site" should be something like "Community Access"
  • Checkbox on/off setting should be a filterable list (either a Select or Radios):
    • Public - Anyone: maps to exist cap: current_user_can( 'exist' )
    • Private - Registered Members - maps to is_user_logged_in() callback
    • (Custom) - added by plugins
  • New member registration setting should be nearby in the GUI to help users imagine what that flow is

Now is a good time to (re)consider the default value of this setting:

  • Public by default for upgrades
  • Private by default for new installations
  • Relative to if the community has open sign-ups, is invite-only, etc...

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


2 years ago

#16 @imath
2 years ago

Hi @dcavins

Thanks a lot for your work and for the patch. I've tested the patch, it behaves as you described. Although efficient, I believe redirecting the user to the login screen when they have no access to the community area is not very user friendly or not very welcoming.

Many thanks for your feedbacks @johnjamesjacoby I'll share a PR just after I've posted this comment that is following most of your suggestions (except: I've left new installs as "open" communities, but I agree we can consider it).

You'll see in the PR that we can use a different approach, leaving users to use the WordPress Block Editor to set the content of the page displayed to the not logged in member. In short if the current user does not have the mapped WP cap to a new bp_read cap, BuddyPress globals are not set and no community content or RSS feeds are generated. We stay in a regular WP Post.

If this post has no content, we can display a default one inviting the user to log in or register/request a membership.

NB: the private post status was a bad idea (I can say it was bad as I was the first one to have it!) as you need to be at least an Editor to view pages having this post status. I wonder why WordPress made this choice in the first place...

This ticket was mentioned in PR #25 on buddypress/buddypress by imath.


2 years ago
#17

  • Keywords has-patch added; needs-patch removed

The main idea is to unset the buddypress()->pages when the user hasn't the WP Cap mapped to a new bp_read one. As a result BP Globals like current_component, current_action and action_variables are not set and we end up into a ! is_buddypress() area for not logged in members when the community visibility option is set to members. Activity RSS feed are then not set. I believe if we use this scenario, we'll need to edit the BP Rest API endpoints to use the bp_current_user_can( 'bp_read' ) into the permissions_check methods.

A benefit of this is the site admin can easily customize the default content displayed to not logged in members using the Block Editor for the corresponding BP Pages.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/8734

#18 @johnjamesjacoby
2 years ago

NB: the private post status was a bad idea (I can say it was bad as I was the first one to have it!) as you need to be at least an Editor to view pages having this post status. I wonder why WordPress made this choice in the first place...

Oh. Yeah. Duh. Of course! LOL 🤣 And, I think even if we tried to be clever and use them, it may unintentionally be a confusing kind of override.

#19 @sbrajesh
2 years ago

Great job on this @imath .
I had a look at your patch and it seems the idea of unsetting the $bp->pages is not a good idea.

A private community does not mean it is not a BuddyPress community/page. It means that it is a BuddyPress page but inaccessible to the current user. In other words, we need an extra flag there.

So, all the things like current_action, current_component and pages should be set but the code that replaces the post content should know that the current page is private and either not replace the content with BP one or should provide a way to load a privacy template.

Another minor enhancement would be to either have dynamic visibility options( filterable array as suggested by JJJ above or have an action on the settings page where options are rendered in bp_admin_setting_callback_community_visibility to allow extending it.

I believe this is a great start for having a privacy for community feature. Keep it up.

#20 @imath
2 years ago

Thanks for your feedback @sbrajesh 😍. We can probably leave the $bp->pages set, I agree. For the other globals I would agree too if we were sure that all BP plugin developers would update their plugins to respect this new community settings. I’m afraid considering their participation to beta testing this is something we can’t be sure of. The other option is the one suggested by @dcavins. If you have another idea that makes sure à not logged in member won’t be able to access any plugin generated BP page, I’m totally open to it.

#21 @shawfactor
2 years ago

Whatever way you go on this is it must be possible to disable this functionality entirely (so that users like me can apply their own logic)

I run a Multisite with some components shared and others not. Sone components are private but some are not e.g. groups. I handle this via code atm and I’d like to continue doing so.

#22 @sbrajesh
2 years ago

Hi @imath
Thank you :)
I agree that plugin authors haven't been(I am guilty too) doing much.
Still, BuddyPress is the upstream and does not need to worry about downstream plugins. the onus should be on plugin authors to make sure the follow the updates of BuddyPress.

I like the approach by @dcavins. It allows to disable the privacy partially(exclude some section/pages) or disable it completely(@shawfactor has requested it). It can be updated to use the capability suggestion by jjj(your implementation in the last patch).

In other words, we can mix your addition of the 'bp_read' capability and use it on the hook 'bp_private_site_user_has_access' in @dcavins patch.

That will be very flexible at the same time will work for most of the setup without any issue. What do you think?

#23 @imath
2 years ago

David’s patch is very efficient I agree. My problem with it is, it’s throwing the user to the login page without more explanation and I believe (I can be wrong, of course), as Admin users often request more & easy customization, we should enjoy the post content of the corresponding page to let them use block widgets (BP Login form for instance) and any other block to attract users to engage into their private community. So it seems to satisfy both potential needs.

Disabling the community restriction can still be done easily in my patch filtering into the mapping caps function, remapping bp_read to exist

Another option, can be to leave the legacy parser do the parsing job to set constants then add a hook to let plugin shortcircuit a potential reset of this constant if the user is not loggedin. I believe it could satisfy Admin users need for « a better UI than the login form to welcome new members » and plugin authors ones 😅

Another option could be to use a specific directory to welcome these users using templates from template packs…

@sbrajesh we need to care a lot about plugins & themes, we think about them almost each time we’re adding a new feature, end users don’t see a difference between BuddyPress and BuddyPress plugins/themes, it just doesn’t work for them and they don’t have time to find what is wrong, they move to another community software. That’s why we always put complicated backcompat code and why we are now using the feature as a plugin model to try to prevent potential issues with plugins (unfortunately not many plugin authors are testing them, but that’s another story 😇)

That being said, many thanks to you and @shawfactor for contributing to this ticket 😍

#24 @johnjamesjacoby
2 years ago

Perhaps we scope-creep this to include a redirection destination, with accompanying filter and/or Page setting, and/or dynamic block template or part(s) pattern to add to the locator?

In other words, an officially documented and preferred approach to split which template parts get located based on the logged in users’ ability to view them?

#25 @dcavins
2 years ago

I like the idea of the bp_read capability. To make it easier to tune the behevior of (in plugins or other code), what if we add a context parameter to it so that you know which capability is being tested. Like current_user_can( 'bp_read', array( 'context' => 'bp_rest_members_get_items_permissions_check' ) )
or maybe

current_user_can( 'bp_read', array( 
	'context' => array( 
		'primary' => 'bp_rest',
		'secondary' => 'members_get_items_permissions_check' 
	)
)

so that you could easily generally allow or deny bp_rest for instance. or maybe the component is the key.

current_user_can( 'bp_read', array( 
	'context' => array(
		'component' => 'members',
		'interface' => 'bp_rest',
		'capability' => 'members_get_items_permissions_check' 
	)
)

I'm not sure what properties to send, but having a central, easy-to-control logic for this would be useful.

#26 @sbrajesh
2 years ago

Thank you @imath
@johnjamesjacoby has a an excellent suggestion which will help site admins use any of the approach(even if that happens via plugins/code). They can go for redirect to login or show the page with login/register message(which is fine if you prefer it that way for default).

My issue with current patch is while bp_read is a good addition, It does not say anything about the current context. If the page/component/actions are not available, there is no way to modify it for a certain directory page(Say, we try to open member directory but keep everything else private).

It will be nice to have a solution that provides context. bp_read is good and it might have it benefits on the REST API as suggested above as all the routes are managed by BuddyPress(core) and any routes added by external plugin will have their own way of dealing with it(BuddyPress won't be affecting them).


#27 @imath
2 years ago

Hi @dcavins @sbrajesh & @johnjamesjacoby

Sorry for this late reply! I was pretty busy this week.

I've improved the initial PR trying to take in account as much feedbacks as possible.

  1. Being able to "bypass" the capability check. Context information are now sent and plugin authors can use it to decide whether they should bypass it or not. Here's a code example to allow the members directory to be viewable by any user.
  2. I've used @dcavins first idea about redirecting the user to the login page when the BP Theme or BP Template pack does not include the members/gate.php template. For this case, I've improved the bp_core_no_access... function so that it's possible to add an error message more suitable for the context. You can test this fallback activating BP Default or the BP Legacy template pack.

@dcavins about your suggestion, I believe these caps should be dealt within the components classes, we could add a filter into the BP_Component class they are extending, this way, they'll be able to manage their specific contexts.

Last edited 2 years ago by imath (previous) (diff)

#28 @johnjamesjacoby
2 years ago

what if we add a context parameter to it so that you know which capability is being tested

I think, the conventional way to do this, would be to invent a naming pattern for unique capability names, and map all of them down to bp_read via map_meta_cap: I.E. “prefix_verb_component_action_subaction(plurality)” = bp_get_group_members_invites

In this way, the cap keys are registered, buildable, and predictable?

Then, I think, anything that is considered “contextual” for the can either pass in a single ID or nothing, to continue to mimic WordPress and just about every other plugin, and would be accessible via some bp() value?

(I’m guess, I am reluctant to introduce a totally new usage pattern into cap parameters, from a healthy fear of them being implemented in a way that allows members to access things they shouldn’t be able to.)

Very open to feedback on these ideas. Never intending to halt progress. Only thinking out loud. 💕

#29 @sbrajesh
2 years ago

Hi @imath
Thank you for the work on it. It is going in the right direction. I have a few suggestions and will love to hear your feedback.

  1. As @johnjamesjacoby points, the context parameter(non id) seems a bit out of the place there. We should avoid it.
  2. It would be nice to have a function similar to the function bp_user_has_access() which provides a simple filter, context and uses bp_read cap.
  1. Also, I am not sure if introducing additional complexity is required. We can let the original parsing work as it did earlier. Then, we simply hook to 'bp_replace_the_content' and if the community is not visible, we just discard any other hooked content and load the community gate template.

Just some thoughts to keep it simple. It is alright if you decide to go with the current implementation.

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


2 years ago

#31 @imath
2 years ago

  • Keywords early added
  • Milestone changed from 11.0.0 to Up Next

Let's take some more time to think about it, for now I'll move this ticket to next release.

#32 @imath
21 months ago

  • Milestone changed from Up Next to 12.0.0

#33 @dcavins
17 months ago

With new rewrite code, refer to https://github.com/buddypress/bp-rewrites/blob/trunk/inc/functions.php#L369%7CL438 for example of "no access" step.

@imath commented on PR #25:


17 months ago
#34

A new & improved way of dealing with this visibility feature will be soon worked on.

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


17 months ago

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


14 months ago

#37 @dcavins
14 months ago

I'm attaching a new version of the patch that blends the work in the earlier patch with the work that @imath did in the bp-rewrites plugin. It's surprisingly straightforward. I will also attach a screenshot of the proposed basic admin setting form that allows the admin to set every BP area to private or to choose specific components to protect. Please give it a try and let me know how it can be improved. There are some @TODOs in the patch to remind me to think about things, so if you have thoughts on those items, please let me know.

@dcavins
14 months ago

Beta site privacy patch, now calles "community visibility".

@dcavins
14 months ago

Community visibility settings pane.

#38 @imath
14 months ago

Hi @dcavins

Thanks a lot for your work on this, the screenshot looks great. I'll have a deeper look at the patch asap.

#39 @imath
14 months ago

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

  1. Some 'bp-rewrites' textdomains need to be replaced by 'buddypress'.
  1. 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
    
  1. 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 😍

#40 @dcavins
14 months ago

Hi @imath, thanks for your review!

Yes, I didn't go down the post_status road, because I've had surprising experiences with non-core post statuses in other plugins, that made me use a simple option instead. That's not to say we can't use the posts in the future. As you said, though, it would be inconsistent because some items/components map to pages and some do not.

I've integrated your feedback patch and moved things around so that the code is scattered here and there in core files and relevant places. I'll also add a PR against the BP-REST git repo that adds user_can checks in the permissions checks there.

I have left the "bp_component" argument in place in the bp_current_user_can( 'bp_view' ) checks. Even though we are not currently using them, they would be useful if you wanted to modify the behavior using the user_can filters.

I have also left the option formatted as though we are tracking visibility for different sections, though I am only interested in the global array entry for now. This will make future expansion more predictable.

Thanks again!

Last edited 14 months ago by dcavins (previous) (diff)

@dcavins
14 months ago

Return to simple option for community visibility.

#41 @imath
14 months ago

  • Keywords commit added; early removed

Hi @dcavins !

I've just tested the patch. It works great. Thanks a lot for your work/reactivity about it. I only have a few last change requests before you can commit it:

  1. This feature requires the BP Rewrites API, so we need to add some 'rewrites' === bp_core_get_query_parser() checks before displaying the setting & inside the bp_get_community_visibility() function.
  2. We should add a default option value for new installs + add an upgrade routine (which made me notices the legacy widgets clean-up should be out of the post type conditional).
  3. We need to force the $query->is_page value to true otherwise some themes like Twenty-Twenty Three are adding additional content like "Posted by ... on ...".
  4. Activity feeds: all except 1 are personal feeds, so the logged-in user would access to potential activities about friends, groups, etc.. So we don't need to check for other components visibility.
  5. Last one is a detail, we should use another inline docblock description than something about private site.

You'll find this 5 change requests in the new complementary patch: 8734.03.feedbacks.patch. Once applied/improved, let's have this in!

#42 @shawfactor
14 months ago

Guy following up on whether there is a filter to turn this toggle off and revert to the previous handling.

It won’t work for my setup and I can think of many others where toggles like this are inappropriate.

Can you please advise this is part of the scope?

#43 @dcavins
14 months ago

Thanks @imath for the review!

Yes, @shawfactor if you do nothing, your site will be open as before. The default behavior is that nothing changes (the community area is open to be viewed by non-logged-in users). As a site admin, you can toggle on the private community visibility if you wish.

#44 @shawfactor
14 months ago

That not what I’m asking. I don’t want the option at all. I’m running a hosted buddypress environment. I don’t want this to even be an option. I want a filter to deactivate the toggle entirely

Yes I can code up an override but then I’m going to have to somehow remove the UI as well to prevent confusion.

More particularly this won’t be just a problem for me. AFAICT the current solution does not handle edge cases where part of a component is publis and part is not. That is a real problem people should be able to easily roll there own 100% code based solution based on the WordPress decisions not options philosophy.

Pete

#45 @dcavins
14 months ago

Hi @shawfactor Sure we can put a filter around the setting being added to the form. You can also filter the bp_view cap to be sure that it always returns true to prevent any visibility code from taking effect (it would be a one-liner).

You are correct that the current version of the functionality lays the groundwork for future, more granular control, but, with this release, just makes the community part of a web site inaccessible to anonymous users. If you have great ideas for how to manage granular control in a way that avoids edge cases, please contribute to this ticket.

Thanks!

#46 @shawfactor
14 months ago

That sounds messy but I’ll find a way to make it work I guess. As I said ideally the whole functionality should be able to removed by a simple filter. Not only will the proposed way break my SAAS setup but those who’ve installed my plugin LH Private Buddypress will now need to deactivate it and toggle sone unnecessary options

Granular access is far better accommodated through code (plugins) rather than options. Trying to build core options will just make the average buddypress implementation even more of a mess.

#47 @dcavins
14 months ago

@shawfactor Great! It won't be messy, and you'll be fine.

It sounds like you have some good experience with privacy, so I'll look forward to your contributions!

#48 @shawfactor
14 months ago

You misunderstand me. I think building an options framework for privacy at a granular level is both difficult and a mistake. Somethings are better set at a code level as separate plugins.

Anyway as long as there are filters I can hack up a solution that works for me.

#49 @dcavins
14 months ago

@shawfactor: Here's one way to prevent the new settings field from being built:

add_action( 'bp_register_admin_settings', 'remover_bp_community_visibility_admin_settings', 14 );
function remover_bp_community_visibility_admin_settings() {
	global $wp_settings_fields;

	unregister_setting( 'buddypress', '_bp_community_visibility' );
	if ( isset( $wp_settings_fields[ 'buddypress' ][ 'bp_main' ][ '_bp_community_visibility' ] ) ) {
		unset( $wp_settings_fields[ 'buddypress' ][ 'bp_main' ][ '_bp_community_visibility' ] );
	}
}

It's simple enough that I don't think we need to add another apply_filters() in BP's register_admin_settings() setup.

#50 @imath
14 months ago

@dcavins thanks a lot for taking the time to explain how to disable the setting. I know the decision we took to introduce this feature caused some insatisfaction. We've been discussing about it for almost a year, it's now time to move on and finally have this in as to end-users this is a very basic feature: feedbacks we got showed it.

#51 @dcavins
14 months ago

In 13531:

Introduce bp_view capability.

Introduce a new, base-level capability, bp_view,
that by default maps to the most base-level WordPress
capability, exist.

See #8734.

#52 @dcavins
14 months ago

In 13532:

Prepare for community visibility.

Reset some code changes in preparation for the new
community visibility feature.
Introduce bp_core_get_component_from_directory_page_id().

Props imath.

See #8734.

#53 @dcavins
14 months ago

  • Owner set to dcavins
  • Resolution set to fixed
  • Status changed from assigned to closed

In 13533:

Introduce basic community visibility feature.

Site admins now have the option to restrict access to the
community portion of a BuddyPress site. If the site admin
chooses "members only," then, when a non-logged-in
user attempts to visit a BuddyPress screen, she will
be shown an access error message and a log-in form.

Props imath, sbrajesh, jjj, shawfactor.

Fixes #8734.

#54 @dcavins
14 months ago

In 13540:

Update bp_view capability check.

Ensure that the bp_view capability check is using the
correct structure when checking the $args variable .

See #8734.

This ticket was mentioned in PR #155 on buddypress/buddypress by dcavins.


13 months ago
#55

Use this filter to send the user to the site login screen when the user does not have the bp_view capability for the current screen or situation. The default behavior is for the user to be shown the content in the assets/utils/restricted-access-message.php file.

Trac ticket: #8734

dcavins commented on PR #155:


13 months ago
#56

Shall I merge it?

@imath commented on PR #155:


13 months ago
#57

Yes, we never merged PR here but rather commit on our SVN repo 😉. It will be sync here 👍.

#58 @dcavins
13 months ago

In 13556:

Introduce bp_view_no_access_redirect_to_login_screen filter.

Use this new filter to send the user to the site login screen
when the user does not have the bp_view capability for the
current screen or situation. The default behavior is for the
user to be shown the content in the
assets/utils/restricted-access-message.php file.

See #8734.

#60 follow-up: @epgb101
13 months ago

Hello.
Do you still plan to add something like this to main BuddyPress plugin in near future: https://buddypress.trac.wordpress.org/attachment/ticket/8734/community-visibility-settings.png - or are you opting for making either all member content private or all visible? Reading the posts but I'm not that technical. This granular ability would be a dream for me but I can see you have to weigh up core v plugin features. Thank you.

Last edited 13 months ago by epgb101 (previous) (diff)

#61 follow-up: @imath
13 months ago

Hi @epgb101 we’ll start with a global setting to set the whole community visibility to public or restricted to members. Then we’ll make it more granular on a component basis.

#62 in reply to: ↑ 60 ; follow-up: @dcavins
13 months ago

Replying to epgb101:

Hello.
Do you still plan to add something like this to main BuddyPress plugin in near future: https://buddypress.trac.wordpress.org/attachment/ticket/8734/community-visibility-settings.png - or are you opting for making either all member content private or all visible? Reading the posts but I'm not that technical. This granular ability would be a dream for me but I can see you have to weigh up core v plugin features. Thank you.

Can you tell us how you would use the feature? Thanks!

#63 in reply to: ↑ 61 @epgb101
13 months ago

Replying to imath:

Hi @epgb101 we’ll start with a global setting to set the whole community visibility to public or restricted to members. Then we’ll make it more granular on a component basis.

Either way is great news to me. Will it be a little while before it's part of BP plugin?

#64 in reply to: ↑ 62 @epgb101
13 months ago

Replying to dcavins:

Replying to epgb101:

Hello.
Do you still plan to add something like this to main BuddyPress plugin in near future: https://buddypress.trac.wordpress.org/attachment/ticket/8734/community-visibility-settings.png - or are you opting for making either all member content private or all visible? Reading the posts but I'm not that technical. This granular ability would be a dream for me but I can see you have to weigh up core v plugin features. Thank you.

Can you tell us how you would use the feature? Thanks!

I think a general all-private/all-public toggle would be great for most sites needs - certainly for my BP membership sites mainly to; a) protect users from having their profile data/images picked up by web search etc b) deter spammers c) I don't have to worry about third-party private plugins conflicting, or not being updated.

Re granular.. priority public areas for me would be Groups and Profiles (if poss at some point with limited info showing - eg name/generalised location) as it seems in this day and age much of user's data/activity feeds should be private by default? I'm sure there are exceptions (like this forum where activity feed is not risky). Just my thoughts.

I read comments that private/public feature should not be baked into BP - but as BP is a networking plugin used by all kinds of public and business (often sensitive) communities it does seem to me like it should be part of basic plugin. Apologies if showing ignorance to developers as I am really just a plugin user with limited plugin dev and coding knowledge.

PS nice if a general private/public toggle worked on multisite :)

Note: See TracTickets for help on using tickets.