Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 4 years ago

#4728 closed enhancement (maybelater)

Visibility API refactoring/restructuring

Reported by: chrisclayton Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch, needs-testing, trac-tidy-2018
Cc: chrisclayton

Description

We need to move some of the general Visibility code ( eg. bp_xprofile_get_visibility_levels() ) out of bp-xprofile and into bp-core so that we can extend the visibility stuff to other components in the future without worrying about whether the Profile component is active and not rewriting the same code into each component.

It might also be worth considering moving it into a seperate component (bp-visibility) so that visibility/privacy can be turned off/on depending on site requirements.

Attachments (2)

4728.01.patch (6.2 KB) - added by r-a-y 7 years ago.
4728.02.patch (7.4 KB) - added by r-a-y 6 years ago.

Download all attachments as: .zip

Change History (16)

#1 follow-up: @DJPaul
9 years ago

  • Keywords reporter-feedback added

Where else would the existing activity visibility code be used?

#2 in reply to: ↑ 1 @chrisclayton
9 years ago

Replying to DJPaul:

Where else would the existing activity visibility code be used?

I'm currently working on a plugin (which I'm hoping to make good enough for a core patch) that mimic (kinda) the functionality of facebooks activity privacy while still having the same feel as the implementation used in xprofile. Eg. You'd have a drop down on the post form with Visibility levels, user attaches a level to the activity, the activity only shows to those users (this part looks like it would be dependent on some of Boones hide_sitewide tickets).

I know there are a lot of people who want privacy extended to other components and i think its smarter if we use a generalised API, rather than building from scratch for individual components as many functions can be reused and filtered by the individual component when appropriate.

Edit: We can assume most components will want the same, logged in, friends only, public etc levels, can't we? and if a component needs extra they can filter it in. For example, the code used in Boones example (https://github.com/boonebgorges/bbg-custom-bp-field-visibility/blob/master/bbg-custom-bp-field-visibility.php) will add/remove the level for all components (we could possibly add component to the array to enable devs to easily modify the levels for an individual component.)

Last edited 9 years ago by chrisclayton (previous) (diff)

#3 @chrisclayton
9 years ago

  • Cc chrisclayton added
  • Keywords 2nd-opinion added; reporter-feedback removed

#4 @chrisclayton
9 years ago

As an example to clarify what I'm saying, the way it is currently - we'll end up with this (modified from Boones example plugin - link above) in plugins when we start building privacy in other components...

     // Add the levels to all components
add_filter( 'bp_xprofile_get_visibility_levels', array( &$this, 'modify_levels' ) );
add_filter( 'bp_activity_get_visibility_levels', array( &$this, 'modify_levels' ) );
add_filter( 'bp_groups_get_visibility_levels', array( &$this, 'modify_levels' ) );
     // the example plugin had to build its thing from scratch and decided to create a fun name
     // it probably changed the array key too, meaning this won't work. 
     // Or, forgot to add a filter.
     // or, built it completely differently, Probably hardcoded the levels in making it impossible to add new levels
add_filter( 'bp_example_awesomesauce_privacy_sauce', array( &$this, 'modify_levels' ) );

	function modify_levels( $levels ) {
		// Remove the 'friends' level, if it exists
		if ( isset( $levels['friends'] ) ) {
			unset( $levels['friends'] );
		}

		// Add a new 'Admins Only' level
		if ( !isset( $levels['admins-only'] ) ) {
			$levels['admins-only'] = array(
				'id' => 'admins-only',
				'label' => __( 'Admins Only', 'textdomain' )
			);
		}

		return $levels;
	}

When it should be something like

add_filter( 'bp_core_get_visibility_levels', array( &$this, 'modify_levels' ) );

	function modify_levels( $levels ) {
		// Remove the 'friends' level, if it exists
		if ( isset( $levels['friends'] ) ) {
			unset( $levels['friends'] );
		}

		// Add a new 'Admins Only' level
		if ( !isset( $levels['admins-only'] ) ) {
			$levels['admins-only'] = array(
				'id' => 'admins-only',
				'label' => __( 'Admins Only', 'textdomain' )
			);
		}

		return $levels;
	}

