Hello, Jeremy.

I'm using Ad module to display a variety of ads across my site's pages. Initially, I found idea with Ad groups great, but later I also found some lack of flexibility in controlling where Ad should be shown.

I created a module which allows to control Ad placement just like Drupal's block visibility settings with some improvements. You have three textareas, where you can:
- Show on which pages ad SHOULD be displayed.
- Exceptions to previos rule.
- PHP visibility rules (which can replace or be added to first two).

This set of rules provide complete control on Ad placement, so I think it should be included into main package or even merged to ad.module itself.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

neochief’s picture

FileSize
63.54 KB
neochief’s picture

FileSize
63.54 KB

Here's an example how it looks like on Ad editing form (initially it's collapsed):

Jeremy’s picture

Status: Needs review » Needs work

What do you need that is not provided by the ad_channels module? It allows the same sorts of rules, except instead of creating a rule for each advertisement, you create rules for channels and then assign advertisements to channels. If anything, I think the ad_channel module could be enhanced if it's missing functionality -- I don't see the need for a new module doing basically the same thing.

neochief’s picture

emm... ad_channels? It seems that I missed it's born :)

neochief’s picture

Okay, I was digging into ad_channel tonight. Except that I can't make it actually work, I have understood the idea and can say that it's nice.

Anyway, I'll fix some bugs I found in ad_channels. Also I'd like to add such features:
1. Visibility rules like in my module:
- URLs rules (in a way that can be seen at screenshot. It provides more flexibility). Also, I would like to make it understand usual drupal's 'path_alias' or 'node/10' , so as 'http://...' that are required now.
- PHP rules that ad_channel simply don't have (but I need to add separate permision for using this method).
2. Option to disable priorities and containers (In some cases we don't need B-52 to kill a squirell).
3. Join Ad Groups and Ad Channels in ad form into one fieldset and maybe make them an equal type.

P.S. By the way, step by step everything bacames too complex, we should think about documentation in near future, shouldn't we?

Concerning Ad Placement — at least I perceived Zen of adserve with caches :)

Jeremy’s picture

Title: New submodule: Ad Placement » improving ad_channels

Updating the title.

> Anyway, I'll fix some bugs I found in ad_channels.

Bug fixes are always welcome. Please file issues for them, too.

> Also I'd like to add such features:
> 1. Visibility rules like in my module:
> - URLs rules (in a way that can be seen at screenshot. It provides more flexibility). Also, I would like to make it understand usual drupal's 'path_alias' or 'node/10' , so as 'http://...' that are required now.

In the current implementation, 'node/10' or 'http://sample.com/node/10' are both supported. This is important, as advertisements can be hosted on remote websites.

I have concerns about your visibility rules, as they make things more complex to configure and understand. Let's take it in slow steps.

> - PHP rules that ad_channel simply don't have (but I need to add separate permision for using this method).

I'm not a big fan of PHP rules, personally, as they're an easy security hole. Weren't they removed from core for this reason? Or just PHP filters?

> 2. Option to disable priorities and containers (In some cases we don't need B-52 to kill a squirell).

Priorities should be moved into their own module, actually. They really have nothing to do with channels.

If you don't need containers, don't use them. They are optional, ad only provided to help admins stay organized.

> 3. Join Ad Groups and Ad Channels in ad form into one fieldset and maybe make them an equal type.

Why would you do this?

> P.S. By the way, step by step everything bacames too complex, we should think about documentation in near future, shouldn't we?

The ad module's documentation is out of date, correct. It would be a good idea to open an issue for this to be sure it happens before 6.x-2.0 is released.

neochief’s picture

> In the current implementation, 'node/10' or 'http://sample.com/node/10' are both supported. This is important, as advertisements can be hosted on remote websites.

Hm, as it not works for me in both cases right now, I can't say for 100%. I meant not only natural path, but also path aliases. I looked into ad_channel.inc and found only one regexp which I suppose checks full URL without chaking aliases. I cant say right now a good way to implement both of this, but I know that it's certainly needed. Let's say that we want to show ads on taxonomy pages, but thay have path aliases. In this case you'll check the 'http://sample.com/aliased_path/tag/*' but not 'http://sample.com/taxonomy/term/*', which would be needed. Again, I can be wrong, please correct me if I didn't dugg to deep.

> I'm not a big fan of PHP rules, personally, as they're an easy security hole. Weren't they removed from core for this reason? Or just PHP filters?

Yes, I agree that PHP filter is potential security hole. Because of this I mentioned about separate permission. But of course, we can do this like people did in Drupal6 — just move it into separate sub-module (yet another sub module :). As for PHP filter benefits — it really can save a lot of time in ad maintenance. There are no big deal to write simple "IF" for anyone in PHP filter, but there is an obstacle to non-Drupaler to make a module which makes the same like that "IF".

> Priorities should be moved into their own module, actually. They really have nothing to do with channels.

Perfect.

As for containers, maybe we should improve their usability instead? Hide them if they not needed, etc. I will think about this and tell you results later.

>> 3. Join Ad Groups and Ad Channels in ad form into one fieldset and maybe make them an equal type.
>Why would you do this?

As for me — now they both looks ugly. It's main reason. They presents relative substance, so why they can't be joined? And if they are relative, why they should look different (select vs. checkboxes). It's my thoughts from usability point of view.