Additional logic in determing access level

[jasonpartyka]

Problem

Sometimes the ability to update/create content that is controlled by a term needs to be restricted further. As an example:

• A term representing group "A" exists, but due to business reasons new content may not be assigned to this group. The term cannot be removed as the association needs to be kept intact because of reporting, theme, et al. Let's call this the 'deprecated' group.
• Content part of "group A" may still be updated, with the deprecated group intact. However, only users who are members of the deprecated group may update it. The content may still in group "A", but if ownership is changed, it must go to a group that is not deprecated.

Proposed Solution

In the generic case, provide a way that a developer can write additional logic to further restrict if access is denied.

There are two ways we could go about this, and both involve updating Drupal\workbench_access\Plugin\EntityReferenceSelection\TaxonomyHierarchySelection::getReferenceableEntities method to execute the chosen pattern.

1. Create a service class to provide the logic. The developer then writes a service class do to provide the logic.
2. Provide an 'empty method' (for lack of a better term) that be used in a Plugin class that inherits from TaxonomyHierarchySelection and then implements this method. This 'empty method' would simply always say access is allowed in the default implementation within TaxonomyHierarchySelection.

Other solutions

These other solutions have been considered and have their own drawbacks:

1. Validate on save. This would preserve data integrity, but presents a sub-optimal user experience as the user may choose an option that is believed to be valid.
2. Completely override Plugin w/o inheriting from class. This is possible, but requires keeping an eye on the copied class instead of leveraging OO patterns.

I will need to make this alteration for our own needs, so I'd like to do it in a sustainable way that could be incorporated back into the module.

[larowlan]

Option 3: use the plugin alter hook to change the class used for the taxonomy plugin, subclass the exisiting one and override get referenced entities, calling the parent in most cases?

…

On Thu, Nov 29, 2018, 7:43 AM jasonpartyka ***@***.*** wrote: Problem Sometimes the ability to update/create content that is controlled by a term needs to be restricted further. As an example: - A term representing group "A" exists, but due to business reasons new content may not be assigned to this group. The term cannot be removed as the association needs to be kept intact because of reporting, theme, et al. Let's call this the 'deprecated' group. - Content part of "group A" may still be updated, with the deprecated group intact. However, only users who are members of the deprecated group may update it. The content may still in group "A", but if ownership is changed, it must go to a group that is not deprecated. Proposed Solution In the generic case, provide a way that a developer can write additional logic to further restrict if access is denied. There are two ways we could go about this, and both involve updating Drupal\workbench_access\Plugin\EntityReferenceSelection\TaxonomyHierarchySelection::getReferenceableEntities method to execute the chosen pattern. 1. Create a service class to provide the logic. The developer then writes a service class do to provide the logic. 2. Provide an 'empty method' (for lack of a better term) that be used in a Plugin class that inherits from TaxonomyHierarchySelection and then implements this method. This 'empty method' would simply always say access is allowed in the default implementation within TaxonomyHierarchySelection. Other solutions These other solutions have been considered and have their own drawbacks: 1. Validate on save. This would preserve data integrity, but presents a sub-optimal user experience as the user may choose an option that is believed to be valid. 2. Completely override Plugin w/o inheriting from class. This is possible, but requires keeping an eye on the copied class instead of leveraging OO patterns. I will need to make this alteration for our own needs, so I'd like to do it in a sustainable way that could be incorporated back into the module. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#122>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAh49p15DxPwjspQy6pFear3V72FHhDbks5uzwNqgaJpZM4Y4fkc> .

[agentrickard]

It's probably the plugin alter. Note that the class @jasonpartyka is mentioning here just controls the autocomplete selection of terms. We're really looking back a step at the alterForm() method for the plugin class.

Drupal\workbench_access\Plugin\AccessControlHierarchy\Taxonomy\AlterForm

[larowlan]

Both are plugins that can be altered though

…

On Thu, Nov 29, 2018, 8:21 AM Ken Rickard ***@***.*** wrote: It's probably the plugin alter. Note that the class @jasonpartyka <https://github.com/jasonpartyka> is mentioning here just controls the *autocomplete* selection of terms. We're really looking back a step at the alterForm() method for the plugin class. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#122 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAh49vd1rXh5IyY9jVkB8HmL7Mj3tTSTks5uzwxbgaJpZM4Y4fkc> .

[jasonpartyka]

So, getReferenceableEntities is only used to determine autocomplete and is not called for form validation? If that's correct then the proposed solution is incomplete.

[larowlan]

Form validation should be from a validation plugin, so it works for REST et al

…

On Thu, Nov 29, 2018, 1:05 PM jasonpartyka ***@***.*** wrote: So, getReferenceableEntities is only used to determine autocomplete and is not called for form validation? If that's correct then the proposed solution is incomplete. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#122 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAh49qG4hpgk68IHBRuOpeicdilaQV8Eks5uz08SgaJpZM4Y4fkc> .

[jasonpartyka]

Yeah, so then why not create a service so that the custom access logic is written just once? But, I don't see a validation plugin in workbench access either. Did I miss it? I'm looking at 8.x-1.0-beta2

[larowlan]

Core uses selection plugins for validating as well as showing options See valid referenced entity validation plugin in core.

…

On Thu, Nov 29, 2018, 1:15 PM jasonpartyka ***@***.*** wrote: Yeah, so then why not create a service so that the custom access logic is written just once? But, I don't see a validation plugin in workbench access either. Did I miss it? I'm looking at 8.x-1.0-beta2 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#122 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAh49ooIku3-2s5NRm_kqt3R1RvIatoSks5uz1FKgaJpZM4Y4fkc> .

[jasonpartyka]

Gotcha. So I'm totally overthinking it then. :-)

[agentrickard]

All that said, I'm not certain that we handle Validation correctly via plugins -- unless @larowlan is saying that core already handles that for us.

This is an area of the code where changes from D7 to D8 have me a bit behind.

[larowlan]

I think core does it for us *if* you select the right selection plugin for the ER field

…

On Mon, Dec 3, 2018, 7:25 AM Ken Rickard ***@***.*** wrote: All that said, I'm not certain that we handle Validation correctly via plugins -- unless @larowlan <https://github.com/larowlan> is saying that core already handles that for us. This is an area of the code where changes from D7 to D8 have me a bit behind. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#122 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAh49pTKlAltDy53GFOAhLZhjy98ZxOMks5u1UKFgaJpZM4Y4fkc> .

[agentrickard]

I believe we do in the case of the autocomplete -- because in that case we swap out the selection plugin in its entirety. I'm not entirely certain that the standard form alter that we do for other field types adds this validation step.

[jasonpartyka]

So, I don't see any Validation plugins in in the src/Plugin directory. This would handle any widget used for term selection, correct?

[jasonpartyka]

So -- I'm picking this issue up again. @agentrickard and @larowlan, do you believe there is any value in providing a generic solution?

[agentrickard]

Very probably, but I haven't given it much thought.