Restricting display of ads by taxonomy terms

Please review INSTALL.txt and README.txt that comes with the module. You can create ad groups that match your age groups, which should accomplish your requirements. (Note that the 6.x version of this module is not yet fully completed, however, so you could run into bugs.)

I too would like to have this feature in the ad module.
I thought that the feature "display ads based on node ids (nids), or taxonomy terms (categories)" meant what you are asking for, but maybe I was wrong.

However I found a way:
- install the pathauto module;
- go to Administer › Site building › URL aliases › Automated alias settings and enter a default path pattern for nodes (i.e. [termpath-raw]/[title-raw]) and taxonomy terms (i.e. [vocab-raw]/[catpath-raw])
- create ad groups like Baby-Banner, Baby-Sidebar, etc.
- set the new blocks to be displayed only in the corresponding pages, i.e. baby/*

I hope this helps.

It would be nice to have this feature inside the ad module, so we could not have to use pathauto and we could set an ad to be displayed for nodes with a specific tag too, or for nodes with a particular content type.

I find the solution of sersim a little cumbersome and not realistic.

If I got many taxonomies categories ("baby" at sersim example), it actually mean that I'll need to create for each Taxonomy category of my site 3 corresponding ad groups (for 3 different locations and banner sizes), I'll also need to associate each banner to lots of groups during the creation if I want a specific banner to be displayed across my site.

There isn't any way to call for an ad with the ad function (or whatever other dynamic way :)) in a format which state the ad group id I want and the term id of the site category that I want? (for example the 'baby' category got term id of 20 and the ad group 'wide banners' got id of 30 I'll just call the ad function like ad(20, 1, 30)).

If this feature doesn't exist how can it be implemented otherwise?

Thanks

forgot to reopen :)

** and noticed that it was for the 6.x-1 and not 6.x-2 version... oops... sorry :)

I found that I can make such a functionality by editing the adserve.inc file and adcache.inc
@ adserve.inc I changed line 96 to:
$variables->tids = isset($values['t']) ? preg_replace('/[^0-9,.]/', '', $values['t']) : ''; // just added the '.' string as an approved value
@ adcache.inc I edited line 221 to:
if (ereg('.', $id)) {
$tids = explode('.', $id);

$result = db_query("SELECT a.aid FROM ads a INNER JOIN term_node n ON a.aid = n.nid INNER JOIN term_node z ON a.aid = z.nid
WHERE a.adstatus = 'active' AND n.tid = %d AND z.tid = %d", $tids);
} else {
$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(%d)", $id);
}

Those changes allow me to use the ad function like this: ad('21.11', 1) which returned for display only banners which are located in an ad group with id of 21 and also in one of my ordinary site categories with id of 11.

Now I know that my hacks are "ugly" and I'm not even sure if what I did doesn't damage any other functionality (I guess it is), how can I make a decent patch out of this?

Cheers!

Bumping it up again with little fixes... would love to know how this feature could be implemented

I confess, it's still not clear to me what exactly you are trying to do. Please provide a very simple explanation. Try, for example, to explain what you're aiming for using two example adds "Foo" and "Bar". What groups are each in? How then are you trying to decide which pages they display on?

(Have you tried ad_channels -- they allow you to control which page an advertisement displays on based on path, though it's not clear to me whether or not this is what you're trying to do.)

I rechecked ad_channels to be sure and it give a solution to my issue which is not practical to a site with lots of categories (taxonomies).

I'll try to explain my issue better now (sorry but as u can plainly see I'm not a native English speaker :)).

My site got various categories (taxonomy values) for various content sections (display articles, service providers of the field, etc..), for example:
* Sport
* Health
* Life

Each page at the site got 3 constant areas for displaying image banners, each area display banners of different size then the rest of the areas, for example:
* Top Area - 234px X 90px
* Right Area - 210px X 60px
* Bottom Area - 150px X 60px

For each image banners area I created a group ad (total 3 group ads for the entire site).

And now to my issue... :)

I want to be able to assign each banner that I create to one of the groups ad & also assign it to one or more of my site categories (taxonomy values).
So for example when a user will surf at the "Health" category he will see only banners that are associated to that specific category.

The embedding of the banner should consist of both the ad group id and the taxonomy id (tid), for example: print ad(5, 1, array('tids' => '76')); will display banners which r associated to group id 5 and also to tid 76.

I hope that I was clearer now :)

Okay, I see what you're trying to do now.

The way I would implement this is in ad_channels. I would add a new "Taxonomy rules" section on the channel configuration page which would allow you to select one or more terms. You could then chose to only display advertisements in this channel to nodes that are in the selected terms. I think this would be a useful feature, and will leave the issue in an active status. Patches welcome!

Hm, the challenging part of this will be figuring out what taxonomy a given node is assigned to. We either have to cache this for all nodes (which could take a lot of memory on a large website), or we have to query the database each time we serve an advertisement (which would be very slow). I don't know an ideal way to solve this at the moment... unless you change the paths of your content for each taxonomy type, then you can use the existing code to match based on path.

Due to the potential memory and/or performance issues that this would cause, it's unlikely that I'll implement this.

How about just creating an embedding option only without setting elaborate admin features?

For example: print ad(null, 1, array('tids' => '5, 76')); will run the current query which is located at adcache.inc line 221 but with a slight modification so the result for the above command will result in an sql query like the following:
"SELECT DISTINCT a.aid FROM {ads} a INNER JOIN {term_node} n ON a.aid = n.nid WHERE a.adstatus = 'active' AND n.tid IN(5, 76)" (I just added the DISTINCT and made the IN get multiple values).

Will that be a problem to the module?

Any feature affecting ad serving needs to also be implemented in the caching layer -- this means, the functionality needs to be implemented in a way that does not require the database layer. If your site gets much traffic, the module simply won't perform well enough without a cache.

Moving issue into a pending state, which means it's waiting on a comprehensive patch or funding.

I'm also highly interested in being able to display ads by taxonomy terms.

My guess is that "node-to-term" mappings could be cached on cron runs - this seems to be a solution to performance concerns. It could even be cached as an include'able PHP file with a single $array - this would allow server-side PHP caching software (like xcache) to keep that mapping in-RAM between requests. With chmod 600, caching a single variable to a PHP file shouldn't be too much of a security issue.

It also appears that http://drupal.org/node/419196 is related: if a channel is enhanced with "PHP field returns true" filter, then it would be relatively easy to put into that field term_id-checking code, and thus implement "ads by taxonomy terms" using existing 'ad channels' infrastructure.

I'll post back as soon as I'm done following your discussion in the actual module code. In the meantime, any further hints and suggestions would be highly appreciated, as I'm not really familiar with Drupal module development. Also, it would be beneficial to understand Jeremy's vision of the best approach to implement "ads by taxonomy".

This one http://drupal.org/node/581450 is also related.

I decided to implement the "terms selection" in channels admin, and not a "PHP field returns true" - largely for ease of use considerations (as Ad module is often used by end-users).

At the moment, I only managed to make the term selects display in a proper fieldset on channel admin page.

The following steps yet have to be performed (any help and hints would be appreciated):
- "Settings" page of the Ad module need a new fieldset "Allowed vocabularies for channels". It should list all the site's vocabularies, and allow checking which of the vocabularies will be used for term selection on the channel admin page. Reason for this fieldset: taxonomy is often used for multiple purposes within a single site, and it is not always necessary to show "ads by taxonomy" on all vocabularies (some of the vocabularies are better hidden than visible). As a more specific example: "ad groups" are implemented as taxonomy, and are already present on the channel admin page, so it's better to hide that vocabulary. Also, consider a website with forum...
- the fieldset I added needs actual code to save the terms selection
- ad_channel.inc needs code to filter display by taxonomy terms
- adache.inc needs code to work around the issues mentioned by Jeremy

Does anybody know if (and how) it is possible to add this functionality by an external module (or a sub-module)? A documentation link would be welcome.

I'm done with the admin part - added a new fieldset "Allowed vocabularies for channels" to Channels->Settings, and a new fieldset "Taxonomy rules" to Channel edit form. This part is fully functional now.

Getting to the actual filtering now. Will first implement non-cached filtering.

I would still appreciate information on adding my changes as a sub-module to Ad module - if my final solution won't be accepted by Jeremy into the ad_channel module. I've looked through a number of infos, but still do not quite understand how do I modify a form in ad_channel module from a new submodule:
http://groups.drupal.org/node/21725
http://drupal.org/node/508
http://drupal.org/node/206753
http://drupal.org/node/22573

Also, it's not clear if 3rd-level submodules are possible (e.g. ad_channel_taxonomy).

patch

it incorporates my fix for http://drupal.org/node/606244

Well, I did it. A kind of.

Let me count the number (and type) of MySQL queries added per single call of adserve.php:

1. one call with JOINs, using taxonomy_node_get_terms() with a fake node object (to avoid loading the node)
2. one simple call (using variable_get()) to retrieve vocabularies allowed for taxonomy filtering
3. now the worst part: foreach(ad), foreach(channel) - one call to variable_get(), until some channel matches or there are no more channels

Provided MySQL has a large enough queries_cache configured, I believe all these extra queries will be very fast (on the order of 0.0005 sec on a common quadcore - tested in the non-related scenario with a query returning 66k items, 5-30KiB each).

I didn't really understand how caching is supposed to work here; all the changes were done to ad_channel.inc - which seems to have no caching for URL-based filtering as well. Or is all the caching hidden under $channels = adserve_cache('get_cache', 'channel');? If yes, then I believe #3 from the list of extra queries above could be incorporated into adcache.inc. If yes, #2 could also be cached.

Finally, the firstmost query could be cached to a PHP file, as I suggested earlier. It would be wise to regenerate that cache only on node updates (I believe "Drupal actions" could help here, but I do not know for sure). Of course, that will be a specific caching - I'm not going to dump full node->taxonomy objects.

Any comments/suggestions are welcome.

Jeremy, could you please comment on your willingness to accept this functionality into the Ad module?

Thanks for the patch. Some feedback after a quick read through:

1. Why are you storing this information in a variable instead of in the ad_channel table? I don't see any advantage to this, and believe this data should be moved into the ad_channel table.
2. Your fake node object will mean that node revisions can't be used. This needs to be fixed. (If you're concerned about node_load performance on your website, try something like this patch: http://tag1consulting.com/patches/load_cache)
3. I only see a patch for the admin side of things. Did you forget to attach the patch for the rest of the code?
4. The taxonomy data will need to be cached in ad_channel_ad_build_cache() in ad_channel.module. This cache data then must be used in ad_channel_cache_filter() in ad_channel.inc. You can not make ANY db_query calls in ad_channel.inc, as Drupal will not be bootstrapped if you are using the ad file cache or the ad memcache cache.
5. Your patch, #606244: Invalid argument supplied for foreach() in includes/form.inc on line 1205, was already committed, it should not be part of this patch.

I will take another look when the above are fixed. You're very much headed in the right direction, thanks for the contribution!

1. That was done solely for the speed of development. I agree this should go into ad_channel table; will do that as time permits
2. Yes, I understand the revisions issue. Thank you for the link (wow, that's a big list of patches you have there!), but I'm trying to not modify Drupal's core at all. I'll try to implement in-module caching (that just wasn't a priority, yet).
3. Sorry, not yet used to contributing - patch attached.
4. Thanks for details on this one, I'll look into that.
5. Ok (I haven't used git's cherry-picking yet, guess its time to learn it)

> That was done solely for the speed of development. I agree this should go into
> ad_channel table; will do that as time permits

It will be necessary before this patch will be merged.

> Yes, I understand the revisions issue. Thank you for the link (wow, that's a big list of patches
> you have there!), but I'm trying to not modify Drupal's core at all. I'll try to implement
> in-module caching (that just wasn't a priority, yet).

I would not worry about caching node_load calls -- that's outside the scope of what you're trying to do here. Besides, it's already solved when you enable the ad file cache or memcache-based cache.

I've not yet had a chance to look at your newly attached code, but suspect it needs to be modified anyway to work with the ad caches based on previous comments.

Could you please provide a combined patch for the latest version? I'm willing to test and rewrite the patch because I need the same feature.

I will do that tomorrow morning (GMT+2 time), too tired now.

I assume you want the patch against the dev-branch? Do you know if it is good enough to be used on a production website? (I'm currently using stable version.)

Also, please keep in mind that although I haven't done anything since my latest "working as expected" post (the code is fully functional on a low-traffic production website since my last post), I will be willing to contribute more.

Here it is.

Note: first chunks (patching adcache.inc) are *not* related to the "ads by taxonomy" feature - they fix #581296: ad module generates block content even if no ads are to be displayed for the Raw display method.

Thx. I will try this one and report my results.

I'm very glad to have found this thread. I'm setting up a new site, and was very surprised that the ad module didn't already have the ability to target ads to content via the same taxonomy used by the content.

You guys rock!

The patch under #28 did not work for me.

But its a great start and I was able to get it working form me. I used the code from #28 and made a new patch.

I wanted to be able to:
- create a channel
- assign terms to this channel
- display ads only if the channels term matched the current page term
- premier ads should still work

Attached you will find my to patched.

Jonathan, thanks for contributing your progress!

I believe my patch stopped (or partially stopped) working after one of the updates.

Do you happen to have a diff of "my patch" and "your patch"? I can make one, but if you already have it - just mail to webmaster at bogdan.org.ua (thanks!).

And one more question: it appears that caching is still absent from the "ads_by_taxonomy" feature (my version issued at least 3 MySQL queries per ad displayed, not sure about your patch); do you have any stats on how well "ads_by_taxonomy" behaves with growing numbers of ad displays?

ayalon,

I had a closer look at your patches, and only have to disagree with the use of this code fragment:

+  if ($is_node >= 0) {
+  	
+		//its a node
+		$is_node = true;

(whereas I had if ($is_node !== FALSE))

Here's why: strpos() will return boolean FALSE when substring is not found, and '>= 0' will convert that FALSE to 0, and evaluate as TRUE; basically, if ($is_node >= 0) is always TRUE.

I agree that AND-logic, introduced by this fragment

         // Filter out unmatched terms.
          foreach($valid_ads[$chid] as $key => $value) {
            if ($value == $aid) {
              unset($valid_ads[$chid][$key])
            }

is more appropriate for ads_by_taxonomy functionality (unlike the previously implemented OR-logic). However, if each $aid occurs only once in $valid_ads[$chid], then I'd rewrite this as unset($valid_ads[$chid][array_search($aid, $valid_ads[$chid])]);.

While working on #680020: Channel URL Rules don't work, found a serious problem with ads_by_taxonomy: with ad_file_cache enabled, serve.php does not always do full Drupal bootstrap, which makes Drupal API functions not available:

Fatal error: Call to undefined function taxonomy_node_get_terms() in sites/all/modules/ad/channel/ad_channel.inc on line 36

Line 36 is just $terms = taxonomy_node_get_terms($node);

This is exactly what Jeremy meant in paragraph 4 of his comment #23:

The taxonomy data will need to be cached in ad_channel_ad_build_cache() in ad_channel.module. This cache data then must be used in ad_channel_cache_filter() in ad_channel.inc. You can not make ANY db_query calls in ad_channel.inc, as Drupal will not be bootstrapped if you are using the ad file cache or the ad memcache cache.

Jeremy, does 'Cache lifetime' also determine the frequency of regeneration of data cached by ad_channel_ad_build_cache()?

Also: should one add an extra column to ad_channel table (storing $allowed_terms(vid => array(0 => tid1, 1 => tid2)), or should that be a separate ad_channel_taxonomy table?

I have just finished implementing 'cached' version of ads_by_taxonomy, which no longer has the error from #34 above.

New version is quite memory-efficient. For a site with ~1700 nodes, 2 vocabularies allowed for ads_by_taxonomy, and well over a 100 of terms in those vocabularies, cache files only grow to ~80Kb (all the nodes-to-terms relations are cached), and while serving ads memory use doesn't get above 3 Mb. However, generating cached nodes-to-terms relations does take several seconds on a 3GHz CPU (hence the question at the top).

Out of initial 3, now only 1 MySQL query remains uncached: $channel_terms = variable_get("ad_channel_allowed_terms_$chid", 0);.

There seems to be only one remaining problem before Jeremy is likely to review and accept this functionality into the ad_channel module:

# Why are you storing this information in a variable instead of in the ad_channel table? I don't see any advantage to this, and believe this data should be moved into the ad_channel table.

I would appreciate help in 'moving ad_channel_allowed_terms_$chid into the ad_channel table'. (Moving ad_channel_allowed_terms_$chid into the ad_channel table must be followed by getting rid of the single MySQL query remaining, as mentioned above.)

There was one more problem - 'Your fake node object will mean that node revisions can't be used. This needs to be fixed.' - but I'm not sure if that still applies.

HI

I am in need of same functionality. Thanks for the patch.

and can you make use of forum taxonomy paths too? I mean forum/1, forum/2.. forum/{term-id} etc.. so that ads can be restricted in forum paths too.

also taxonomy/term/tid ?

Forum taxonomy might work, but I've never tested that.

ahh
forum NODE page might work, but what about forum listing / topic listing pages?
forgive me if it is added in that patch.

The patch only works for nodes which were assigned certain taxonomy terms, and will not (currently) work for topic listing pages - those can be added via URL rules, for example, or with a "PHP filter" box (there's a patch for that somewhere in the beginning of this thread).

Hii.. I’m new in drupal so I will like to know if the option of displaying embedded ads based on the taxonomy terms of the node is available ...

Thanks

Hey Tetramentis --
So I applied your patch to ad_channel.module and ad_channel.inc, and I'm able to select a vocabulary to use as a filter for a given channel. However, this doesn't seem to be having an effect (i.e., I'm looking at nodes on my site which are tagged with the taxonomy term which I selected for this channel, and yet the channel's block doesn't appear). Can you help me understand why that might be? Also, I'm not clear logically about how the URL filter should work in conjunction with the taxonomy filter. It is supposed to be logical OR (show this channel's block if the URL filter returns true OR if this is a node tagged with one of the channel's terms) or AND?

FWIW I'm hoping OR :)

I've been sifting through the patched module's code to figure this out, and I have run across your comment:

      // Skip taxonomy filter if $match is true, or if not on a node page (taxonomy makes sense only for nodes).
      // To implement OR-logic, add this code: (is_array($valid_ads) && is_array($valid_ads[$chid]) && in_array($aid, $valid_ads[$chid]))
      if ($match || !$is_node) {
        continue;
      }

Applying this or any other change (even a simple die()) to this function seems to have no effect, though. I'm assuming this is because of ad module's caching.

Anyway, thank you very much for the patch, and all the work that has clearly gone into it.

Judging by comment 33, AND-logic is used.

From what I can see in my git repository, there's also a code fragment which says

          // Filter out unmatched terms; remove/bypass to implement OR-logic.                                    
          unset($valid_ads[$chid][array_search($aid, $valid_ads[$chid])]);

somewhere below what you cited.

Unfortunately, I do not quite remember how the fragment you cited is supposed to work, and it may even have an error in it (need to have a closer look at $match to be sure). But 2nd fragment (that single 'unset' line) is clear: if taxonomy filter finds no rule match, node is removed from further comparisons with other filters; commenting that out will at least help achieving OR-logic.

Try enabling debug mode. I have added quite a number of debug messages throughout the code, and that should help spotting what isn't working. Code is quite heavily commented, but I'd be willing to explain if something is obscure.

I believe my patches did work back in February; however, I haven't checked the code since then, as the project I was doing this for is currently frozen. I may need some incentive (or help) to review/fix and maybe improve it enough for Jeremy to include taxonomy filtering into the ad_channel module.

OK, got it working, I think. You were right -- that second chunk was where I needed to start looking. There were a couple of issues elsewhere, though, which prevented the thing from working. The main one was the use of variable_get here in ad_channel_cache_filter() (which is in ad_channel.inc), where Drupal hasn't been bootstrapped:

       reset($channels['ads'][$aid]);
       foreach ($channels['ads'][$aid] as $chid) {
         $channel = $channels['channels'][$chid];
         $channel_terms = variable_get("ad_channel_allowed_terms_$chid", 0);

I changed ad_channel.module so that it sticks that variable into the cache, which lets you use it in this context.

If anybody else wants to apply this patch and test it out, I'd appreciate it -- love to have a couple more eyes on this, as I'm just getting the hang of this module and there's a lot to it. Thanks for the tip about debug mode, by the way. I hadn't known about it before, and it is an enormous help in figuring this sucker out!

I read every reply here, and am a little confused still because of all the attachments of patches and diff files.

I have uploaded an image of what I'd like to match. Say while creating a new node I add a Tag to it called "Foo".

Next I add a new ad I'm creating, now regardless of the group it is in, I only want it to show on pages where the Tags section matches the Tags section of the node being displayed.

So I go ahead and tag my new ad with Foo, so in theory it would only show up on nodes tagged with Foo also, if one of the patches attached to this thread will accomplish that, please let me know which one, and if it needs any further editing.

Thank you!

i applied the patch from #43 to july 10 ad on drupal 6.19 and received:

Parse error: syntax error, unexpected T_IF, expecting ')' in /sites/all/modules/contrib/ad/channel/ad_channel.module on line 90

@Branndon: +1, Looking for *exactly* this functionality and also trying to determine which combination of the listed patches get this done.

subscribe

subscribe

Sorry if this is a silly question, but what about the Context module? Has anyone tried it with ad? I haven't tried it yet, but wonder if it could work.

Context module with ad (6.x-2.x-dev):
It works for us: One channel - Everywhere, separate ad group (drupal block) for each ad, we use the same name for both, group and ad. Then Context defines if that block/ad will be visible.
You may also group few ads under one block, so they will be displayed based on their Probability.
Too bad Channels aren't working with Context (at least not in our OpenPublish 2.2 Drupal distro) - all ads are displayed in the block no matter what conditions are defined for that channel.

UPDATE: It's actually not so bad that Channels aren't working. If you choose the Context module over the Ad's Channel then you will have all the power of it to define the context rules, including:
- taxonomy (not just from the one keywords vocabulary, but from all other vocabularies as well);
- user roles;
- language
and other conditions.
The only one thing you will miss from the Ad's Channel - it's inside-block conditional ad display as opposed to outside-block conditional ad display for the Context module that results in more then one ad block shown on the page when more then one ad-context's conditions are met.

Hi guys,
Please read "UPDATE:" part in the #50 post for what I consider a very good solution to the issue.

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