Added targeting to DFP async provider #81

Closed
wants to merge 10 commits into from

2 participants

@harvitronix

Modified the DFP async provider code to pass keywords, title, path and domain to the defineSlot method. This will allow users to target specific ads based on these attributes through DFP itself.

@rinatkhaziev

I don't think this will work.
What if I want to target by custom taxonomy?
What if I don't want to set targeting at all?
What if I want to do targeting by some weird parameters?

Here are some real life examples:

.setTargeting( "pos", "atf" );
.setTargeting( "right", "top" );
.setTargeting( "pixel", "1" );

Perhaps it's better to add a filter for targeting and make the value empty by default.

@harvitronix

Yes, I'd have to agree. Should instead be a filter.

@harvitronix

I've instead made target setting filterable (with a default of empty). Not sure that this is the correct place to put the apply_filters line, though.

@rinatkhaziev

@harvitronix How about something like this:
https://gist.github.com/rinatkhaziev/6063442

The idea is to make targeting parameters an array, and then apply filters to it, passing array for filtering and tag id for any additional logic. After that, format a string with a special method, making sure that passed parameters are validated and escaped.

@harvitronix

@rinatkhaziev Realized we may have an issue with esc_js on the value output. The value sent to the targeting key will often need to be a JS array. Escaping it will turn ['foo','bar'] into [\'foo\',\'bar\'] which will trip up the function, I believe.

@jeffstieler jeffstieler commented on the diff Apr 28, 2015
providers/doubleclick-for-publishers-async.php
@@ -204,6 +212,24 @@ public function action_wp_head() {
do_action( 'acm_tag', 'dfp_head' );
}
+ /**
+ * Format setTargeting string
+ * @param array $params_array Key value pair you'd like to target. Example: ["title"]=>"Your post's title"
+ * @return string The targeting string to append to slot definition
+ */
+ function format_targeting_string( $params_array = array() ) {
+ $ret = '';
+ var_dump($params_array);

@harvitronix You probably want to kill this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeffstieler jeffstieler added a commit to voceconnect/Ad-Code-Manager that referenced this pull request Apr 28, 2015
@jeffstieler jeffstieler Automattic#81 ac6e39d
@harvitronix

Closing this PR as it's stale and not structured well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment