tableau-mkt / entity_xliff Goto Github PK

May need to do some type coercion in the case that a field is of type integer.

Some example use-cases:

• Support custom moderation workflows
• Custom data syncing logic
• Custom logging

There appears to be an issue where field collection translation relies on the host entity having already been initialized for translation (meaning the host already exists). This may also affect other entity references, or may be unique to field collections.

This can be worked around easily by sorting fields for export/import so that field collection fields are pushed to the bottom... The underlying problem should probably be resolved, though.

Problem / motivation

Depending on the state of the Entity module's cache at any given time, it's possible for imports to catastrophically break. The most noticeable symptom for us: target translations intermittently and irreversibly overwriting source translations on import. This is extremely rare, but happens about once a month.

You can force these errors to happen by forcibly including all module includes on all requests. A Travis CI run with this issue can be found here. You could also add the following code to entity_xliff.module to get the same outcome:

function entity_xliff_init() {
  _entity_xliff_load_module_incs();
}

More useful reproduction steps available here: #117

Proposed resolution

Always load Entity XLIFF module include files (or do something as outlined below in the original report). Investigate the test failures and resolve them.

Original report

Coming off of #112. The way we handle hook/module includes for modules for which we provide functionality (everything in the modules directory) is error-prone.

Unless we explicitly call the _entity_xliff_load_module_incs() function before every hook/alter invocation, there's a possibility that functionality we provide could be unintentionally lost.

We should create a wrapper/s around drupal_alter() and/or module_invoke() that calls the the includes loader before invoking the hook. Then, we should replace all of our existing calls with the new wrapper/s.

If you manually download an XLIFF for a given language through an entity's XLIFF portal, the file downloaded does not have an extension.

It should have a .xlf file extension associated with it.

Problem / motivation

With workbench moderation installed, the "current" revision is no longer being pulled during XLIFF export--only the "live" revision.

Proposed resolution

Issue is that, in many workflows, the custom Entity XLIFF module includes have not yet been included.

Short term, we should add the include call before the alter call in the TranslatableFactory. Mid-term, we should run all Entity XLIFF hooks through a special hook executor that does this include for us (then we can kill all the spurious calls to the include method).

Currently, if you have a content type managed by Workbench Moderation... Even if there are revisions ahead of the live version, this module will always use the live version for translation source.

In this case, it makes sense to pull the latest/current draft revision.

If a module provides a custom field type and that field ought to be translated, we ought to allow the module to provide a way to get/set that data.

Follow-up to #80. The fix put in place doesn't seem to account for the situation described by the following conditions:

• An entity contains an embedded entity (like a field collection) with cardinality.
• An instance of this entity exists in a translation set and is already translated.
• The source language has some number of embedded entities (say 3)
• While the target language is slightly out-of-date and has fewer embedded entities (say 2)

Under the above conditions, an import does not seem to sync over the third embedded entity.

Results in a PDOException because part of the string that Drupal attempts to write to the DB is encoded incorrectly (not just an HTML encoding issue, but more along the lines of UTF-8).

Drupal will usually gracefully handle HTML encoded entities that come back during XLIFF import. Sometimes, because the text field isn't formatted or because the format setting is too restrictive, we get literals: Page d'accuei

