As per http://drupal.org/node/1697182, we should replace calls to get the $node->title with the $node->label() method instead. This patch covers the existing (already converted) tests, and one instance in search.views.inc. The other (3) instances I can take care of in the sandbox, as they are in the plugins.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

So the node.title field handler should call $node->label() as well?

damiankloip’s picture

Maybe, I did a commit in the sandbox earlier of the obvious calls to this on in the plugins : http://drupalcode.org/sandbox/damiankloip/1685040.git/commitdiff/6410651

aspilicious’s picture

Thats incorrect:

"However, setting the node title directly will still use $node->title = $new_title;."

damiankloip’s picture

What part is incorrect? I know we should be using $node->title if we are setting but I don't see where are we setting the node title in this patch or the sandbox. In views we aren't really going to be setting anything on nodes.

aspilicious’s picture

Ah I thought "$item->title = $node->label();" was setting the node title direclty but I was wrong :)

dawehner’s picture

FileSize
10.18 KB

Some more places...

damiankloip’s picture

Good spot! Looks good.

tim.plunkett’s picture

Status: Needs review » Needs work

What about views_handler_argument_node_vid and views_handler_argument_node_nid?

dawehner’s picture

Status: Needs work » Needs review
FileSize
11.94 KB

URGS, anyone with a better idea?

    // @todo: Come up with a better and more performant solution.
    $nodes = node_load_multiple($this->value);
    $result = db_query("SELECT n.title, n.nid FROM {node_revision} n WHERE n.nid IN (:nids)", array(':nids' => $this->value));
    foreach ($result as $revision) {
      $nodes[$revision->nid]->set('title', $revision->title);
      $titles[] = check_plain($nodes[$revision->nid]->label());
    }
    return $titles;
damiankloip’s picture

FileSize
12.04 KB

Hmm, not sure. I can't really think of a clean way to do this either. We do need to change the query/logic slightly though I think, as we can't just node_load_multiple($this->value) as that will use vids as nids.

    $results = db_query("SELECT nr.vid, nr.nid, nr.title FROM {node_revision} nr WHERE nr.vid IN (:vids)", array(':vids' => $this->value))->fetchAllAssoc('vid', PDO::FETCH_ASSOC);

    $nids = array();
    foreach ($results as $result) {
      $nids[] = $result['nid'];
    }

    $nodes = node_load_multiple(array_unique($nids));

    foreach ($results as $result) {
      $nodes[$result['nid']]->set('title', $result['title']);
      $titles[] = check_plain($nodes[$result['nid']]->label());
    }
dawehner’s picture

OH i see this handler is basically broken in 7.x as well

dawehner’s picture

Status: Needs review » Fixed

Tested a bit and committed them.

I will patch all other instances of entities in another issue.

Status: Fixed » Closed (fixed)

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