Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When a user not being allowed to edit shared values accesses the entity form she should not see any form element except multilingual or explictly allowed ones.
Comments
Comment #1
plachThis has been rolled while working on #1280546: Introduce a language selection widget for every entity thus won't apply to HEAD until that one is committed.
Reviews welcome anyway.
Comment #2
plachNow we are ready for bot review.
Comment #3
bforchhammer CreditAttribution: bforchhammer commentedPermission change looks fine.
I think the bundle/id properties should actually be set by the
setEntity()
method to make sure they always match the wrapped entity... as far as I can see, this also means that we can remove the$entity_id
constructor argument.Comment #4
plach#3 was addressing two completely different issues in one patch. I split them: #1799770: Update id and bundle when setting a wrapped entity.
Committed and pushed the relevant part.
Comment #5
plachComment #6
bforchhammer CreditAttribution: bforchhammer commentedMakes sense, thanks.
Comment #7
bforchhammer CreditAttribution: bforchhammer commentedI'm afraid this issue needs a little more work...
From #1495570-20: Update Entity translation integration:
Also, the behavior around what is considered a shared field in which cases has grown inconsistent; for "fields" (changed field_attach_form_alter), the fields is only hidden on translation forms, whereas properties (change by the committed patch) are also hidden on the source edit form. Fields also do not respect the entity-type specific shared fields permission yet.
I'm working on a patch...
Comment #8
plachYou are right, there is an inconsistency: I think one should be allowed to edit shared fields only if she has the related permission, we will then have an #1770748: Option to display shared fields only when editing the original values to match the current field behavior.
Comment #9
bforchhammer CreditAttribution: bforchhammer commentedAttached patch tries to fix both issues mentioned above.
With commerce products we now have a "(shared field)" suffix on titles for the following elements: Product SKU, Price, Status and Change history.
Comment #10
plachCan we handle everything here? I don't think we have a particular reason to deal with fields in a separate place, and that makes the logic harder to follow.
Why private?
This should not be needed, as the language widget and the translation settings do have an
#access
key.This looks promising, however I think in the future we will need a more reliable way to determine whether a widget is multilingual or not, and enforce shared access based on that, even if we already have an
#access
key. I'm thinking about a'#multilingual'
key instead of'#translatable'
(which is more specific) that we would assign to core form elements (fields included) and we would ask other modules to provide to integrate with ET. It would simply be ignored if ET is not installed. This must be a follow-up though, as we need to ensure modules actually accept this integration stuff.This is already planned for D8, btw: #1498724: Introduce a #multilingual key (or similar) for form element.
Comment #11
bforchhammer CreditAttribution: bforchhammer commentedA new patch should also consider #1802452: Notice for form elements without a #type.
Comment #12
plachMarked that one duplicate. Let's not forget about crediting @Berdir in the changelog and commit message.
Comment #13
plachWorking on this.
Comment #14
plachThe attached patch fixes #10 and adjusts the update function 7002 to cover also shared fields permissions.
Comment #16
plachActually we want to prevent access only to the entity form fields:
hook_field_access()
was too much.Comment #17
bforchhammer CreditAttribution: bforchhammer commentedComment #18
bforchhammer CreditAttribution: bforchhammer commentedCool, looks good to me. I'll commit your patch with minor changes in a minute...
Comment #19
bforchhammer CreditAttribution: bforchhammer commentedCommitted and pushed, thanks! :)