Digging into support for Entity Field Translation (#2)... The current assumed strategy for supporting both Entity Field and Content translation (via inheritance) is going to be too cumbersome.

Better strategy is to have all of the logic for negotiating those paradigms in an EntityTranslatable factory, with specific instance classes like NodeContentTranslatable vs. NodeEntityFieldTranslatable that only have to account for their own getting/setting/initialization strategies.

Net-new imports of content with Paragraphs work as expected... However, if content that includes paragraphs is imported over an existing translation, the translations on the import will overwrite the source content (irrevocably losing the source content).

Here's the build in Travis, and here's the test code.

When you run drush en entity_xliff after freshly downloading the module, the install fails due to the following fatal error:

Fatal error: Class 'EntityXliff\Drupal\Factories\EntityTranslatableFactory' not found in /path/to/drupal/sites/all/modules/entity_xliff/modules/comment.entity_xliff.inc on line 20

Full stack trace below:

PHP Stack trace:
PHP   1. {main}() /path/to/.composer/vendor/drush/drush/drush.php:0
PHP   2. drush_main() /path/to/.composer/vendor/drush/drush/drush.php:16
PHP   3. _drush_bootstrap_and_dispatch() /path/to/.composer/vendor/drush/drush/drush.php:61
PHP   4. drush_dispatch() /path/to/.composer/vendor/drush/drush/drush.php:92
PHP   5. call_user_func_array:{/path/to/.composer/vendor/drush/drush/includes/command.inc:182}() /path/to/.composer/vendor/drush/drush/includes/command.inc:182
PHP   6. drush_command() /path/to/.composer/vendor/drush/drush/includes/command.inc:182
PHP   7. _drush_invoke_hooks() /path/to/.composer/vendor/drush/drush/includes/command.inc:214
PHP   8. call_user_func_array:{/path/to/.composer/vendor/drush/drush/includes/command.inc:362}() /path/to/.composer/vendor/drush/drush/includes/command.inc:362
PHP   9. drush_pm_enable() /path/to/.composer/vendor/drush/drush/includes/command.inc:362
PHP  10. drush_module_enable() /path/to/.composer/vendor/drush/drush/commands/pm/pm.drush.inc:1005
PHP  11. module_enable() /path/to/.composer/vendor/drush/drush/commands/core/drupal/environment_7.inc:143
PHP  12. module_invoke_all() /path/to/drupal/includes/module.inc:499
PHP  13. call_user_func_array:{/path/to/drupal/includes/module.inc:895}() /path/to/drupal/includes/module.inc:895
PHP  14. entity_modules_enabled() /path/to/drupal/includes/module.inc:895
PHP  15. _entity_modules_get_default_types() /path/to/drupal/sites/all/modules/entity/entity.module:974
PHP  16. entity_crud_get_info() /path/to/drupal/sites/all/modules/entity/entity.module:1028
PHP  17. entity_get_info() /path/to/drupal/sites/all/modules/entity/entity.module:723
PHP  18. drupal_alter() /path/to/drupal/includes/common.inc:7758
PHP  19. entity_xliff_entity_info_alter() /path/to/drupal/includes/module.inc:1101
PHP  20. _entity_xliff_get_translatable_info() /path/to/drupal/sites/all/modules/entity_xliff/entity_xliff.module:72
PHP  21. module_invoke_all() /path/to/drupal/sites/all/modules/entity_xliff/entity_xliff.module:131
PHP  22. call_user_func_array:{/path/to/drupal/includes/module.inc:895}() /path/to/drupal/includes/module.inc:895
PHP  23. comment_entity_xliff_translatable_info() /path/to/drupal/includes/module.inc:895

Problem / motivation

There are a variety of scenarios when an XLIFF export (and the implied content structure therein) can become stale compared to the underlying content and its structure on a live site:

• A piece of content with a certain field is exported and then the field is subsequently removed from the entity/bundle.
• A piece of content is exported with a referenced entity and then the referenced entity is de-referenced sometime between the initial export and later import.
• A piece of content contains a Paragraphs field with cardinality, then the order of two paragraphs items of different bundles is swapped.

In these and many other situations, an attempt to import the XLIFF will result in an Entity API exception of some kind: an EntityMalformedException if we're attempting to set content on a referenced entity that no longer is referenced, or an EntityMetadataWrapperException if we're attempting to set content on a field that doesn't exist on the entity/bundle.

Sometimes, the exception happens mid-import, leaving imported content in a potentially broken state.

Proposed resolution

There might be a solution to this based on entity revisions, but this would be a big shift in the underlying API of this module. There are also situations where revisions might not necessarily help (e.g. with entities that do not support revisioning).

Probably the best we can do now is to catch these exceptions and provide better error messaging. In addition, if possible, it'd be ideal to be able to roll back the import attempt so we don't end up with half-imported content.

See EntityTranslatableBase::entitySetReferencedEntity()

Work on this issue is blocked by official Entity Translation support in Field Collection. See: https://www.drupal.org/node/1344672?

When a node using the content translation paradigm performs translation initialization (meaning, the source node is moved from "language neutral" with no translation set, to the site default language + a translation set), the EntityTranslatableInterface::getSourceLanguage() method begins to return the language of the last imported piece of content, rather than the actual source language.

This is only made apparent when multiple XLIFFs for different languages are being imported for a single entity within a single request thread.

An example use-case: the Automatic Node Title module should be able to render the node title property untranslatable if a particular content type's title generation settings are configured as automatically generate the title and hide the title field.

Follow-up to #1 and #13. It's useful to have a concept of translation paradigm for nodes... But it also may be appropriate to bring this concept up to the level of EntityTranslatableBase.

This would definitely be helpful in the case of implementing a TaxonomyTermTranslatable class (e.g. i18n-based translation vs. entity field translation). But could also be helpful when dealing with Field Collection translatability.

Pre-requisite to #2.

The current NodeTranslatable::getTargetEntity call will sometimes initialize content translation (by updating the language from und to en and saving the original node). This should only happen on a EntityTranslatableInterface::setData, and even then, only when the "save data" flag is passed.

In the Entity Translation ticket, we'll need a similar initialization method... May be worth adding to EntityTranslatableInterace.

While implementing #76, discovered that it would be much simpler / more standard to call translation_node_prepare(), rather than doing a manual clone of the node.

Suppose you have a set of nodes that are part of a translation set...

Suppose you then add a field collection or paragraph item field to the content type, and populate the field collection / paragraph on the source language node.

If you export an XLIFF file from the source language, then attempt to import a translation, you will get an error like the following:

Recoverable fatal error: Argument 1 passed to EntityXliff\Drupal\Translatable\Content\FieldCollectionTranslatable::getHostEntity()
must be an instance of FieldCollectionItemEntity, boolean given,
called in /path/to/src/Drupal/Translatable/Content/FieldCollectionTranslatable.php on line 57
and defined in EntityXliff\Drupal\Translatable\Content\FieldCollectionTranslatable->getHostEntity()
(line 118 of /path/to/src/Drupal/Translatable/Content/FieldCollectionTranslatable.php).

Because, when it attempts to set the new embedded entity content values on the existing translation, it has no entity to set against...

This is not a problem for net-new translations in a set because the original content of the embedded entities is first duplicated.

Follow-up to #9: we should move the referenced-entity getting/setting used in the current entity translatable base to something like an EntityReferenceFieldHandler class.

In the class map building process on the field mediator, we'd just need to add entries for each entity type (via entity_get_info()).

We've not been doing any exception catching during development, but things are starting to settle down now and we should start preparing the module for production use.

Consequently, we should start catching Entity API exceptions and logging warnings or errors in watchdog / possibly setting message, rather than letting the exception bubble up.

We assume English is the source for all translations... This is also a limitation upstream in Eggs'n'Cereal.

The import of an XLIFF file fails and does not set any values. The title gets set to "Title", body field to "Body", etc.

Version: 7.x-1.x 338de36

The following warnings are thrown during the import process:

Notice: Trying to get property of non-object in EggsCereal\Utils\ConverterToHTML->toHTML() (line 42 of /opt/development/rh/webdrupal/html/sites/all/vendor/tableau-mkt/eggs-n-cereal/src/Utils/ConverterToHTML.php).
Warning: Invalid argument supplied for foreach() in EggsCereal\Utils\ConverterToHTML->toHTML() (line 42 of /opt/development/rh/webdrupal/html/sites/all/vendor/tableau-mkt/eggs-n-cereal/src/Utils/ConverterToHTML.php).

Problem

Occasionally, you might have a reference field that is used to reference the same content on a large set of content, such as referencing a parent node used to categorize a subset of child nodes. In this or similar cases, if you want to export all the child nodes, you'll end up exporting the parent node multiple times with each child node.

Proposed Resolution

This issue could be resolved by adding the ability to set a particular reference field to be ignored by entity_xliff. This could be done on a field-by-field on field settings or a blacklist of field machine names on an admin page.

Problem / motivation

If a field of type "Node Reference" (supplied by the node_reference sub-module) exists on a node translated via Content Translation, the translation set of the referenced node will become corrupt after the parent node is imported multiple times. The particular "corruption" is that multiple versions of a piece of content will exist for a given language in a single translation set:

mysql> select nid, type, language, title, tnid FROM node;
+-----+------+----------+---------------------------------+------+
| nid | type | language | title                           | tnid |
+-----+------+----------+---------------------------------+------+
|   1 | page | en       | Parent Node                     |    1 |
|   6 | page | en       | Child Node                      |    6 |
|  46 | page | fr       | Parent Node (FR)                |    1 |
|  47 | page | fr       | Child Node (FR)                 |    6 |
|  48 | page | de       | Parent Node (DE)                |    1 |
|  49 | page | de       | Child Node (DE)                 |    6 |
|  50 | page | fr       | Child Node (FR)                 |    6 | <-- Duplicate FR child node
+-----+------+----------+---------------------------------+------+
7 rows in set (0.00 sec)

In addition to really screwing up underlying reference trees (especially as nested references pile up), it prevents editing a piece of content manually through the UI because of an "invalid" error on the "language" field.

Proposed resolution

The key will be to disable node_reference_field_prepare_translation() in the context of an Entity XLIFF import.

We should write unit tests for solidified parts of the API.

• EntityXliff\Drupal\Translatable\Content\NodeTranslatable
• EntityXliff\Drupal\Translatable\Content\FieldCollectionTranslatable
• Both Translatable abstract base classes (at least some parts)
  • EntityFieldTranslatableBase
  • EntityTranslatableBase
• FieldMediator
• All instances of FieldHandlerInterface
• EntityTranslatableFactory?

Also, how about some as-simple-as-possible sanity behavioral tests for:

• Local tasks on relevant entities, including access checks.
• Basic path/to/%entity/as.xlf checks, also including access checks.
• Bare-bones translation attempts for each entity and paradigm, including referenced entities...
  • Entities
    • Node: Content Translation
    • Field Collection: Content Translation
    • Node: Entity Translation
    • User: Entity Translation
    • Term: Entity Translation
    • Comment: Entity Translation
  • Fields
    • Plain text
    • Formatted text
    • Image field
    • Link field

Things not to test:

• Drupalhandler

"Unknown data property alt." is the one that crops up (for image fields).

Problem / motivation

Still having issues saving entities that have entity reference fields with cardinality: on save, rather than overwriting the reference with a reference to the translation, it appends the referenced translation to the list.

I've seen this both with Field Collections and Node references.

Proposed resolution

Determine root cause, evaluate, fix...

We should document...

• Use for sitebuilders (very limited)
• How to provide translatables for custom entities.
• How to provide field handlers for custom fields.
• Drupal hook documentation:
  • hook_entity_xliff_presave_alter()
  • hook_entity_xliff_translatable_fields_alter()
  • hook_entity_xliff_get_xliff_alter()
  • hook_entity_xliff_set_xliff_alter()
  • hook_entity_xliff_translatable_source_alter()
  • hook_entity_xliff_target_entities_alter()

Probably hang it off of the Paragraphs i18n module, since it simplifies things.

Follow-up to last note in #5: We should consider stripping out custom field-type specific field handlers and instead making generic handlers like:

• AtomicFieldHandler - for scalar field types (e.g. text, integer, boolean, etc)
• CompositeFieldHandler - for composite fields encapsulating several atomic fields (e.g. formatted text or text with summary)
• EntityReferenceFieldHandler - for fields that reference whole entities that can consist of either of the above.

Some things to consider:

• Maybe create a "basic," non-abstract implementation of the TranslatableInterface that all fieldable entities will default to? But maybe not. That may make too many assumptions.
• At the very least, existing Translatable implementations should be converted to use this mechanism.

Entity API's validation is still strictly checking types when values are set. We should introduce an extremely simple IntegerFieldHandler that handles this type juggling for us.

There appears to be a bug in the way content is re-imported when it contains field collections.

Here's repro: https://travis-ci.org/tableau-mkt/entity_xliff/builds/105820068 (104f061)

We're using the Drupal language code for source/target language. This is a sane default for XLIFFs, but we should make it possible to alter it in the event that a different language/locale combination needs to be provided than the one specified by the Drupal language.

Per #112 and #114... We need a to add support for exporting the most current workbench moderation revision without breaking imports.

Turns out the "path" key for an entity's info (e.g. entity_get_info('entity_type')['path'];) is non-standard. Module development occurred in the context of a site that happened to have that info included (by Metatag module, for the record).

For core entities, we should just include this metadata via hook_entity_info_alter(). For contrib entities, we just need to document this oddity in #24.

Specifically, I'm thinking of nodes using the Content Translation paradigm which are not configured to be translated.

Presumably, there's a similar state for entity types/bundles using the Entity Field Translation paradigm.

There appears to be a bug in the way we pull target entities on initial import in situations where the target entity is determined multiple times on a single request... The bug is resolved by re-evaluating translation sets before pulling the target node, but I haven't yet found the circumstances for the bug / a solid repro...

Problem / motivation

In #106, we introduced a rollback mechanism for cases when the structure in an XLIFF file had become stale compared to live content.

In situations where multiple XLIFFs are handled on a single request, it's possible for an error in an earlier XLIFF import to trigger a rollback which undoes the creation of a translation set. In such cases, subsequent XLIFF imports (on the same request) can fail because the translation set is assumed to exist (because it exists across various static caches), but actually doesn't.

Proposed resolution

This seems to be best most easily caught when a Translatable attempts to load and manipulate the raw, underlying entity, but comes up empty-handed. In such cases, we should throw a new type of exception (maybe EntityGoneAwayException), catch it at the top, perform another rollback, and provide appropriate messaging to the user.

Seems like the best course of action for the user is to fix the stale XLIFF problem of a prior import, then try again.

Pre-requisite to #2... The logic required to save an entity via Entity Field Translation lives in the translation handler, not on the wrapper itself... Thus, we'll need to make the saveWrapper method non-static. Might also consider renaming it.

Currently, we have Node and Taxonomy modules providing translatable classes themselves... It probably makes more sense to split out the entity-by-entity translatable class info hooks to the modules that actually enable translation (like Entity Translation, and the Core Translation modules).

And even then, only if the module that provides the sub-entity is enabled (e.g. Entity Translation should only provide a translatable for Taxonomy Terms if the taxonomy module is enabled too).

This also has the benefit of making the process of providing alternative translation paradigms more straightforward (e.g. a class for Taxonomy Term translatability provided on behalf of the i18n_taxonomy module).

Problem / motivation

Suppose you have two nodes: a host node referencing a child node. If the child node had previously been translated (and thus, existed in a translation set), but the translation(s) had been deleted (so that the translation set is now empty), then subsequent imports for the host page will create corrupt translation sets.

Proposed resolution

This is caused by a bug in the way nodes translated via content translation are initialized and their targets are found.