I have come across the situation that the tnid of a translation node is always set to 0 instead of the source node's nid.

I have the content type set up with Workflow and Trigger modules such as that when a workflow state is set to "approved", this will trigger the action to set the node to be "published".

I have verified that the translation module did set the tnid to the source id when I first submitted the new translation node. But immediately, the tnid value is set to zero by another module and I suspect this is by the Trigger module.

By examining the function translation_nodeapi() of the Translation module. I noticed that in the "insert" case, it will update the tnid of the new node in the database. However, if another module (and in this case, the Trigger module) makes use of the hook_nodeapi(), this module will not see the new tnid value and may override the database value.

I think whenever the translation module updates some values of the node in the database, it should update the memory values so other module will see those new values.

Files: 
CommentFileSizeAuthor
#21 357785_tnid_save_D6.patch758 bytesmiro_dietiker
#16 translation-357785-16.patch3.27 KBplach
PASSED: [[SimpleTest]]: [MySQL] 25,316 pass(es).
[ View ]
#16 translation-357785-16-test.patch2.71 KBplach
FAILED: [[SimpleTest]]: [MySQL] 25,295 pass(es), 6 fail(s), and 0 exception(es).
[ View ]
#14 translation-357785-14.patch2.56 KBplach
FAILED: [[SimpleTest]]: [MySQL] 25,318 pass(es), 1 fail(s), and 1 exception(es).
[ View ]
#14 translation-357785-14-test.patch2 KBplach
FAILED: [[SimpleTest]]: [MySQL] 25,300 pass(es), 6 fail(s), and 1 exception(es).
[ View ]
#12 translation-357785-12.patch2.56 KBplach
PASSED: [[SimpleTest]]: [MySQL] 19,208 pass(es).
[ View ]
#9 translation-357785-9.patch2.76 KBplach
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in translation-357785-9.patch.
[ View ]
#4 357785_tnid_save.patch523 bytesmiro_dietiker
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 357785_tnid_save.patch.
[ View ]
#7 357785_tnid_save_D7.patch389 bytesmiro_dietiker
Passed on all environments.
[ View ]
#5 357785_tnid_save_D7.patch349 bytesmiro_dietiker
Unable to apply patch 357785_tnid_save_D7.patch
[ View ]

Comments

mdupont’s picture

Subscribing. This issue seems not so minor to me.

rvalery’s picture

I came accross this bug by creating a new module, and overriding nodeapi hook. I'm quite new to Drupal so I don't really know if it's the right way to do it, but it seems to work for me now.

function moduleMenu_nodeapi (&$node, $op, $teaser, $page) {
switch ($op) {
case 'insert':
drupal_set_message ( print_r($node, true) ) ;
if ( isset($node->translation_source->nid) ) {
$tnid = $node->translation_source->nid ;
} else {
$tnid = $node->nid ;
}
$result = db_query("UPDATE {node} SET tnid = %d WHERE nid = %d", $tnid, $node->nid);
if ( $result ) {
drupal_set_message ("Translation update was done successfully.") ;
} else {
drupal_set_message ("An error occured during translation update.") ;
}
break ;
}
}
arnoldc’s picture

Yes, I have a similar code in my module (the existence of it is just to work around this issue) to work around the problem. A quick glaze, the only different is that I also set the translate field to be zero. It has been a while and I forgot why? lol

I also set the weight of my module to be 1 so it will be executed later than Trigger module which has a weight of zero.

function parasun_translate_nodeapi(&$node, $op, $teaser, $page) {
  // Only act if we are dealing with a content type supporting translations.
  if (!translation_supported_type($node->type)) {
    return;
  }

  switch ($op) {
    case 'prepare':
      break;

    case 'insert':
      if (!empty($node->translation_source)) {
        $tnid = $node->translation_source->nid;      
        db_query("UPDATE {node} SET tnid = %d, translate = %d WHERE nid = %d", $tnid, 0, $node->nid);     
      }
      break;

    case 'update':
    case 'delete':
      break;
  }
}

miro_dietiker’s picture

Version:7.x-dev» 6.8
StatusFileSize
new523 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 357785_tnid_save.patch.
[ View ]

This is really ugly. If $node finally leads to resave within the same refresh, tnid is set to 0 again after being set.
I think translation.module at least should update the $node object to represent the current state. It seems so obvious... and possibly should be considered a core bug.

Can someone please check this bug in D7? Is there a D7 port needed to make this into core?

Meanwhile a custom module might override $node->tnid to make persistency work this way as a workaround:

// tnidpreserve.module
function tnidpreserve_nodeapi(&$node, $op, $teaser, $page) {
  switch ($op) {
    case 'insert':
      if (!empty($node->translation_source)) {
        $node->tnid = $node->translation_source->tnid;
      }
      break;
  }
}

(Please note translation_source->tnid instead of nid which is wrong if you translate from non-tnid as source)

miro_dietiker’s picture

