Intuitively, the idea I had of entity translation was that some instances of fields could be translatable.

For example, I had the misconception that changing Page's body field to translatable would not affect Article's body field.

The field modification page has two sections, one, on top, relative to the instance of the field, and the other, on the bottom, relative to all instances of the field. The field translatability option is on the bottom of the bottom section, hence applying to all instances of the field.

Still, even though I am quite used to Drupal, I somehow missed this, and thought I was affecting the instance of the field, not the field itself.

Because this can have some dire implications, including affecting data (see for example #1916712: Provide an API to update content programmatically, not just through the UI (useful for deploying with Features, for example), where the body field was being affected my mistake), it might be a good idea to add a warning or something make absolutely sure that the user knows that *all* fields, not just one instance, are being affected.

See attached.

Cheers,

Albert.

Comments

plach’s picture

Sorry, but the phrase you are suggesting is very similar to the one below the fieldset title, if my french serves :)

I don't think repeating it would be particularly useful: if you didn't notice the former why should you notice the latter?

alberto56’s picture

@platch you are right, but for some reason it just did not register for me. Either others will make the same mistake, or I'm just weird :)

I know that if I had seen a bold sentence like: "this will make the body field in Article, Page, Event, and Press Release" translatable, I would have been alerted, if the sentence had been just below the widget rather than at the top of the box, where I'm used to just ignoring it.

plach’s picture

This sounds a bit overkill to me but an additional warning cannot do too much harm. Care to provide a patch?

alberto56’s picture

Here is a patch. The following strings are changed:

example: FIELD = body, LIST = page, article, event

  • users may translate this field => users may translate all instances of FIELD (LIST)
  • Enable translation => Enable translation for all instances of FIELD (LIST)
  • Disable translation => Disable translation and delete translations for all instances of FIELD (LIST)
  • Are you sure you want to enable translation for the FIELD field? => Are you sure you want to enable translation for all instances of the FIELD field (LIST)?
  • Are you sure you want to disable translation for the FIELD field? => Are you sure you want to disable translation for all instances of the FIELD field (LIST)?
  • All the existing translations of this field will be deleted. => All the existing translations of this field in LIST will be deleted.
alberto56’s picture

StatusFileSize
new90.57 KB

Here is one example of what this patch does. (see image)

alberto56’s picture

Status: Active » Needs review
plach’s picture

Status: Needs review » Needs work

Sorry for being vacant. This looks promising to me but the end result looks a bit redundant to me. I'd prefer something like the following:

Users may translate all instances of Body (Article, Page, ...).
Disable translation

I think we can avoid repeating the warning that translations will be deleted as we already have it in the confirmation form.
Also it seems that you are using the field and instance machine names instead of the corresponding labels.

alberto56’s picture

@platch thanks, you are probably right on all counts. I'm curious to see if there is more interest in this approach. Cheers.

plach’s picture

I'm terribly sorry I let all this time pass after asking you a patch, I completely lost track of this issue until now. If you provide a new one implementing #7 I swear I'll commit it asap.

alberto56’s picture

Status: Needs work » Needs review
StatusFileSize
new3.85 KB

Hi,

Coming up with a new version of the patch has been a bit more of a challenge than I at first thought! Here is why.

Challenge #1: Fields do not have human-readable names

Field instances have human-readable names, but fields do not. Take this example

  • Field field_body in Article has the name "Body"
  • Field field_body in Page has the name "Main text"
  • Field field_body_translatable in Press release has the name "Body"
  • Field field_main in Press release has the name "Main text"

In the above example, it does not really make sense to say "All instances of field Body will be deleted".

Challenge #2: Node types are not the only entity types are can have fields

Users, and taxonomy terms, and other entity types can also have fields, so one can have a situation where:

  • User accounts (entity type: user, entity: user) have the field xyz
  • Taxonomy term abc (entity type: taxonomy term, entity: abc) has the field xyz
  • Node type abc (entity type: node, entity: abc) does not have the field xyz
  • Node type page (entity type: node, entity: page) has the field xyz

In the above scenario it would be misleading to say "All instances of xyz (user, abc, page) would be misleading because the taxonomy term abc has yxz, but not the node type abc.

Proposed solution

Because fields do not have labels, the machine name will have to be used.
Each field instance is then uniquely defined by its label, entity type and entity.

For examplpe: field machine_name (Body in node Article, Body in node Page, Bio in user User, Description in taxonomy term Tags)

I have removed the warning in disable translations, which makes the whole thing clearer, as mentioned above.

Status: Needs review » Needs work

The last submitted patch, entity_translation--1916738-10-ux-affects-all-instances.patch, failed testing.

plach’s picture

"Body in node Article" looks a bit confusing to me: maybe it's just lack of context but it seems we are talking about a concrete node titled "Article" and not about a bundle named "Article". And "Bio in user User" makes this frist impression even stronger. Moreover, this way the line can get veery long. What about something like this?

Users may translate all instances of this field:

  • Body in Article
  • Body in Page
  • Bio in User
  • Description in Tags

Disable translation

alberto56’s picture

Status: Needs work » Needs review
StatusFileSize
new4.43 KB
new440.58 KB

Yes, that's a lot more readable.

I removed the bundle types node and user, but I left taxonomy term; which makes sense for me.

See enclosed image for the results.

Here is a patch

alberto56’s picture

Here is a better one: there were some out-of-date comments in the patch above.

Status: Needs review » Needs work

The last submitted patch, entity_translation--1916738-14-ux-affects-all-instances.patch, failed testing.

plach’s picture

+++ b/entity_translation.module
@@ -1911,3 +1911,32 @@ function entity_translation_pathauto_alias_alter(&$alias, array &$context) {
+      if (in_array($instance_info['entity_type'], array('node', 'user'))) {

Sorry, but this won't scale: Entity Translation supports any entity type, there is no reason to special case core ones. Moreover I really cannot see why taxonomy should behave differently.

alberto56’s picture

@platch thanks for the feedback. You're probably right: there are some edge cases where the bundle type (node, taxonomy term...) could make things a bit clearer, but it's just not worth the trouble and confusion :)

So here is a patche where I dispensed with the bundle type altogether.

The result can be seen in the enclosed image

Cheers

alberto56’s picture

Status: Needs work » Needs review
alberto56’s picture

This one is a bit cleaner. I removed some old code from previous patches in $t_args.

plach’s picture

StatusFileSize
new18.04 KB
new17.49 KB
new4.64 KB

Looks good to me.

I slightly improved the formatting of the confirmation form and restored the original wording in the links to make them more compact. This looks good to go to me now. What do you think?

1.png

2.png

alberto56’s picture

@platch yes that's cleaner, thanks.

plach’s picture

Status: Needs review » Fixed

Cool :)

Committed and pushed, thanks!

Status: Fixed » Closed (fixed)

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

  • Commit cc2234f on 7.x-1.x, factory, et-fc, revisions authored by alberto56, committed by plach:
    Issue #1916738 by alberto56, plach: Added Make it more obvious to those...

  • Commit cc2234f on 7.x-1.x, factory, et-fc, revisions, workbench authored by alberto56, committed by plach:
    Issue #1916738 by alberto56, plach: Added Make it more obvious to those...