Problem/Motivation
Other module can not react on node translation. Everything is done in form submit handler (see i18n_node_select_translation() / i18n_node_select_translation_submit()). There are not hooks. This is bad.
This is the current code for updating translation sets. There are no APIs involved, just a couple of queries.
// ...
$translations = $node->tnid ? translation_node_get_translations($node->tnid) : array($node->language => $node);
foreach ($translations as $trans) {
$current[$trans->language] = $trans->nid;
}
$update = array($node->language => $node->nid) + array_filter($form_state['values']['translations']['nid']);
// Compute the difference to see which are the new translations and which ones to remove
$new = array_diff_assoc($update, $current);
$remove = array_diff_assoc($current, $update);
// The tricky part: If the existing source is not in the new set, we need to create a new tnid
if ($node->tnid && in_array($node->tnid, $update)) {
$tnid = $node->tnid;
$add = $new;
}
else {
// Create new tnid, which is the source node
$tnid = $node->nid;
$add = $update;
}
// Now update values for all nodes
if ($add) {
db_update('node')
->fields(array(
'tnid' => $tnid,
))
->condition('nid', $add)
->execute();
if (count($new)) {
drupal_set_message(format_plural(count($new), 'Added a node to the translation set.', 'Added @count nodes to the translation set.'));
}
}
if ($remove) {
db_update('node')
->fields(array(
'tnid' => 0,
))
->condition('nid', $remove)
->execute();
drupal_set_message(format_plural(count($remove), 'Removed a node from the translation set.', 'Removed @count nodes from the translation set.'));
}
// ...
Proposed resolution
Refactoring i18n_node_select_translation_submit(). Decouple CRUD from form submission. Implement update hooks.
Remaining tasks
API Tests
Code
User interface changes
None.
API changes
API additions, wont affect existing modules.
Original report by [webflo]
...
Comments
Comment #1
webflo commentedRelated issue: #1349566: Add tests for content translation
Comment #2
webflo commentedComment #3
webflo commentedHere we go. Is that better?
Comment #4
jose reyero commentedThe idea looks good to me, though I wish we could build something more generic for all translation sets. Since we already have a i18n_translation_set class, maybe we could reuse that.
(However it is not clear to me whether these node translation sets should be entities too).
Also if we are introducing any new hook, it needs documenting in an api.php file.
Comment #5
nedjoIf we're introducing a hook, presumably it should be invoked for all translation set changes, not just for those that are initiated by i18n. See http://drupal.org/project/translation_helpers for a related hook. Is this relevant?
Comment #6
jose reyero commentedAfter giving it a try (node translation set object) it just gets too complex.
I think we better reuse nedjo's module.
@nedjo,
Right, that would be a more useful hook. Do we need to add anything here for the hook to be triggered?
I mean from i18n_node_select_translation_submit().
Comment #7
nedjo@Jose Reyero: One approach would be to reset properties on the node to be saved and then call
node_save()rather than directly manipulating the node data. Then modules could respond inhook_node_update(). However, this won't in itself trigger thehook_node_translation_change()in translation_helpers, because in implementing that I responded only to node deletion, on the assumption (incorrect) that the only way a translation set source could change was through the source being deleted. So this approach would also require changes to translation_helpers, probably an implementation ofhook_node_update()parallel to the existinghook_node_delete()one. I opened #1510662: Respond to change in node translation set on node update to cover this.Alternately, it would be possible to simulate a changed node, then trigger
hook_node_update()andhook_node_translation_change(), but that would be more error prone.Comment #8
mgiffordSounds like there's some of @nedjo's code here which could be used to help clean up some node translation sets.
I'm not sure what the hold up is on this issue. Would love to see this problem resolved.