Some background info:
I have a site that uses termpath-raw aliases for some node types. I noticed that sometimes when the assigned terms changed the alias did not update accordingly or reverted to a very old version. Deleting existing aliases and recreating them didn't help either.

The issue lies in pathauto_token_values() function. For a node it attempts to retrieve taxonomy related data using the following query:

$category = db_fetch_object(db_query_range("SELECT t.tid, t.name FROM {term_data} t INNER JOIN {term_node} r ON r.tid = t.tid WHERE t.vid = %d AND r.nid = %d ORDER BY weight", $vid, $object->nid, 0, 1));

This query should account for different versions of the term assignment eg. if my node was tagged with "foo", then "bar" and eventually "baz" in the same vocabulary then this query will return the term based on term weight and not the currently assigned term.
The solution to fix it is actually simple:

$category = db_fetch_object(db_query_range("SELECT t.tid, t.name FROM {term_data} t INNER JOIN {term_node} r ON r.tid = t.tid WHERE t.vid = %d AND r.nid = %d ORDER BY r.vid DESC, weight", $vid, $object->nid, 0, 1));

Fixing this query is only half of the problem - Token module borrowed this functionality in node_token_values().
Instead of 

$term = db_fetch_object(db_query_range("SELECT t.tid, t.name FROM {term_data} t INNER JOIN {term_node} r ON r.tid = t.tid WHERE t.vid = %d AND r.nid = %d ORDER BY weight", $vid, $object->nid, 0, 1));

it should be

if (!isset($term->name)) {
  $term = db_fetch_object(db_query_range("SELECT t.tid, t.name FROM {term_data} t INNER JOIN {term_node} r ON r.tid = t.tid WHERE t.vid = %d AND r.nid = %d ORDER BY r.vid DESC", $vid, $object->nid, 0, 1));
}
else {
  $term = db_fetch_object(db_query_range("SELECT t.tid, t.name FROM {term_data} t INNER JOIN {term_node} r ON r.tid = t.tid WHERE t.vid = %d AND r.nid = %d ORDER BY weight", $vid, $object->nid, 0, 1));
}

Since this issue spans across two different modules I'm not sure how to submit patches. Any suggestions?

CommentFileSizeAuthor
#13 423132_pathauto_term_wrong_revisioning_compatible_v2.patch1010 byteseltrufa
#11 423132_pathauto_term_wrong_revisioning_compatible.patch448 byteseltrufa
#3 423132_pathauto_term_wrong_revision.patch1.27 KBgreggles
#2 pathauto.module_423132.patch1.21 KBnonsie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

Great find. Can you just make two issues and submit two patches?

nonsie’s picture

nonsie
she/her
Primary language Estonian
CreditAttribution: nonsie commented
Status: Active » Needs review
FileSize
pathauto.module_423132.patch1.21 KB

Patch for Pathauto attached.
Patch for Token can be found here - http://drupal.org/node/423322

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
FileSize
423132_pathauto_term_wrong_revision.patch1.27 KB

This makes sense, but perhaps even better would be a change like the attached. Thoughts?

nonsie’s picture

nonsie
she/her
Primary language Estonian
CreditAttribution: nonsie commented
Status: Needs review » Reviewed & tested by the community

+1 This seems like a better solution than mine

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Title: Incorrect retrival of node's taxonomy terms » Incorrect retrival of node's taxonomy terms with revisions

Applied to

6.x-1.x: http://drupal.org/cvs?commit=199982
5.x-2.x: http://drupal.org/cvs?commit=199984
6.x-2.x: http://drupal.org/cvs?commit=199992

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Closed (fixed) » Needs work

#485210: SQL Error - Unknown column 'r.vid' in 'where clause' query

I think the patches for 5.x should be reverted. Right?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Needs work » Fixed

Fixed - http://drupal.org/cvs?commit=223522

Status: Fixed » Closed (fixed)

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

eltrufa’s picture

eltrufa CreditAttribution: eltrufa commented
Status: Closed (fixed) » Active
FileSize
423132_pathauto_term_wrong_revisioning_compatible.patch448 bytes

The current implementation of pathauto_token_values() has a problem when the revisioning module is enabled.

In this case the where clause of the second query should filter by the actual node revision (the one indicated by $object->vid value).

I attached a very simple patch to resolve this issue.

Thanks for this excelent module.

Leandro

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Active » Needs review

The revisioning module?

Seems reasonable enough. Anyone else care to check?

eltrufa’s picture

eltrufa CreditAttribution: eltrufa commented
FileSize
423132_pathauto_term_wrong_revisioning_compatible_v2.patch1010 bytes

Yes, the revisioning module.

In my previous patch, I only considered the bulk node alias generation, but I didn’t consider saving a pending revision. In this case, the $object->vid value corresponds to the saved revision, not to the current node revision (indicated by vid column value in the node table). For this reason, a join with node table is required.

The new node alias shouldn’t be generated until the new revision is approved. The alias could be generated using the rules module.

I attached a new version of the patch and also submited the corresponding patch to the token module (#423322: Incorrect retrival of node's taxonomy terms with revisions).

I hope this will be the good one.

Leandro

eft’s picture

eft CreditAttribution: eft commented

I had same issue outlined at top of this thread. I already had the patched versions of pathauto so it seems the official patch has not resolved the issue in all cases. I did find, however, that Leandro's patch in comment #13 worked for me (even though I didn't have the revisioning module installed).

tseven’s picture

tseven CreditAttribution: tseven commented

I experienced the exact same issue with versions enabled. The patch in #13 fixed the issue using the current development version 6.x-1.x-dev.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Version: 6.x-1.1 » 6.x-1.x-dev

The token patch still has a follow-up like this to be committed. Does this still apply or cause problems?

Status: Needs review » Needs work

The last submitted patch, 423132_pathauto_term_wrong_revisioning_compatible_v2.patch, failed testing.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Strike me silly, but shouldn't we be able to just use what's available in $node->taxonomy at that point? I'm trying to write tests to back this up but I think this should work:

      // Tokens [termpath], [termpath-raw], and [termalias].
      if (module_exists('taxonomy') && !empty($object->taxonomy)) {
        // Get the lowest-weighted term from the lowest-weighted vocabulary.
        $tids = array_keys($object->taxonomy);
        $tid = db_result(db_query_range("SELECT td.tid FROM {term_data} td INNER JOIN {vocabulary} v ON td.vid = v.vid WHERE td.tid IN (" . db_placeholders($tids) . ") ORDER BY v.weight, td.weight, td.name", $tids, 0, 1));
        $values = array_merge($values, _pathauto_token_values('taxonomy', $object->taxonomy[$tid], 'term'));
      }

We don't have to join on {term_node} or anything else. And this way it's up to taxonomy.module or whatever to make sure the right data is in $node->taxonomy.

EDIT: Nevermind, this failed horribly and only works on node load, not on node save.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Assigned: Unassigned » Dave Reid
bgilday’s picture

bgilday CreditAttribution: bgilday commented

subscribing

marcoBauli’s picture

marcoBauli CreditAttribution: marcoBauli commented

In my case the [term-raw] token in the alias gets completely omitted (removed) as soon as a new revision is posted..

is there by chance a more recent patch that applies? thanks

PostScriptum: tried applying manually, but the Token patch related to this issue is in wrong format, so guess it cannot work anyway..

nonsie’s picture

nonsie
she/her
Primary language Estonian
CreditAttribution: nonsie commented
Status: Needs work » Fixed

This is resolved in 6.x-1.4 release

Status: Fixed » Closed (fixed)

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