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.

Files: 
CommentFileSizeAuthor
#18 et-shared_fields-1798456-17.interdiff.do_not_test.patch2.49 KBbforchhammer
#18 et-shared_fields-1798456-17.patch15.5 KBbforchhammer
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
#16 et-shared_fields-1798456-16.interdiff.do_not_test.patch1.98 KBplach
#16 et-shared_fields-1798456-16.patch14.77 KBplach
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
#14 et-shared_fields-1798456-14.interdiff.do_not_test.patch14.13 KBplach
#14 et-shared_fields-1798456-14.patch15.38 KBplach
FAILED: [[SimpleTest]]: [MySQL] 609 pass(es), 89 fail(s), and 24 exception(s).
[ View ]
#9 et-shared_fields-1798456-9.patch7.02 KBbforchhammer
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
#3 et-shared_fields-1798456-3.interdiff.do_not_test.patch4.53 KBbforchhammer
#3 et-shared_fields-1798456-3.patch11.96 KBbforchhammer
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
#1 et-shared_fields-1798456-1.patch8.2 KBplach
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]

Comments

Status:Active» Postponed
StatusFileSize
new8.2 KB
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]

This 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.

Status:Postponed» Needs review

Now we are ready for bot review.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new11.96 KB
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
new4.53 KB

Permission 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.

#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.

Status:Reviewed & tested by the community» Fixed

Makes sense, thanks.

Status:Fixed» Active

I'm afraid this issue needs a little more work...

From #1495570-20: Update Entity translation integration:

One minor thing: the "Product SKU" and "Status" fields are not marked as "shared fields" (i.e. no "translatability clue") but are treated as them (i.e. changes in one language affect the other language). This will need to be changed in the ET module, so as far as this patch is concerned I'd say it's RTBC :)

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...

You 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.

Status:Active» Needs review
Issue tags:+Needs tests
StatusFileSize
new7.02 KB
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]

Attached 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.

Status:Needs review» Needs work

+++ b/includes/translation.handler.inc
@@ -1075,20 +1075,37 @@ class EntityTranslationDefaultHandler implements EntityTranslationHandlerInterfa
+   * Fields are handled separately in entity_translation_field_attach_form().

Can 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.

+++ b/includes/translation.handler.inc
@@ -1075,20 +1075,37 @@ class EntityTranslationDefaultHandler implements EntityTranslationHandlerInterfa
+  private function entityFormSharedElements(&$form) {

Why private?

+++ b/includes/translation.handler.inc
@@ -1075,20 +1075,37 @@ class EntityTranslationDefaultHandler implements EntityTranslationHandlerInterfa
+    $ignored_elements = array_flip(array('language', 'translation'));

This should not be needed, as the language widget and the translation settings do have an #access key.

+++ b/includes/translation.handler.inc
@@ -1075,20 +1075,37 @@ class EntityTranslationDefaultHandler implements EntityTranslationHandlerInterfa
+        $form[$key]['#translatable'] = FALSE;

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.

A new patch should also consider #1802452: Notice for form elements without a #type.

Marked that one duplicate. Let's not forget about crediting @Berdir in the changelog and commit message.

Assigned:Unassigned» plach

Working on this.

Assigned:plach» Unassigned
Status:Needs work» Needs review
StatusFileSize
new15.38 KB
FAILED: [[SimpleTest]]: [MySQL] 609 pass(es), 89 fail(s), and 24 exception(s).
[ View ]
new14.13 KB

The attached patch fixes #10 and adjusts the update function 7002 to cover also shared fields permissions.

Status:Needs review» Needs work

The last submitted patch, et-shared_fields-1798456-14.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new14.77 KB
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
new1.98 KB

Actually we want to prevent access only to the entity form fields: hook_field_access() was too much.

Status:Needs review» Reviewed & tested by the community

StatusFileSize
new15.5 KB
PASSED: [[SimpleTest]]: [MySQL] 681 pass(es).
[ View ]
new2.49 KB

Cool, looks good to me. I'll commit your patch with minor changes in a minute...

Status:Reviewed & tested by the community» Fixed

Committed and pushed, thanks! :)

Automatically closed -- issue fixed for 2 weeks with no activity.