Basic profile privacy

[boonebgorges]

During our last dev chat, cnorris23 showed interest in doing some very simple profile privacy. What should this look like? Jeff Sayre's plugin is nice but is pretty heavy-duty - it touches all components, requires its own db tables, etc - which suggests that that level of control is perhaps best left to a plugin for the moment. (At the very least, we should implement in stages.)

I'm imagining something like: Each user can set profile privacy on a per-profile, per-profile-group, or per-profile-field setting. Possible settings may include, for now: everyone, logged-in members only, my friends.

It's possible to strip this idea down even further for a first implementation - for instance, leaving out the 'friends' integration, or allowing less fine-grained control (maybe restrict by group instead of by field). But I think that this strips it down a little too far.

Even this modest implementation will touch a variety of places (xprofile queries, stuff like that) so we should sketch it conceptually as a group before getting too deep in code.

[cnorris23]

I went ahead and assigned myself to #1995, but I'll leave it up to you core devs on whether or not to close that one as a duplicate. I didn't think about adding groups as a parameter, but that shouldn't be too difficult. I just need to add admin code to allow people to choose who can view. I've kept if very basic, and added filters to allow plugin devs to get more fine grained. Here's an outline of what I have so far

1. Site admins can set defaults on a per field basis
2. Users can set privacy on a per field basis with the options: everyone, logged in users, friends, no one, and now groups
3. Privacy settings are saved in Xprofile meta

I haven't done anything so far to allow users to restrict a whole profile or profile group. Per boone's suggestion in the dev chat, I kept it very basic. As stated above, code would have to be touched in a variety of places to make it work. I stuck with just profile fields as that only touches xprofile code, and it would be the same for profile groups. I think profile groups/fields would be the most realistic for a first implementation. I don't think restricting a whole profile would be that difficult, but it would reach further into the code. From what boone has stated above, and what I've coded so far, I think we're pretty much on the same page :). I hope to have the code up tonight, so we can talk about it in tomorrow's dev chat if needed.

[boonebgorges]

Awesome.

Let's leave #1995 open as a separate ticket for now, because its scope is wider than the scope here. In particular, it suggests mods to the activity stream and other stuff.

Looking forward to seeing what you've got :)

[cnorris23]

Okay, so first off, I just want to say that this is rough. The functionality is there, but it needs a spit-n-shine. Hopefully more shine than spit ;) The UI is incredibly rough. The biggest thing as of now as far as UI: you can select every option. Which means you can have both "everyone" and "no one" selected. There's also redundancy in cases where you have friends or groups or both selected, and then you also have logged in users selected. Keep that in mind as you test, especially if you start getting unexpected results.

Limiting profile groups got a little more involved than my self-imposed deadline would allow, so I've left it out for now. Also, I didn't get the UI/functions to allow site admins to set defaults, but again, it came down to just wanting to get the framework out before the dev chat.

[boonebgorges]

Great start! Thanks for giving us something to work with. Some thoughts:

1) For efficiency's sake, we should probably load privacy levels into $profile_template. Probably introduce some sort of get_profile_data_extras() method in BP_XProfile_ProfileData, along the lines of what we do in BP_Core_User. Then bp_profile_field_is_private() will try to get it from there first. Matters more on the public profile than on the edit page.

2) Re: UI fields. I suggest dropping No One (having none checked would be the equivalent), and adding some JS that makes checking Everyone select the others, and deselecting any of the others uncheck Everyone.

3) Might want to consider moving the per-field privacy options out from under the individual fields, and into a collapsible "privacy" section on the edit screen. Might make things less cluttered.

4) Hiding fields as late in the game as you do (ie in the template itself) means that zebra-striping is broken. If we move up the chain a bit, either as a filter on bp_has_profile or in the query itself, we won't have this problem (though that is, of course, more complicated, and may interfere with my suggestion in (1) about $profile_template). For reference, see the way that bp_has_profile()'s show_empty_fields works.

5) Something seems to be broken, so I'm getting errors when I save my profile; also, when I set privacy level for one field, it's getting applied to all the other fields being saved as well. Something to fix in the spit-n-shine :)

Anyway, these are all initial thoughts from looking at the code for just a few minutes. Would like to hear more from cnorris23 and the other core devs.

[cnorris23]

Replying to boonebgorges:

1) For efficiency's sake, we should probably load privacy levels into $profile_template. Probably introduce some sort of get_profile_data_extras() method in BP_XProfile_ProfileData, along the lines of what we do in BP_Core_User. Then bp_profile_field_is_private() will try to get it from there first. Matters more on the public profile than on the edit page.

That sounds good to me. I like efficiency!

2) Re: UI fields. I suggest dropping No One (having none checked would be the equivalent), and adding some JS that makes checking Everyone select the others, and deselecting any of the others uncheck Everyone.

Again, sounds good to me.

