add additional permission to Custom Fields: core.display.field

[Ruud68]

Pull Request for Issue # https://issues.joomla.org/tracker/joomla-cms/19468.

Summary of Changes

Currently there are two setting with which you can control the availability of Custom Fields:

1. Access: the group that can access the Custom Fields AND the Custom Fields value
2. Permission: set the core.edit.value to the group that can add AND change the Custom Field value.

This leads to the following problem use case:
Custom Fields are used to add a Avatar (media), followme link, author info (editor), company logo (media) to users that are member of the group 'Author'. The Values set by these authors in their profile should be available to public (they are read and displayed via a content plugin to show in articles).

Because in the current implementation of Custom Fields, the access setting of the fields is used to BOTH show the Custom Fields AND the Custom Field values, this will result in a registration form with all the Custom Fields for the authors displayed as read-only.
This leads to unwanted support calls: I want to register an account, what are these fields for that are not working? Why do I need to scroll down passed all of these non-working fields to click the [Register] button]?

Changing the access of the fields to value other then Public will result in the values (the avatar, the link, the author info, etc.) to NOT be available in the article to visitors: the values will then only be visible to logged in users with author rights.

With this PR, there is introduced a new permission resulting in the following settings:

1. Access: the group that can access the Custom Fields VALUE
2. permission: the new 'core.display.field' setting determines who these fields will be shown to in the registration / edit / create forms (for com_users, com_articles, com_contact)
3. permission: set the core.edit.value to the group that can add AND change the Custom Field value.

In the above example you can set the Access to public (everybody should be able to see the Custom Field VALUE), set the permission 'core.display.field' and 'core.edit.value' to author: the fields will only show in the profile view / profile edit form of users that are member of the author group.

The same applies to com_content. if you have a Custom Field that only Publishers should be able to set, in the current situation this field would be visible in the content edit form for all users who can edit articles. With this PR you can set the Custom Field to ONLY show to for example publishers WITHOUT making the VALUE of the Custom Field unavailable to other groups (like public).

Testing Instructions

See description above.

Expected result

See description above.

Actual result

See description above.

Documentation Changes Required

Not sure

[laoneo]

From a quick glance, using permissions for display is for me confusing. Having an additional permission just for this use case is wrong in my opinion. You need to add that permission then also to the global Joomla configuration and some other places, it needs some more testing.
If it is a problem that somebody who can't edit a field but can see it, then we need to reconsider that behavior. A way can also be to add a flag in the field if the user permission is not sufficient to hide it or to disable it.

[Ruud68]

Hi @laoneo, thanks for looking into this :)

You need to add that permission then also to the global Joomla configuration and some other places

It is added to the global configuration (application.xml) and all relevant access.xml files (com_contact, com_content, com_users).

If it is a problem that somebody who can't edit a field but can see it, then we need to reconsider that behavior.

This is exactly the problem. If you have for example 20 Custom Fields on com_users, then a new user who registers will see all these fields, even when these are not for him (edit permissions is disallowed).

In the current implementation it was opted to show the fields that a user does not have edit rights to, this is 'conflicting' with default joomla behavior: for example the article field published (state) is NOT visible to authors as they do not have the right permissions, but IS shown to publishers.

It is easy to fix the Custom Fields to follow this same behavior by checking the field edit permissions and if not allowed, NOT add them to the form (like I check now for display permission). But that would mean that it will change the current implementation for end-users. Before the change they see the Custom fields as read-only, after the change they only see the Custom Fields they have edit access to. If they also want to see the read-only fields, they cannot.

The current PR facilitates both: you can set to show, or not to show the (read-only) fields: this opens new possibilities for implementing Custom Fields in components etc.

[laoneo]

It is added to the global configuration (application.xml) and all relevant access.xml files (com_contact, com_content, com_users).

I meant the installation routine. Or I am wrong?

I see the use case, but for me a permission is too much overhead. If it is really needed to control the display behavior if a field should be shown or not when the user has no permissions, then I would solve it with a flag in the field and not a new permission. I guess there are tons of similar use cases which can be solved with a permission, but it doesn't mean we do it. Permissions should be extended carefully. I'm pretty sure that it increases the complexity to much for little gain. At the end it is really only a yes/no decision if a field should be shown when there are no permissions and nothing more.

[Bakual]