Title:tnid value update may be overrided by other modules» tnid value lost in case of node resave
Version:6.8» 7.x-dev
Priority:Minor» Normal
Status:Active» Needs review
StatusFileSize
new349 bytes
Unable to apply patch 357785_tnid_save_D7.patch
[ View ]

Checked D7 and still found this issue to be present based on code.

Attached a simple fix to make tnid persistent after save.

Can someone please make a test to bring this in?

Status:Needs review» Needs work

The last submitted patch, 357785_tnid_save_D7.patch, failed testing.

miro_dietiker’s picture

Status:Needs work» Needs review
StatusFileSize
new389 bytes
Passed on all environments.
[ View ]

retrying patch from drupal root..

Minoo’s picture

Version:6.8» 7.x-dev

#4: 357785_tnid_save.patch queued for re-testing.

plach’s picture

StatusFileSize
new2.76 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in translation-357785-9.patch.
[ View ]

The patch in #7 works as expected and fixes the issue, the attached one adds a simpletest that proves it. This should be ready to go now. Please confirm and set this RTBC.

miro_dietiker’s picture

Looks great. Thanks for adding the test.
Didn't test the simpletest in real till now, but the code / app logic seems fine.
Someone else please review and set it RTBC to make it pass.

Status:Needs review» Needs work

The last submitted patch, translation-357785-9.patch, failed testing.

plach’s picture

Status:Needs work» Needs review
StatusFileSize
new2.56 KB
PASSED: [[SimpleTest]]: [MySQL] 19,208 pass(es).
[ View ]

rerolled from Eclipse (i.e. without the -p options)

plach’s picture

Issue tags:+Quick fix
plach’s picture

StatusFileSize
new2 KB
FAILED: [[SimpleTest]]: [MySQL] 25,300 pass(es), 6 fail(s), and 1 exception(es).
[ View ]
new2.56 KB
FAILED: [[SimpleTest]]: [MySQL] 25,318 pass(es), 1 fail(s), and 1 exception(es).
[ View ]

Here is a reroll agains the current head. Attached there is also the test-only patch which shows where the bug is and therefore is supposed to fail tests.

Status:Needs review» Needs work

The last submitted patch, translation-357785-14-test.patch, failed testing.

plach’s picture

StatusFileSize
new2.71 KB
FAILED: [[SimpleTest]]: [MySQL] 25,295 pass(es), 6 fail(s), and 0 exception(es).
[ View ]
new3.27 KB
PASSED: [[SimpleTest]]: [MySQL] 25,316 pass(es).
[ View ]

Rerolled to match a change in the test function. Again the test-only version is supposed to fail.

plach’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, translation-357785-16-test.patch, failed testing.

plach’s picture

Status:Needs work» Reviewed & tested by the community

Since I reviewed and tested the patch hunk that fixes the issue (which has been originally provided by @miro_dietiker), and the bot tells us that the test covers the issue correctly, I'm marking this RTBC.

webchick’s picture

Version:7.x-dev» 6.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed to HEAD. Thanks!

Marking for back-port to D6.

miro_dietiker’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new758 bytes

Providing patch for D6. Currently untested.

plach’s picture

Status:Needs review» Reviewed & tested by the community

Patch works as expected.

alexanderpas’s picture

Priority:Normal» Major

confirming RTBC, and upgrading to major, as this bug simply makes the translation module "useless" when used together with Workflow, Trigger, Rules and similar modules, and actively destroys translation relations in certain circumstances when used together with those modules.

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Fixed

Oh, nasty bug. Thanks for the fix, committed.

Dret’s picture

Another modules is affected by this problem, this: http://drupal.org/node/995500#comment-3865998

The modules fix another drupal bug: a single node without translation is considered as NOT part of "translation group" with result to show always all icons in language switcher blocks even if only one is available.

It work fine in 6.19 but not in 6.20, where translation system result broken.

klonos’s picture

If this major bug exists in 6.20, then a 6.21 should be released really soon IMHO. This effects all multilingual sites, so at least should be in the 'known issues' section of 6.20 in bold!.

Dret’s picture

Status:Fixed» Active

This problem was described for so long time here: http://drupal.org/node/518364#comment-2870818

But seems to be considered a minor issue and probably will not be fixed in core.

miro_dietiker’s picture

Status:Active» Fixed

I almost can't believe this breaks a module that implements nodeapi correctly.

Having a $node's tnid=0 after adding a node to a translation group must be considered a mistake. No module should have relied on this ever.
If there's something broken (newly), please describe the issue clearly here. In any other case the new tnid behaviour is as-it-should-be.

The object $node is now (after a tnid attachment/save) exactly in the same state as it is if the node was reloaded again. This is perfectly fine.

tnid for a single new node (without any translations) had a tnid=0 SINCE EVER. We didn't intend to change this behaviour ever with this issue here.
I can't follow you, why this is something like a new bug we introduced.

Reopen if you can provide more info. Switching back to fixed as we did successfully.

Status:Fixed» Closed (fixed)
Issue tags:-Quick fix

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