#6344 closed defect (bug) (wontfix)
Visibility levels: naming
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Core | Keywords: | |
Cc: | sascha@… |
Description
Hi there,
I explored adding a visibility level "admin_only" to the range of visibility levels according to https://github.com/boonebgorges/bbg-custom-bp-field-visibility
That works fine. But I came across this in the core bp-xprofile-loader:
'adminsonly' => array( 'id' => 'adminsonly', 'label' => _x( 'Only Me', 'Visibility level setting', 'buddypress' ) ),
I find the naming here is just asking for confusion - especially if you want to add some fields that are truely admin_only (i.e latitude/longitude data etc). Could it be changed to something like this:
'admins_and_me' => array( 'id' => 'admins_and_me', 'label' => _x( 'Only Me', 'Visibility level setting', 'buddypress' ) ),
Change History (7)
#2
@
10 years ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
#3
@
10 years ago
Well, it's not a big deal anyway, but would it not be enough to put a note in the changelog about a name change like this, when they update? Obviously it only affects people who have custom code I assume? Normal buddypress installs should not be affected with a name/key change like that or would they?
When I update a plugin, I always expect to clean some things up/change code, if I have written custom code for that plugin. But maybe not all people think like that.
But yes, I guess on update a function would need to run to replace all existing keys with the new one.
#4
@
10 years ago
Normal buddypress installs should not be affected with a name/key change like that or would they?
They would. The 'id' is what we store in usermeta to identify a user's settings. So we'd have to swap it out for every member on the site. Because the data is in a serialized array (for some annoying historical reasons), we would have to do it in PHP rather than pure MySQL. This means that it'll be a fairly heavy operation on large sites, meaning that we'd have to have an asynchronous script for the migration. You see how this gets complicated fast :)
Then yes, there would be a problem with plugins expecting the 'adminsonly' visibility field. When necessary, we will make changes like this, and attempt to inform developers in the way you've suggested. But we generally prefer to remain backward-compatible, unless breaking changes cannot be avoided. The current case is one where the implementation is perhaps a little unclear and annoying, but there's no real bug, so it's definitely not worth breaking existing plugins.
#5
follow-up:
↓ 6
@
10 years ago
Yes, totally understand. It is a lot of work for such a little thing!
BTW: why are the privacy levels saved in user meta and not in a column with each field in the wp_bp_xprofile_data table? That's somehow what I expected when I first looked at privacy settings...
#6
in reply to:
↑ 5
@
10 years ago
Replying to landwire:
BTW: why are the privacy levels saved in user meta and not in a column with each field in the wp_bp_xprofile_data table? That's somehow what I expected when I first looked at privacy settings...
It was an error in judgment. They should be migrated for a number of reasons. See #6211 for one of these reasons.
See also #5317. We can't easily change the key of the existing visibility level without breaking people's sites (or without a complicated migration routine). If you'd like to add a "true" admins only field type, you'll have to use a different 'id' for it.