3) Might want to consider moving the per-field privacy options out from under the individual fields, and into a collapsible "privacy" section on the edit screen. Might make things less cluttered.

I think I know what you're saying. If I do, then I like it. Maybe you could expound a little to be sure we're on the same page?

4) Hiding fields as late in the game as you do (ie in the template itself) means that zebra-striping is broken. If we move up the chain a bit, either as a filter on bp_has_profile or in the query itself, we won't have this problem (though that is, of course, more complicated, and may interfere with my suggestion in (1) about $profile_template). For reference, see the way that bp_has_profile()'s show_empty_fields works.

I'll take a look at that, and maybe some others could weigh in too. I wasn't happy with the current implementation, but I started low in the chain because it was an easy place to verify functionality with little effort. I ended up having less time over the past week than I had originally thought, so you're seeing code that I wouldn't normally release into the wild :(

5) Something seems to be broken, so I'm getting errors when I save my profile; also, when I set privacy level for one field, it's getting applied to all the other fields being saved as well. Something to fix in the spit-n-shine :)

I did notice some wonkiness, but I didn't see the issue you're having, specifically. The biggest issue I saw was that you'll get an error message if you try to save a profile group where no changes were made to the privacy settings. I'll take a look at that too.

[InterMike]

Another great suggestion: An optional "Except these members:" checkbox textfield for all the privacy options where you can select (type in) members you want excluded from the selected privacy rule (otherwise the privacy rule is applied to everyone, even if you may have just one person you don't want it to).

For example: For my "Fav Beer Brand" profile field, I select "Friends", but I don't want my mom to see it, so I exclude her from the rule, thus not allowing her to view it (lol, sorry, first example that came to my mind). This would eliminate the need of a another plugin for members blocking members, once/if "profile blocking" is added. Facebook has something like this, too, I believe.

[boonebgorges]

We had some discussions about this in last week's dev chat. Some takeaways:

• My suggestion (1) above is really a necessity. The patch currently requires another database hit for every profile field, which will not scale well.
• Paul's main concern (which I share) is that we are compatible with any future move that WP might make to using WP caps/current_user_can(). My understanding of WP caps is incomplete (to say the least), but from what I do get, you can filter map_meta_cap with whatever data you want, which means that your suggested method should work fine. But in any case, before considering for inclusion in BP core, I think it's crucial that we either do a proof-of-concept to show that we are able to do the necessary mapping down the road, *or* that we actually rewrite cnorris23's patch to use current_user_can(). In this way, this privacy stuff could be a testing ground for the future migration of BP caps (which are currently all over the place) to a more organized WP caps schema
• It probably makes sense to plugin-first with this, since time is getting short on this dev cycle. It should be fairly easy to wrap into a plugin shell. If you need any filters in xprofile to make it possible for such a plugin to work with 1.6, please don't hesitate to ask.

If I get a chance myself, I can start repatching in accordance with these notes, though I'm pretty booked for the very near future.

[boonebgorges]

A client of mine wants this done, so I've taken another look.

After some discussion with Paul and John, and taking some inspiration from cnorris23's original patch, I had another go at this. See 3695.002.patch.

Some architectural decisions of note:

• Profile privacy itself works by excluding certain fields from the BP_XProfile_Group::get() query. This is done inside the query method so that you can still pass in exclude_fields parameters to bp_has_profile() without accidentally bypassing privacy settings.
• Everything is accomplished with, at most, a single additional query - for the displayed_user's usermeta bp_xprofile_privacy_levels.
• There's basic infrastructure (in the form of filters) in place to extend with custom privacy settings. Register your privacy levels at bp_xprofile_get_privacy_levels, and control which fields are excluded from the query with bp_xprofile_get_hidden_fields_for_user.
• Supported out of the box: Public, Logged In Users, My Friends
• I wrote a small bit of javascript (with no-js fallback) for the profile editing section. I think the UI is passable.
• If you are using a theme with a custom version of members/single/profile/edit.php, you will not have the privacy setting markup. That means that all of your profile fields will fall back to the admin-set default privacy setting for that field (set on the Dashboard panel where you edit individual profile fields); if no admin-defined default is provided, falls back on public. In other words, it degrades nicely.

Would love to have a few sets of eyes on it. Thanks.

[DJPaul]

Tested this a bit. Initial thought is that you commit this so we can iterate. It's a lot more compact than what I feared.

• Maybe call this visibility instead of privacy.
• If user's profile field value is emptied, and the privacy option is changed at the same time, the empty field is to the database, but the privacy option isn't updated.
• With the new bit of javascript, use jQuery 1.7's $.on() to bind the click event (I can't see there's much point in updating the rest of the javascript as it's backwards compatible, but may as well get any new javascript correct).
• Add a class property for "privacy_levels" to BP_XProfile_Component or BP_Component.
• Hardcoded html in bp_profile_privacy_radio_buttons(), not sure how best to work around that.
• $fetch_privacy_level_default in bp_has_profile(). If, for the sake of argument, someone had a custom widget that displayed profile data of a specific user, would that sort of use case be handled? Or some sort of profile field RSS feed?
• Maybe use wp_parse_id_list() to sanitise integer arrays.

