Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow modifications on the adsense code #336

Closed
FuSan21 opened this issue Jul 26, 2019 · 10 comments
Closed

Allow modifications on the adsense code #336

FuSan21 opened this issue Jul 26, 2019 · 10 comments
Labels
Module: AdSense Google AdSense module related issues P1 Medium priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Milestone

Comments

@FuSan21
Copy link

FuSan21 commented Jul 26, 2019

Currently there isn't any way to make modifications (ex. make anchor tags always come from bottom) to the adsense code. Make it easier for new/novice users to do this kind of modifications.

Feature Description

Within AdSense Auto Ads there are different ad formats (Display ads, in-fedd ads, matched etc) that users can enable or disable globally across their sites.

Some users would like additional options from within Site Kit (specifically the AdSense module) to toggle such ad formats.

While the first step would naturally be allowing users to select specific ad formats some users would benefit from follow on features. The ability to disable anchor ads at the top of a page is an example. At the moment this feature must be implemented via adding parameters to Auto Ads code. With Site Kit placing the Auto Ads code this is not easily achievable for users.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The options which are passed to the AdSense snippet that Site Kit places should be filterable.
  • For non-AMP:
    • The associative array that is currently JSON-encoded should first be passed through a new googlesitekit_auto_ads_opt filter.
    • It should be ensured though that the google_ad_client key is never overwritten (e.g. by re-setting it afterwards, similarly to how it is done in the AMP tag for Analytics.
  • For AMP (amp-auto-ads):
    • There should be a filterable array which contains only a data-ad-client key with the tag ID as value. It should be passed through a new googlesitekit_amp_auto_ads_attributes filter.
    • It should be ensured that the data-ad-client key is never overwritten.
    • Then, every key in the array should be added as an attribute to the amp-auto-ads tag (so that by default the tag looks the same as today, with a data-ad-client attribute).
  • For Web Stories (amp-story-auto-ads):
    • There should be a filterable array which contains data-ad-client and data-ad-slot keys with the respective values as currently used. It should be passed through a new googlesitekit_amp_story_auto_ads_attributes filter.
    • It should be ensured that the data-ad-client key is never overwritten.
    • Then, the array should be merged into the ad-attributes array currently used, appending to the type key (which should not be hard-coded because it should always be adsense, basically like a constant, as it is also enforced in the other 2 snippets).

Note that the filter name suffix _opt was chosen for parity with the similar Analytics filters. For AMP however, what's being filtered is not direct options passed as AdSense configuration, but rather attributes - that's why these two filters use the _attributes suffix.

Implementation Brief

  • Using includes/Modules/AdSense/Web_Tag.php, within the render method,
    • Create a new variable which has the array within the wp_json_encode function as value.
    • Pass the above variable through the googlesitekit_auto_ads_opt filter.
    • If the filtered array is empty or is not an array, set the filtered array to be the non filtered array.
    • To make sure the google_ad_client key is not overwritten in the filtered array, set the value again in the array, i.e $this->tag_id.
    • Use the filtered array within the wp_json_encode function.
  • Using includes/Modules/AdSense/AMP_Tag.php, within the render method,
    • Create a new variable which is an array containing an item with data-ad-client as key and the tag ID as value.
    • Pass the above variable through the googlesitekit_amp_auto_ads_attributes filter.
    • If the filtered array is empty or is not an array, set the filtered array to be the non filtered array.
    • To make sure the data-ad-client key is not overwritten in the filtered array, set the value again in the array, i.e $this->tag_id.
    • Update the rendered markup to display all key values from the filtered array.
  • Using includes/Modules/AdSense/AMP_Tag.php, within the render_story_auto_ads method,
    • Create a new variable which is an array which contains the data-ad-client and data-ad-slot with their appopriate values, i.e $this->tag_id and $this->story_ad_slot_id respectively.
    • Pass the above variable through the googlesitekit_amp_story_auto_ads_attributes filter.
    • If the filtered array is empty or is not an array, set the filtered array to be the non filtered array.
    • To make sure the data-ad-client key is not overwritten in the filtered array, set the value again in the array, i.e $this->tag_id.
    • Merge the filtered array with the ad-attributes array.

Test Coverage

  • No new tests to be added.

Visual Regression Changes

  • N/A

QA Brief

  • Verify if the conditions in the ACs are met.
  • Or, connect AdSense module and edit the theme's functions.php to run the filters in the AC.
    • For e.g
     add_filter( 'googlesitekit_auto_ads_opt', function () {
     	return array( 'tag_partner' => 'my-custom-tag-partner' );
     } );
  • Verify markup on the front end contains the appropriate filtered values.

Changelog entry

  • Add filters to allow modifications on the AdSense code.
@jamesozzie jamesozzie self-assigned this Jul 29, 2019
@jamesozzie jamesozzie added the Type: Support Support request label Jul 29, 2019
@jamesozzie
Copy link
Collaborator

@fu-san Thanks for the suggestion. Site Kit uses Auto Ads (Auto Ads for AMP). The recommended placement for Auto Ads code is within the tags on a users website. As intended this is where Site Kit places the code.

If you have other suggestions feel free to also add them individually, with some screenshots or context and we can certainly investigate.

@FuSan21
Copy link
Author

FuSan21 commented Jul 29, 2019

@jamesozzie
Yes, I agree sitekit places the adsense code on the correct place. But the problem is that there isn't any possibility to do any modification here.

For example if someone wanted to stop anchor ads from autoads to coming from top, according to to help center he has to do some modification. (https://support.google.com/adsense/answer/7478225?hl=en)

That kind of modifications isn't possible at the moment with sitekit. So I thought it would be great if one was able to modify the auto ad code for their purpose.

@jamesozzie jamesozzie added Type: Enhancement Improvement of an existing feature and removed Type: Support Support request labels Aug 2, 2019
@jamesozzie jamesozzie removed their assignment Aug 2, 2019
@jamesozzie
Copy link
Collaborator

@fu-san I will label this as an enhancement, and modify the content for that specific feature.

@U-se
Copy link

U-se commented Nov 10, 2020

Has there been any update on this? I also would need this exact feature. Either manually editing of the AdSense code or the possibility that Sitekit automatically adds the correct code of common modifications to the AdSense code (like forcing anchor ads to the bottom).

@jamesozzie
Copy link
Collaborator

@U-se We've no update on this just yet. In the meantime if you're looking to modify the code snippet to include such modifications the plugin can recognize existing AdSense Auto ads code placed in your site when connecting the module.

@marrrmarrr marrrmarrr added the Team Review Issue needs to be reviewed by team for triaging label Mar 30, 2021
@felixarntz felixarntz self-assigned this Apr 7, 2021
@felixarntz felixarntz added Module: AdSense Google AdSense module related issues P1 Medium priority and removed Team Review Issue needs to be reviewed by team for triaging labels Apr 7, 2021
@felixarntz
Copy link
Member

Given that the AdSense API doesn't allow write interactions, unfortunately we cannot manage these settings from within Site Kit. However, being able to modify the AdSense snippet is definitely a great idea. We're going to prioritize this one and introduce a filter which can be used to modify the snippet parameters.

@felixarntz felixarntz removed their assignment Apr 19, 2021
@asvinb asvinb assigned asvinb and unassigned asvinb Jul 7, 2021
@eugene-manuilov eugene-manuilov self-assigned this Jul 12, 2021
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Jul 12, 2021
@fhollis fhollis added this to the Sprint 54 milestone Jul 14, 2021
@asvinb asvinb self-assigned this Jul 19, 2021
@asvinb asvinb removed their assignment Jul 20, 2021
@asvinb asvinb added the QA: Eng Requires specialized QA by an engineer label Jul 20, 2021
@asvinb asvinb assigned eugene-manuilov and unassigned asvinb Jul 21, 2021
@asvinb asvinb assigned eugene-manuilov and unassigned asvinb Jul 22, 2021
@asvinb asvinb assigned eugene-manuilov and unassigned asvinb Jul 23, 2021
@asvinb asvinb assigned eugene-manuilov and unassigned asvinb Jul 27, 2021
@eugene-manuilov eugene-manuilov removed their assignment Jul 27, 2021
@eclarke1 eclarke1 added the Rollover Issues which role over to the next sprint label Aug 2, 2021
@eclarke1 eclarke1 modified the milestones: Sprint 54, Sprint 55 Aug 2, 2021
@ivankruchkoff ivankruchkoff self-assigned this Aug 16, 2021
@fhollis fhollis added Rollover Issues which role over to the next sprint and removed Rollover Issues which role over to the next sprint labels Aug 16, 2021
@fhollis fhollis modified the milestones: Sprint 55, Sprint 56 Aug 16, 2021
@ivankruchkoff
Copy link
Contributor

ivankruchkoff commented Aug 16, 2021

QA

✅ On homepage without amp
Filter: googlesitekit_auto_ads_opt
array(3) { ["google_ad_client"]=> string(23) "ca-pub-1111111111111111" ["enable_page_level_ads"]=> bool(true) ["tag_partner"]=> string(8) "site_kit" }

✅ On homepage with amp
Filter: googlesitekit_amp_auto_ads_attributes
array(1) { ["ad-client"]=> string(23) "ca-pub-1111111111111111" }

✅ On webstory, with / without amp
Filter: googlesitekit_amp_story_auto_ads_attributes
array(2) { ["ad-client"]=> string(23) "ca-pub-1111111111111111" ["ad-slot"]=> string(10) "1111111111" }
Expected filter: googlesitekit_amp_story_auto_ads_attributes
However it wasn't called. Is there something that needs to be enabled to trigger sitekit filters with webstory adsense settings?

In /wp-admin/edit.php?post_type=web-story&page=stories-dashboard#/editor-settings
There are separate settings for adsense, and when I set monetization to none, no amp-story-auto-ads appears in the page source.

Tried with and without these settings:

Screen Shot 2021-08-16 at 1 41 11 PM

For reference, used this mu-plugin to test:

<?php
add_filter( 'googlesitekit_auto_ads_opt', function ($foo) {
    var_dump( 'googlesitekit_auto_ads_opt');
    var_dump( $foo );
 } );
add_filter( 'googlesitekit_amp_auto_ads_attributes', function ($foo) {
    var_dump( 'googlesitekit_amp_auto_ads_attributes');
    var_dump( $foo );
 } );
add_filter( 'googlesitekit_amp_story_auto_ads_attributes', function ($foo) {
    var_dump( 'googlesitekit_amp_story_auto_ads_attributes');
    var_dump( $foo );
 } );

@asvinb
Copy link
Collaborator

asvinb commented Aug 16, 2021

@ivankruchkoff So, turns out that if we don't have a story specific ad unit, the snippet will not be displayed:
https://github.com/google/site-kit-wp/blob/develop/includes/Modules/AdSense/Tag_Guard.php#L41

Can you add add an unit id in AdSense settings and test again?
image

Thanks!

@ivankruchkoff
Copy link
Contributor

Thanks for the update, that works @asvinb .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: AdSense Google AdSense module related issues P1 Medium priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

10 participants