And then the individual components that need that info will get that info from core.
We need to make it super easy to add visibility to plugins and other components by talking to bp-core instead of writing from scratch (which is the only option at the moment since everything, including globals rely on xprofiles being active)

Last edited 9 years ago by chrisclayton (previous) (diff)

#5 @DJPaul
9 years ago

  • Keywords reporter-feedback close added

I see, but I believe the xprofile visibility settings are so specific, they wouldn't work without a lot of adjustments for other types of data. If we were to building visibility controls into each component, I agree that we'd definitely build something universal and put it into a common component.

#6 @boonebgorges
9 years ago

  • Keywords close removed
  • Milestone changed from Awaiting Review to Future Release

but I believe the xprofile visibility settings are so specific, they wouldn't work without a lot of adjustments for other types of data.

+1. I'm totally behind the idea of working toward privacy/visibility features that will work across more content types, but just porting over the stuff from the xprofile component won't work. (We could keep the levels - 'loggedin', 'friends', etc, but that's about it.)

I'm going to put this ticket in Future Release, where we can use it for discussion of what a more general privacy schema might look like. It'll probably be the kind of thing that gets implemented in a plugin first, with some prerequisite mods made to BP to handle some of the fancy filtering that'll be necessary. You might have a look at Jeff Sayre's bp-privacy component as a starting point.

#7 @r-a-y
7 years ago

  • Keywords 2nd-opinion removed

I'm totally behind the idea of working toward privacy/visibility features that will work across more content types, but just porting over the stuff from the xprofile component won't work. (We could keep the levels - 'loggedin', 'friends', etc, but that's about it.)

I think the comments by Chris about extrapolating the default levels from XProfile is valid. This is so plugins can work from a consistent set of levels instead of having to define their own custom ones.

Visibility userdata can be handled by plugins and can follow how XProfile is doing it with usermeta.

Going to work on a patch just for levels.

@r-a-y
7 years ago

#8 @r-a-y
7 years ago

  • Keywords has-patch added; reporter-feedback removed

01.patch extrapolates the visibility levels from XProfile to Core and adds a basic API to register a visibility level. Registration should occur on the 'bp_setup_actions' hook or before.

Feedback appreciated.

#9 @boonebgorges
7 years ago

I like this a lot, r-a-y. Thanks!

A couple technical thoughts:

  • I think we should probably leave (a reference to) the visibility levels at buddypress()->profile->visibility_levels for backward compatibility.
  • Instead of overloading buddypress()->visibility_levels with the if ( empty( $bp->core->visibility_levels ) ) check in bp_register_visibility_level(), how about declaring the $visibility_levels property as an array in the class definition of BP_Core_Component?
  • I don't see any reason why we should prevent people from deregistering core visibility levels. It will break lots of things, but as you note, the filter is there anyway. Let's solve that problem if it actually turns out to be one.

@r-a-y
6 years ago

#10 @r-a-y
6 years ago

Thanks for the feedback, boonebgorges.

02.patch is updated based on those thoughts.

The only thing I'm worried about is if plugins are registering new levels and a site disables one of those plugins after a user has saved one of these non-core visibility levels as a particular option.

There needs to be some form of fallback to drop to a core level in these cases. Need to think about this some more.

#11 @boonebgorges
6 years ago

The only thing I'm worried about is if plugins are registering new levels and a site disables one of those plugins after a user has saved one of these non-core visibility levels as a particular option.

Yeah. This reminds me of https://core.trac.wordpress.org/ticket/16956#comment:34.

#12 @DJPaul
6 years ago

  • Keywords needs-testing added

#13 @DJPaul
4 years ago

  • Keywords trac-tidy-2018 added

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/

#14 @DJPaul
4 years ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.