node_reference_field_prepare_translation needs refactoring [#1359778]

I just tested content translation in Drupal 7 and got some notices in node_reference_field_prepare_translation when adding new translation for node.

Notice: Undefined index: nid in node_reference_field_prepare_translation() (line 881 of /vhosts/play.local/sites/all/modules/references/node_reference/node_reference.module).

And other the same notices about replacing $node with $entity etc.
One more question: bug http://drupal.org/node/362021 is closed, so this function can be implemented for now? I fixed errors in node_reference_field_prepare_translation and have seen it works.

function node_reference_field_prepare_translation($entity_type, $entity, $field, $instance, $langcode, &$items) {

  if (isset($entity->translation_source->$field['field_name'])
  && is_array($entity->translation_source->$field['field_name'])) {
    foreach ($entity->translation_source->$field['field_name'] as $key => $reference) {
      $reference_node = node_load($reference[0]['nid']);
      // Test if the referenced node type is translatable and, if so,
      // load translations if the reference is not for the current language.
      // We can assume the translation module is present because it invokes 'prepare translation'.
      if (translation_supported_type($reference_node->type)
      && !empty($reference_node->language)
      && $reference_node->language != $entity->language
      && $translations = translation_node_get_translations($reference_node->tnid)) {
        // If there is a translation for the current language, use it.
        if (isset($translations[$entity->language])) {
          $items = array();
          $items[] = array('nid' => isset($translations[$entity->language]));
        }
      }
    }
  }
  
}

Comment	File	Size	Author
#1	1359778-node-ref-translation.patch	2.14 KB	netsensei
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

netsensei CreditAttribution: netsensei commented 9 December 2011 at 11:26

Title:	"prepare_translation" generates notices when content translation enabled.	» node_reference_field_prepare_translation needs refactoring
Version:	7.x-2.0-beta3	» 7.x-2.x-dev

File	Size
1359778-node-ref-translation.patch	2.14 KB

Okay. I'm seeing this too.

This hook implementation badly needs some refactoring. :-)

1. When you translate the node, it doesn't create a reference to the translation of the referenced nodes. Rather, it reuses the references created in the original source node. And it triggers
2. The error is triggered because of changes to the Field API.
3. Your code doesn't take multiple elements into account. $reference_node = node_load($reference[0]['nid']); only translates the first element in the items array. Subsequent elements won't get translated.
4. The hook itself has been updated (http://api.drupal.org/api/drupal/modules--field--field.api.php/function/...)

The @todo in the comments is a giveaway. * @todo Correctly implement after http://drupal.org/node/362021 is fixed. ;-)

So, putting any reservations aside, I've tried to fix all of the above in one go. Patch attached.

A note: if you have 2 references on your source translation of which the second doesn't have a translation, this patch will just add the orginal (untranslated) reference to the 'parent' node. This might be confusing, and I'm not sure how to solve this. Should we keep it like this? Do we need to unset those references until they get a proper translation?

Comment #2

good_man CreditAttribution: good_man commented 9 December 2011 at 16:30

I didn't till now understand why this function is here? do the original design tells to only reference nodes from the same language? or what? Anyhow, removing the whole function or adding the patch end up with the same results, which is, all nodes from the same type are populated (default settings of the field).

Comment #3

C. Lee CreditAttribution: C. Lee commented 13 February 2012 at 05:52

#1 works for me.

Comment #4

netsensei CreditAttribution: netsensei commented 13 February 2012 at 10:48

Status:

Active

» Needs review

Putting this into "needs review". The patch works, but it's implementation might not support a range of possible use cases this hook is intended to cater.