Posted by flacoman91 on December 13, 2012 at 10:06pm
5 followers
| Project: | Doubleclick for Publishers (DFP) |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Hi,
My client tasked me to create something to override the ad unit slots in the context settings page.
The client wanted to be able to define the ad unit to whatever arbitrary value when the url or taxonomies do not match in the context reactions.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| context_adunits.patch | 2.74 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 385 pass(es). | View details |
Comments
#1
sorry, disregard the above patch. it was working on the rc version, but for the dev version it's not working.
#2
Okay again, here's a complete patch. for the context addition.
I left some things out accidentally the first time (I'm somewhat of a noob).
#3
I like this idea a lot ... so much so that i improved it a bit. The attached patch (based on #2) adds the ability to choose which tags should be overridden by offering checkboxes and allowing for tokens in the overridden ad unit:
I wish we could have simpletests for this, but since context is not a dependency, alas...
#4
I think this is a great idea... Any chance you could have the option to override the ad unit size that is sent?
This might be a simple way of supporting advertising on reactive theming? It seems you could do the same by targetting custom ad units in this patch as it stands, but if you could just change the ad unit size, you wouldn't need to create ad units for different breakpoints/layouts....
#5
@gurrmag ... did you test the patch in #3 ? Is it RTBC?
As for adding similar functionality for ad size... there are some subtle differences with that (like you would never want to change two tags in one "context" because the new sizes would be different for each tag) such that I think that should be in its own issue. Please feel free to open one.
#6
Limited local testing... which worked as expected.
I need our advertising guys to set up new units in DFP, but they're not so quick when there's no sniff of commission!
I take the point about opening a separate issue - will do so as soon as I've posted this.
Link to issue: http://drupal.org/node/1884052
#7
I can confirm that this works...
For our implementation, we surfaced global tokens for targeting and ad unit patterns so that the standard URL tokens and tokens from the Custom Token module were available.
Not sure if you would be interested in a combined patch for this?
#8
sure .. please share your patch
#9
@bleen18, do you want me to run the patch against dev (7.x-1.x) or your 7.x-1.0-rc2?
See http://drupal.org/node/1895522 - One other issue we are having is that we need to surface node tokens in the adunit pattern and currently they are not being processed. Can you see any problem in doing this? We probably need to work on the login for checking for "pattern" in dfp_format_targeting() (thanks @gurrmag)
template_preprocess_dfp_tag()
<?php
function template_preprocess_dfp_tag(&$variables) {
$tag = $variables['tag'];
// Create the attributes for the wrapper div and placeholder div.
$variables['placeholder_attributes'] = array(
'id' => $tag->placeholder_id,
'class' => array(
'dfp-tag-wrapper',
),
);
//Patched to move adunit pattern processing here so more tokens are available
//build array expected by recipient function
$pattern=array(array(
"target"=>"pattern",
"value"=>'[dfp_tag:network_id]/' . $tag->adunit
)
);
$pattern = dfp_format_targeting($pattern, $tag);
// Format certain tag properties for display.
$tag->adunit = $pattern[0]['value'];
$tag->size = dfp_format_size($tag->size);
$tag->slug = dfp_format_slug($tag->slug);
// Define a javascript ad slot for this tag.
_dfp_js_slot_definition($tag);
}
?>
dfp_format_targeting()
<?php
function dfp_format_targeting($targeting, $tag = '') {
if (!empty($targeting)) {
// Prepare information needed for token replacements.
global $user;
$data = array('user' => $user);
if (!empty($tag)) {
$data['tag'] = $tag;
}
if (arg(0) == 'node' && is_numeric(arg(1))) {
$data['node'] = node_load(arg(1));
}
if (arg(0) == 'taxonomy' && is_numeric(arg(2))) {
$data['term'] = taxonomy_term_load(arg(2));
}
}
foreach ($targeting as $key => &$target) {
$target['target'] = '"' . check_plain($target['target']) . '"';
$target['value'] = token_replace(check_plain($target['value']), $data);
// The target value could be blank if tokens are used. If so, removed it.
if (empty($target['value'])) {
unset($targeting[$key]);
break;
}
// Convert the values into an array and trim the whitespace from each value.
//Patched to allow adunit pattern to be processed through this function
// Adunit pattern cannot go through this – double double quotes added
if ($target['target'] != '"pattern"') {
$values = explode(',', $target['value']);
$values = array_map('trim', $values);
if (count($values) == 1) {
$target['value'] = '"' . $values[0] . '"';
}
elseif (count($values) > 1) {
$target['value'] = '["' . implode('","', $values) . '"]';
}
}
}
return $targeting;
}
?>
#10
Always role patches against -dev. Thats true for all Drupal projects...
I'm confused here ... node based tokens are not working for adunit?? That doesnt sound right. Can you give me an example of exactly what value you are using for adunit, what you expect it SHOULD evaluate to and then what it actually evaluates to. I'd like to try and reproduce... BUT, please do this by opening a new issue. I dont want to confuse the current issue any more then we already have... :)
This sentence I simply dont understand... "work on the login"?
As for your code changes, when you open that new issue, feel free to include them as a patch. I cant tell you for sure if Id accept it because I dont yet grok the problem, but assuming there is an issue and this is a possible fix, I dont see anything crazy in there just yet.
#11
Based on #7 I committed the patch from #3... thanks @flacoman91
@gurmag, feel free to role a new patch for your idea about global tokens in a new issue
@soulston, feel free to role a patch (and give more info) about your comment in #9 in new issue
#12
Thanks for the feedback @bleen18,
I re-read my post and confused myself - login should be logic!
Patch on it's way
#13
Awesome. thanks a bunch !
#14
Patched
* Brought global tokens into scope
* Fixed a few typos - Implements of hook_... -> Implements hook_...
* Fixed typo $tagets[] = check_plain($data['target']) . '=' . check_... -> $targets[] = check_plain($data['target']) . '=' . check_...
I also updated my repo to include the patch from http://drupal.org/node/1895522.
#15
#16
soulston ... would you mind creating a new issue "Allow global tokens" or something. I just dont want to confuse this issue.
Thanks!
#17
No probs - http://drupal.org/node/1896188
#18
Automatically closed -- issue fixed for 2 weeks with no activity.