In the Drupal 6 taxonomy system, terms are attached to nodes per revision. This means that if a node is removed from a taxonomy term, and a new revision is created, an entry will persist in the term_node table for the old revision. (This is a change from Drupal 5, which only maintained a "current" term state per node.)

The part of the ad module that fetches ads based on the categories does not appear to be aware of this. Ads are still appearing in the categories they used to be in. In cases where the old category is displayed on the same page as the new category, and the old category appears first, this can also stop them appearing in the categories their active revision is actually in.

I've had a quick shufty through the code and I think this is broadly the responsibility of the function adserve_cache_id() (in adcache.inc) - for example:

    case 'tids':
      $result = db_query("SELECT a.aid FROM {ads} a INNER JOIN {term_node} n ON a.aid = n.nid WHERE a.adstatus = 'active' AND n.tid IN(%s)", $id);

(There are a few more entries with other cases, some of which also mention term_node. Presumably the changes outlined below would need to be applied to the other cases as well.)

In order for this query to respect the revision system, this needs to be modified to incorporate a reference to the node table. Something like this:

    case 'tids':
      $result = db_query("SELECT a.aid FROM {ads} a INNER JOIN {node} n ON a.aid = n.nid INNER JOIN {term_node} t ON n.vid = t.vid WHERE a.adstatus = 'active' AND t.tid IN(%s)", $id);

Note the inner join from node to term_node on VID.

I've tested this one change on a local site and it appears to work correctly as far as it goes, but I'm not familiar enough with the intricacies of the ad module to know if that's all that needs to be done.

CommentFileSizeAuthor
#6 ad-taxonomy-search-honor-revisions.patch1.73 KBjohn franklin

Comments

alanburke’s picture

alanburke commented

Subscribe.
Came across a related issue, I think.
Where a new revision is created for an advertisement node,
the listing of ads at admin/content/ad
shows the ad in the same region multiple times.

summit’s picture

summit commented

Subscribe, greetings, Martijn

str1’s picture

str1 commented

Subscribing ... wasted my time figuring this one out, for others having the same issue: turn off node revisions for ads and delete all revisions for existing nodes

Molot’s picture

Molot commented

Solution seems to work for me.
If you would create a patch I might review it ant then we would make it easier for other ppl here. My review has no official meaning, but I'll be using it on site that easily gets 1.5k simultaneous sessions, so you know.

Now to make it work with revision moderation :D

john franklin’s picture

john franklin commented

Are there other queries in the module that need to respect revisions?

john franklin’s picture

john franklin commented
Status: Active » Needs review
StatusFileSize
new ad-taxonomy-search-honor-revisions.patch1.73 KB

The 'default' case just below the 'tids' case also needs to be update. Patch attached.

john franklin’s picture

john franklin commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

jonathan_hunt’s picture

jonathan_hunt commented
Status: Closed (fixed) » Needs work

The admin page at admin/content/ad still shows multiple ad groups if an ad node has revisions. The following changes worked for me:

diff --git a/modules/ad/ad.module b/modules/ad/ad.module
index 9692508..1a16e2b 100644
--- a/modules/ad/ad.module
+++ b/modules/ad/ad.module
@@ -1592,10 +1592,15 @@ function _ad_get_group($nid) {
   static $groups = array();
 
   if (!isset($groups[$nid])) {
-    $result = db_query('SELECT d.name FROM {term_data} d LEFT JOIN {term_node} n ON d.tid = n.tid WHERE n.nid = %d AND d.vid = %d', $nid, _ad_get_vid());
-    while ($term = db_fetch_object($result)) {
-      $terms[] = $term->name;
+    $ad_terms = taxonomy_node_get_terms_by_vocabulary(node_load($nid), _ad_get_vid());
+
+    $terms = array();
+    if (!empty($ad_terms)) {
+      foreach ($ad_terms as $term) {
+        $terms[] = $term->name;
+      }
     }
+
     if (!empty($terms)) {
       $groups[$nid] = implode(', ', $terms);
     }
lrwebks’s picture

lrwebks commented
Status: Needs work » Closed (outdated)

Drupal 6 is EOL and no longer supported. Closing this as outdated for that reason. Thanks for your contribution!