Scenario:

- Content type set to 'Enabled, with translation'
- The body field set to 'translatable' (as Entity translation module is installed as well)

Manual translation works fine.

TMGMT returns the translated body correctly (as seen in the review). But it does not get saved in the right place. The translated node view shows the source language text in body, the edit view shows an empty field.

CommentFileSizeAuthor
#5 translatable-node-translation-2011744-5.patch11.4 KBberdir
#5 translatable-node-translation-2011744-5-interdiff.txt9.02 KBberdir
#2 translatable-node-translation-2011744-2.patch3.62 KBberdir

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Priority: Normal » Major
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new translatable-node-translation-2011744-2.patch3.62 KB

I think this is how it should be. This will probably fail tests but then our tests are wrong. And if it doesn't, then we need better test coverage.

cgalli’s picture

cgalli commented
Status: Needs review » Reviewed & tested by the community

Before patch: the translated field came back to review correctly, but it never showed up in the translated node.

After patch: the translation is saved to the correct node translation

RTBC assuming the test will pass eventually.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs work

Needs work because the tests *did* pass.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
Issue tags: -Needs work
StatusFileSize
new translatable-node-translation-2011744-5-interdiff.txt9.02 KB
new translatable-node-translation-2011744-5.patch11.4 KB

That's why writing tests is important, fixed a stupid variable overwrite bug with multiple fields.

Cleaned up node source tests, they were quite a mess, nobody looked at those in ages.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/tests/tmgmt.base.entity.testundefined
@@ -27,8 +27,11 @@ abstract class TMGMTEntityTestCaseUtility extends TMGMTBaseTestCase {
+   * @pparam bool $attach_fields

@param :-)

Otherwise (interdiff) looks fine.

+++ b/sources/field/tmgmt_field.moduleundefined
@@ -89,20 +89,26 @@ function tmgmt_field_get_source_data($entity_type, $entity, $langcode, $only_tra
     if ($field = field_info_field($field_name)) {
       $function = $field['module'] . '_field_type_tmgmt_populate_entity';
       list(, , $bundle) = entity_extract_ids($entity_type, $entity);
       $instance = field_info_instance($entity_type, $field_name, $bundle);
       if (function_exists($function)) {
-        $function($entity_type, $entity, $field, $instance, $langcode, $data, $use_translatable);
+        $function($entity_type, $entity, $field, $instance, $langcode, $data, $use_field_translation);

Usually i would expect we use some kind of hook_info to define callbacks. There's no such here, right?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

It is defined in an info hook. That hook is called hook_field_info(). You don't specify every callback there, it's based on the name. If you define field types and define the mymodule_field_update() function, it's called and otherwise it's not. No additional registration necessary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Fixed

Fixed typo and committed.

Status: Fixed » Closed (fixed)

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