The problem with a permission approach is that by default, the permission is denied until explicitely allowed. That means this proposal will hide all fields for all usergroups (except super admin) and thus isn't B/C. You will not be able to fix that since it's is the way our ACL works.

I also like Allon think a "display" permission looks wrong and is very unintuitive.
A better approach may be to have two viewlevel parameters. One for regular display and one for "display in form".
Or follow the suggestion by Allon with a toggle that ties edit and view together.

[Ruud68]

Hi @laoneo and @Bakual , good to have this discussion :)

At the end it is really only a yes/no decision if a field should be shown when there are no permissions and nothing more.

Agree, just now to figure out what the best method is to do so

The problem with a permission approach is that by default, the permission is denied until explicitely allowed.

True, I ran into that issue myself when creating the Custom Fields for the first time and they where read-only... The permission to edit the field value 'core.edit.value' was disallowed by default. I was told that this could be changed to a default of 'Allowed' (as it is a global permission) but only for new installs, not for upgrades.
This would then also apply to the permission core.display.field. with the update of a component you can change settings via a script (like alter a table). I think that if it is possible to have the global permission for core.edit.display be set as 'allowed' there is no script necessary as this is a new permission: so no change is required... that should be tested however...

I also like Allon think a "display" permission looks wrong and is very unintuitive.

I agree, but the core.edit.value feels for me the same: all permissions are for creating / editing / publishing the objects (in this case a Custom Field), The permission to 'create a value' is for me also unintuitive but somehow it ended up in there. That could also have been done a different way. I haven't seen or followed that discussion.

A better approach may be to have two viewlevel parameters. One for regular display and one for "display in form".

Can you explain this a little further: would this then be a second 'Access' (called display) where you can select groups? like the image below? Or where do you want these settings?

Or follow the suggestion by Allon with a toggle that ties edit and view together.

I am keen on a solution that can be set on Field group level instead of on every Field separately. That is in my opinion the best practice: always configure as high up in the tree as possible (not sure if that translated correct, but you know what I mean I hope :))

It goes without saying: If anybody wants to jump into this discussion, feel free :)

[Bakual]

I was told that this could be changed to a default of 'Allowed' (as it is a global permission) but only for new installs, not for upgrades.

It could be done for new installations, yes. For upgrades, you don't want to touch this even with a script. You don't know which usergroups are present on an existing installation and you don't know which one should be allowed to edit something.

Can you explain this a little further: would this then be a second 'Access' (called display in form) where you can select groups? like the image below? Or where do you want these settings?
display

Yeah, like in your screenshot. Of course it needs a clear parameter title.

[Ruud68]

My original approach was not core.DISPLAY.field but core CREATE.value

So a permission to allow or disallow creation of a value.
When the permission is disallowed, the following two situations can occur:

1. no value exists > do not display the field (it adds no value as you cannot create a value)
2. a value exists (administrator created or history): show the value (read-only when core.edit.value = disallowed)

When the permission is allowed:
same as current implementation.

Difference is that when a value is set, the field will always show: there is no way to NOT show the field on forms other then changing the access level, but by doing that you also NOT show the value in other (non-forms) views.

This still holds the default permission value of disallowed issue (you want it to be allowed by default to reflect the current setup).
But would this make more sense as to be intuitive in the permissions tab?

Of course it needs a clear parameter title.

of course: it needs to be self explanatory (at least as much as possible)

[Ruud68]

Okay, after a long night of little sleep...
the idea to add this feature (or solve this bug) via a toggle, or an extra 'access' field would work, but would also be 'very' complicated as it holds a database change, a new method to check for the display access, implementation of that method in com_fields (or in the classes com_fields is extended from?), updates to database tables and values for existing fields and groups, etc.

where this PR

is confined to the com_fields helper class and adds only one (very simple) method to the FieldsHelper

	public static function canDisplayField($field)
	{
		$parts = self::extract($field->context);

		return JFactory::getUser()->authorise('core.display.field', $parts[0] . '.field.' . (int) $field->id);
	}

changes the method public static function prepareForm($context, JForm $form, $data)
with the following (again very simple logic)

	if (!FieldsHelper::canDisplayField($field) && JFactory::getApplication()->isClient('site'))
	{
		// If logged in user (front-end) does NOT have display rights on Custom Field
		continue;
	}

