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

webflo’s picture

webflo’s picture

Title: Implement update hooks for node translation sets. Clean-up Node translation sets. » Implement update hooks for node translation sets. Clean-up node translation sets.
webflo’s picture

Status: Active » Needs review
StatusFileSize
new4.82 KB

Here we go. Is that better?

jose reyero’s picture

Status: Needs review » Needs work

The 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.

nedjo’s picture

If 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?

jose reyero’s picture

After 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().

nedjo’s picture

@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 in hook_node_update(). However, this won't in itself trigger the hook_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 of hook_node_update() parallel to the existing hook_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() and hook_node_translation_change(), but that would be more error prone.

mgifford’s picture

Sounds 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.