add 'inurl' parameter to pages/posts; refine 'category' parameter

[dbeniaminov]

Added 'inurl' parameter to single pages and posts (line 231), improved 'category' parameter logic (line 254)

[dbeniaminov]

1. 'is_singular() is more encompassing, includes more content types
2. using post-name instead of full URL is more efficient. DFP (or other ad services) have a character limit on URL targeting (not more than 40 chars), so for long URLs, or long domains, targeting becomes difficult. Only including post_name simplifies the process, and reduces errors.
3. many wordpress themes default a specific page to be a homepage. This condition excludes home-page and front-page targeting, to refine the results.

Hope this helps.

Dmitry Beniaminov

Google: dmitry.beniaminov@gmail.com Twitter: @beniaminov Web: http://www.pixelstudioz.com Phone: 416-669-3457

On Tue, Jul 10, 2018 at 12:55 PM Ben Keith notifications@github.com wrote:

@benlk requested changes on this pull request.

In dfw-init.php https://github.com/INN/doubleclick-for-wp/pull/62#discussion_r201418247:

@@ -228,8 +228,11 @@ private function targeting() { }

  // Templates

• if ( is_single() ) {
• // change from is-single() to is_singular() to target pages; only define 'inurl' parameter on non-category, and non-home pages

remove this comment

In dfw-init.php https://github.com/INN/doubleclick-for-wp/pull/62#discussion_r201418264:

        $targeting['Page'][] = 'single';

• global $post;
• $targeting['inURL'][] = $post->post_name;

Why does this need to be defined to be the post name? Can't inURL targeting be done with the actual URL?

In dfw-init.php https://github.com/INN/doubleclick-for-wp/pull/62#discussion_r201418471:

@@ -248,7 +251,8 @@ private function targeting() { $targeting['Page'][] = 'search'; }

• if ( is_single() ) {
• // change from is-single() to !(is-home() or is-front-page()

What's the reasoning behind this change?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/INN/doubleclick-for-wp/pull/62#pullrequestreview-135917306, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjU_5irSt8YVGMLagFXiXwTCvL0baNGks5uFNx8gaJpZM4SXGy3 .

[benlk]

I'm closing this pull request because its changes are no longer necessary.

The is_single/is_singular changes have been made in https://github.com/INN/doubleclick-for-wp/pull/72/files and https://github.com/INN/doubleclick-for-wp/pull/74 using code that @dbeniaminov provided in issue https://github.com/INN/doubleclick-for-wp/issues/61. I hadn't realized that this PR was connected with issue https://github.com/INN/doubleclick-for-wp/issues/61 before; their changes were different.

Adding inURL targeting is not necessary, in light of jQuery.dfp.js' built-in URL targeting options: https://github.com/coop182/jquery.dfp.js#default-url-targeting

This PR also contains several comments that should not remain in production code.