[boonebgorges]

(In [5789]) First pass at per-field visibility/privacy for XProfile:

• Allows admins to set default levels for specific fields
• Allows users to set visibility/privacy on a field-by-field basis
• Modifies the profile templates to show markup for editing visibility status

See #3695

[boonebgorges]

(In [5791]) Use .on() for profile privacy jQuery. See #3695. Props DJPaul.

[boonebgorges]

(In [5792]) Changes 'privacy' to 'visibility' throughout, when referencing xprofile fields. See #3695. Props DJPaul.

[boonebgorges]

(In [5793]) Skip check for profiledata when saving visibility level (this was a remnant of an earlier implementation). See #3695

[boonebgorges]

(In [5794]) Use wp_parse_id_list() for parsing exclude_fields parameter of BP_XProfile_Group::get(). See #3695. Props DJPaul.

[boonebgorges]

DJPaul - Thanks for your very helpful feedback. Most of your issue have been fixed.

Hardcoded html in bp_profile_privacy_radio_buttons(), not sure how best to work around that.

$fetch_privacy_level_default in bp_has_profile(). If, for the sake of argument, someone had a custom widget that displayed profile data of a specific user, would that sort of use case be handled? Or some sort of profile field RSS feed?

All that means is that there'll be an extra query involved. Just a bit of extra overhead that I engineered to avoid.

Hardcoded html in bp_profile_privacy_radio_buttons(), not sure how best to work around that.

I thought about putting it right into the template, but it was ugly. For form elements, I think that outputting markup is not the worst thing in the world, especially if it's filtered (which this is) and if there is an alternative way to get the required raw data (which there is - bp_xprofile_get_visibility_levels()). So I'm suggesting we leave it as is.

Marking this ticket resolved. Specific issues can get separate tickets.

[r-a-y]

Glad to see something like this for BP 1.6!

Just glancing the code a bit. Should get/update_user_meta() be replaced with bp_get/update_user_meta()?

[boonebgorges]

Good call, r-a-y - can you open a ticket and/or patch?

[r-a-y]

I've opened a ticket - #4013. If someone doesn't beat me to it, I'll add a patch.

[hnla]

Had a quick play with this and it's a really great addition cheers for adding it Boone.

Haven't looked under the hood as such so may be talking rubbish here but this works by setting 'bp_profile_group_has_fields()' to false? If so does that not feel odd as this field does have data so is in effect 'true' is there no issue that may arise if people have used that function for any reason and expect it to return true

[boonebgorges]

hnla - Glad it's useful!

bp_profile_group_has_fields() is not touched in this implementation. I reach deeper into the database query and prevent hidden-for-this-user fields from being returned in the first place. Then bp_profile_group_has_fields() behaves as normal.

These changes should be totally backward compatible. If existing themes do not support the markup for changing the field visibility, then it will default to public. And all existing use of the bp_has_profile() stack (including bp_profile_group_has_fields()) will continue to work just as before.

[hnla]

Was wondering about the display none mentioned earlier and whether it isn't a useful option especially from admin end of things.

I've been asked to do a little customisation to my map plugin which gets the profile field set by the user i.e 'location' and uses the value to set map address. The 'client' is using a single field for mapping address string and a series of fields for 'nice' display.

So ideally for them would be an option on the admin settings profile field to set it to 'display none' and have it enforceable but still editable by user.

So we have a form of pseudo meta data that can be set from users profile but that has it's public display removed.

[boonebgorges]

hnla - I promised at ​https://bpdevel.wordpress.com/2012/03/16/profile-field-visibility-in-bp-1-6/ to make a sample plugin that shows how to add/remove privacy levels. I will make an 'Admins Only' option as one of the examples. My thought is that this is enough of an edge case to be plugin territory.

[hnla]

I think I tend to agree that's its an edge case too; I was slightly throwing it out there as I saw mention of this type of option at the beginning of the ticket.

I also have always had a moral dilemma over my notion that one can use the profile fields for a form of options table for an easy route to adding data to retrieve at some point.

My only slight objection to the plugin route is when one talks about privacy, visibility, access etc in general terms that it's a somewhat fundamental core function not something that should be handled by third party code.

[boonebgorges]

My only slight objection to the plugin route is when one talks about privacy, visibility, access etc in general terms that it's a somewhat fundamental core function not something that should be handled by third party code.

There are several reasons why I'm not swayed by this, but the foremost is that an "Admins Only" visibility setting is not really a "privacy" setting at all, but is really a tool for admins to hack around building usermeta input (as you suggest).