It could be done for new installations, yes. For upgrades, you don't want to touch this even with a script. You don't know which usergroups are present on an existing installation and you don't know which one should be allowed to edit something.

This would actually be a relative small and simple change:
for new installations: in the ./installation/sql/[db]/joomla.sql you can change:

INSERT INTO `#__assets` (`id`, `parent_id`, `lft`, `rgt`, `level`, `name`, `title`, `rules`) VALUES
(1, 0, 0, 105, 0, 'root.1', 'Root Asset', '{"core.login.site":{"6":1,"2":1},"core.login.admin":{"6":1},"core.login.offline":{"6":1},"core.admin":{"8":1},"core.manage":{"7":1},"core.create":{"6":1,"3":1},"core.delete":{"6":1},"core.edit":
{"6":1,"4":1},"core.edit.state":{"6":1,"5":1},"core.edit.own":{"6":1,"3":1}}'),

into (adding ,"core.display.field":{"1":1})

INSERT INTO `#__assets` (`id`, `parent_id`, `lft`, `rgt`, `level`, `name`, `title`, `rules`) VALUES
(1, 0, 0, 105, 0, 'root.1', 'Root Asset', '{"core.login.site":{"6":1,"2":1},"core.login.admin":{"6":1},"core.login.offline":{"6":1},"core.admin":{"8":1},"core.manage":{"7":1},"core.create":{"6":1,"3":1},"core.delete":{"6":1},"core.edit":
{"6":1,"4":1},"core.edit.state":{"6":1,"5":1},"core.edit.own":{"6":1,"3":1},"core.display.field":{"1":1}}'),

there is NO need to do this for all components because when the permission is NOT found for a component it 'falls back' to the global setting! So this one change is sufficient!

For the upgrade, that would mean a change to the ./administrator/components/com_admin/scripts.php file.
Here we can add the logic to read the current global settings and add the 'core.display.field' to the existing value. This needs to be developed and tested.

@laoneo can you have a look (if you didn't; already look at it) at the (FieldsHelper) code changes in this PR (summarized above)?

Who should (or wants to) be involved apart from @laoneo and @Bakual to give direction so we minimize the risk of developing several solutions just to see them rejected?

[Bakual]

Imho, using a permission to display the field is still wrong. Yes the other solutuons need more work and database changes but they are the way to go if you want to implement this.

[Ruud68]

just for the record here is quick (working) mockup for updating the permissions when updating Joomla:

$asset = JTable::getInstance('Asset');
$asset->loadByName('root.1');
$rules = json_decode($asset->rules, true);
if (!(array_key_exists('core.display.field', $rules)))
{
        $rules['core.display.field'] = array('1' => 1);
}
$asset->rules = json_encode($rules);
$asset->store();

[Ruud68]

@Bakual bakual, would this #19473 (comment) make a difference in the permission discussion?

[laoneo]

An additional option doesn't need db changes as it is saved in the params column. I'm here with @Bakual, using permissions for this use case is too much of overhead.

[Bakual]

would this (comment) make a difference in the permission discussion?

No, still think it's wrong to have display as permission,

[Ruud68]

An additional option doesn't need db changes as it is saved in the params column. I'm here with @Bakual, using permissions for this use case is too much of overhead.

Just to make sure that we envision the same 'solution'.
@laoneo you want to implement via option: options are part of parameter.
But a Field Group does not have options (and parameters). Doing it this way you cannot control the display via a field group setting, only on a per Custom Field setting.
If you have for example 20+ fields in a group that you need to change the display option for, that would mean 20+ times the work.

I think that what @Bakual was hinting at was like in below screen mockup. the value set here needs a db change for com_fields to hold the value.

[Bakual]

Imho field groups have the params database column already. It's just not used.
But anyway, adding a new column is really no issue.

[Ruud68]

Imho field groups have the params database column already. It's just not used.

Yes, found it, used it... fixed it :)
It (params) was not implemented in the group and groups model (it was loaded as array so $model->params->get('...') etc. didn't work.
further more I had to 'fix' the group model getTable as this needs to be 'forced' into the com_fields table path (because when used in front-end it can be invoked from other contexts (like com_user)

Anyway, although way out of my comfort zone, I can report that I was able to get all of it working with an additional option in the field and group (via params) *padding my own shoulder here* :)

I will do some more testing and then I will create a new PR for this. I will ping you (@laoneo and @Bakual ) so that you don't miss it

[laoneo]

If you want to add it as an option then I would just go with a yes/no option if the field should be shown in the form. If you want to go with the access flag, then I would put it into the position after the general access flag and call it access form. I would not spread stuff across different tabs which do have a similar job.

[Ruud68]

If you want to add it as an option then I would just go with a yes/no option if the field should be shown in the form.

a simple yes / no will not be enough, you want to display it on a per group membership.

Running currently into an issue where when field is not added to form, the value gets deleted from the database when the form (user / article) is saved.

Will try to refactor the PR where the option is handled like the view level.... the plus side of all these refactoring is that I learn a lot :S

[Bakual]

a simple yes / no will not be enough, you want to display it on a per group membership.

I think Allon meant a yes/no parameter like "Only show in forms when editable" (needs better name of course).

[Ruud68]

I think Allon meant a yes/no parameter like "Only show in forms when editable" (needs better name of course).

Yes, that would work I guess. is a small change to what I have now.

The challenge that I am facing now is that the field values that are not shown on the form get deleted from the database when saving the form.

What would be the best function to exclude the 'hidden' fields?

[laoneo]

Here https://github.com/joomla/joomla-cms/blob/staging/plugins/system/fields/fields.php#L100 you have the information if the user can save the field.

[Ruud68]

Thanks, I was already 'poking' around in the models setFieldValue, ran into a get on bool issue when trying to get the params out of the field / group model...
Good to know I am on the right track :) need to step back for a while (when I wasn't self employed, my boss would kick my #%@ :))

What would your preference be: yes/no via options (params) or via access level (with db change)?

[laoneo]

I don't mind. Yes/no is probably easier for the average user. At the moment I can't see a use case to have a specific access level for forms. But I'm not a power user of custom fields.

[Ruud68]

For the customer that I using the Custom Fields for now, a simple yes no based on read-only would be enough. But although not used currently adding it as access / view level we enable more specific use cases and there for let Joomla! rock even more :)

