| Project: | Advertisement |
| Version: | 6.x-2.2 |
| Component: | ad module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
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:
<?php
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:
<?php
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.
Comments
#1
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.
#2
Subscribe, greetings, Martijn
#3
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
#4
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
#5
Are there other queries in the module that need to respect revisions?
#6
The 'default' case just below the 'tids' case also needs to be update. Patch attached.
#7
#8
Automatically closed -- issue fixed for 2 weeks with no activity.
#9
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);
}