it would be nice if views_attach could expose the translation nid (tnid) of the node we attach to as a views argument, as an alternative to the nid.

CommentFileSizeAuthor
#9 views_attach.tnid_.6-x-2-x.patch926 bytesdonquixote

Comments

donquixote’s picture

Ok.. as I understand this only requires that the tnid is exposed as a token.

donquixote’s picture

How to do this in a custom module:

<?php
function xxx_token_list($type = 'all') {
  if ($type === 'node' || $type === 'all') {
    $tokens['node']['tnid'] = t('Node translation nid (tnid)');
    return $tokens;
  }
}

function xxx_token_values($type, $object = NULL) {
  $values = array();
  if ($type === 'node') {
    $values['tnid'] = $object->tnid ? $object->tnid : $object->nid;
  }
  return $values;
}
?>

Related issues:
#533254: Token Starter: how to add a [tnid] token?
#297371: Provide tnid number token (nid of the "source node" and translations (duplicate)
#736178: Add a [node:source] token for source node of a translated node

Crell’s picture

I'm unclear here. If tnid is not being exposed as a token, then that's probably a bug against token. Should we refile this issue? It doesn't seem like a views_attach issue...

donquixote’s picture

Status: Active » Closed (duplicate)

The issue for token already exists, so all I can think of here is to mark this one as a duplicate.
#736178: Add a [node:source] token for source node of a translated node

donquixote’s picture

Title: Provide tnid argument » Provide tnid argument (w/o token)
Status: Closed (duplicate) » Active

I just lost my faith in token.
Yes, it does give me the tnid (via a custom module that implements hook_token_values)
However, token_get_values() always runs a crazy amount of code, and has a crazy amount of side effects.

In my particular case, I have 2 views with attach displays, both attached to the same node.
Now, the first attach display is generated and attached to $node->content['NewsSlider_node_content_1'].
Then the second attach display is generated and attached to $node->content['Documents_node_content_1'].

The tiny flaw of this:
- the second operation indirectly calls module_invoke_all('token_values')
- this calls content_token_values()
- this does a "$node->content = array()". (besides other things, like drupal_render($node->content))
- the NewsSlider stuff is wiped.

Nice, eh?
Btw, for some reason this all used to work for a while, and only recently (not sure what i changed) the slider view is gone.

Conclusion:
This is not the first time I found this kind of crazy side effects with token.
And besides, this also has performance implications, if a node is rendered only to get the tnid.
Token needs a lazy algorithm with preg_replace_callback(). But I don't see that happen.

For views_attach I think it's a good idea if we either introduce a php option, and/or a tnid option.
If you agree, I make a patch.

Crell’s picture

That sounds more like a bug in token to me, doesn't it?

I'm extremely wary about a PHP option, as those are all, without exception, security risks. Sometimes acceptable risks, but still a security risk, and I do not like introducing security risks.

If this cannot be fixed in token, I'd be OK with a tnid option.

Note though that this module is going into maintenance mode, as it has been supplanted in Drupal 7 by EVA. I'm not planning to do much with this module anymore.

donquixote’s picture

> That sounds more like a bug in token to me, doesn't it?

Or in content, if you want.
I think preg_replace_callback() would fix it, but I have my doubt this will be easy.
#1307890: token_replace() performance when there are no tokens

> I'm extremely wary about a PHP option, as those are all, without exception, security risks. Sometimes acceptable risks, but still a security risk, and I do not like introducing security risks.

I would agree if this was something other than Drupal..

> If this cannot be fixed in token, I'd be OK with a tnid option.

Cool!

donquixote’s picture

I think the preg_replace_callback() on token is too much of an adventure

Trying to make a patch for views_attach tnid option instead. Looks ok so far.
One question: I don't understand what this is about, in options_submit():

<?php
        if ($form_state['values']['argument_mode'] == 'token') {
          $this->set_option('default_argument', $form_state['values']['default_argument']);
        }
        else {
          $this->set_option('default_argument', NULL);
        }
?>

I checked, and defautl argument was just an empty string. So it is empty string vs NULL.
I guess I need to do something else if the value is 'tnid' ?

EDIT:
Got it, that's the text field to enter the token string :)

donquixote’s picture

Status: Active » Needs review
StatusFileSize
new926 bytes

That's a simple patch.
I wonder if we need a fallback to nid, if there is no tnid to be found? Can this even happen at all? Would this be yet another option then?

Crell’s picture

I, um, don't know enough about how tnids work to say if we should fallback. We should follow whatever is convention there. If so, it should not be an option. Fallback or don't.

donquixote’s picture

It's more than that.
We could
a) Require non-empty tnid, or otherwise show an empty view / display all values.
b) Fall back to nid.
c) Just let it be tnid = 0, and probably see weird results. This is what we would get with token [tnid].

I could imagine use cases for both (a) and (b), but I am not sure what would be the correct implementation of each.

donquixote’s picture

Maybe I should describe my own use case a little bit (simplified).

Say, I have one node type "document", and one node type "document collection".
The "document" node type has a (single-value) nodereference to the "document collection" node type.
Document collections exist in English and French, associated as translations of each other.

When I visit a French document collection node, I want to get a list of all documents which are reference either this doc collection, or its English translation.

I solve this with a views relationship based on the nodereference, plus a views_attach tnid argument.
If one of the doc collection nodes would have an empty tnid, then I would like to fall back to its nid. But then I would also have to write a custom argument handler, or otherwise it just would not work.

Crell’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)