What I am trying to avoid is spending a lot of time on a change that will never make it :s

[laoneo]

I fear more that when we add an access level that it can lead to confusion, when users do have the permission and no access level.

[Ruud68]

I fear more that when we add an access level that it can lead to confusion, when users do have the permission and no access level.

I personally don't fear that. The value defaults to 'public' so nothing changes in the implemented functionality. What does change is that it adds functionality that users can make use of. So when things don't show up on the form, it is because they changed the value.

My experience is that people who are using Custom Fields are not the 'average' end-users as custom fields are easy to set from a Joomla! perspective, but not so easy to utilize from a displaying on front-end perspective.
So I think the target audience for Custom Fields are advanced users and website developers / integrators that build functionality (for customers) with these Custom Fields where they would otherwise purchase a Content Construction Kit component for (or create a custom component).

[Quy]

@Ruud68 Is #16457 related and is this PR also for it?

[Ruud68]

hi @Quy ,
this is not related. The referenced issue is about the initial permission. It issue is currently open as the answer given by @Bakual is not a definitive: NO we are not going to do that (then the issue can be closed) or YES, that sounds like a good improvement (then a PR can be created) :)

[Ruud68]

The challenge that I am facing now is that the field values that are not shown on the form get deleted from the database when saving the form.

What would be the best function to exclude the 'hidden' fields?

@laneo I have fixed this behavior that appeared to be a 'bug' (at least that is what I learned) :)
can you have a look / test this PR (#19742) as this will impact the way I can handle this change :)

[Ruud68]

Hi, I have implemented this functionality here: Ruud68@6ca1617
you can now set an Access Form view level. This extends the Custom Fields functionality.
e.g. you can now set Custom User fields that you only want to be available and shown in the profile form to registered users (currently all fields are displayed on the registration form as read-only when access rights are set to registered).

There is currently one blocking issue: when the field is not shown on the form, any value set in the field will be removed from the database when saving the object (user or article). This is a know bug and also affects the display on [front-end][administrator][both] implementation.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19473.}

[Ruud68]

Yes, now that #19884 is in staging I can finaly get this change into motion :)
closing this PR as new 'clean' PR has been created for this new functionality: https://issues.joomla.org/tracker/joomla-cms/19996

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19473.}