-
Notifications
You must be signed in to change notification settings - Fork 295
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
Place Ad Blocking Recovery Tags on the website #7212
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
da6f96c
Place Ad Blocking Recovery tags.
kuasha420 b407dea
Remove failing test.
kuasha420 4157afa
Update `render` method.
kuasha420 bfdaf24
Add test for `WP_Query_404_Guard`.
kuasha420 e788bde
Add test for `Ad_Blocking_Recovery_Tag_Guard`.
kuasha420 d195382
Set values in the ABR tag.
kuasha420 8b3bdf8
Fix typo.
kuasha420 ccc3da3
Add test for `Ad_Blocking_Recovery_Web_Tag`.
kuasha420 75d4073
Merge branch 'develop' into enhance/7186-place-abr-tags.
kuasha420 c345e31
Remove test group.
kuasha420 ce2313b
Remove unused param.
kuasha420 8071bcd
Use setters for correct thing.
kuasha420 b098923
Fix phpdocs of `Ad_Blocking_Recovery_Tag_Guard`.
kuasha420 bcd195f
Update `Ad_Blocking_Recovery_Tag_GuardTest`.
kuasha420 826d6a4
Update phpdoc for `AdSense\Tag_Guard::can_activate` to be more accurate.
kuasha420 5ba46f6
Return early if tag is blocked.
kuasha420 0c72a7d
Refactor `Ad_Blocking_Recovery_Web_Tag` to be based on `Tag` class.
kuasha420 968965b
Update class description.
kuasha420 136eb77
Update WP_Query_404_Guard.
kuasha420 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php | ||
/** | ||
* Class Google\Site_Kit\Core\Tags\Guards\WP_Query_404_Guard | ||
* | ||
* @package Google\Site_Kit\Core\Tags\Guards | ||
* @copyright 2023 Google LLC | ||
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
* @link https://sitekit.withgoogle.com | ||
*/ | ||
|
||
namespace Google\Site_Kit\Core\Tags\Guards; | ||
|
||
use Google\Site_Kit\Core\Guards\Guard_Interface; | ||
|
||
/** | ||
* Class for WP_Query 404 guard. | ||
* | ||
* @since n.e.x.t | ||
* @access private | ||
* @ignore | ||
*/ | ||
class WP_Query_404_Guard implements Guard_Interface { | ||
/** | ||
* Determines whether the guarded tag can be activated or not. | ||
* | ||
* @since n.e.x.t | ||
* | ||
* @return bool TRUE if guarded tag can be activated, otherwise FALSE or an error. | ||
*/ | ||
public function can_activate() { | ||
return ! is_404(); | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
includes/Modules/AdSense/Ad_Blocking_Recovery_Tag_Guard.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<?php | ||
/** | ||
* Class Google\Site_Kit\Modules\AdSense\Ad_Blocking_Recovery_Tag_Guard | ||
* | ||
* @package Google\Site_Kit\Modules\AdSense | ||
* @copyright 2023 Google LLC | ||
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
* @link https://sitekit.withgoogle.com | ||
*/ | ||
|
||
namespace Google\Site_Kit\Modules\AdSense; | ||
|
||
use Google\Site_Kit\Core\Modules\Tags\Module_Tag_Guard; | ||
|
||
/** | ||
* Class for the AdSense Ad Blocking Recovery tag guard. | ||
* | ||
* @since n.e.x.t | ||
* @access private | ||
* @ignore | ||
*/ | ||
class Ad_Blocking_Recovery_Tag_Guard extends Module_Tag_Guard { | ||
|
||
/** | ||
* Determines whether the guarded tag can be activated or not. | ||
* | ||
* @since n.e.x.t | ||
* | ||
* @return bool TRUE if guarded tag can be activated, otherwise FALSE or an error. | ||
*/ | ||
public function can_activate() { | ||
$settings = $this->settings->get(); | ||
|
||
return ! empty( $settings['adBlockingRecoverySetupStatus'] ) && $settings['useAdBlockerDetectionSnippet']; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
<?php | ||
/** | ||
* Class Google\Site_Kit\Modules\AdSense\Ad_Blocking_Recovery_Web_Tag | ||
* | ||
* @package Google\Site_Kit\Modules\AdSense | ||
* @copyright 2023 Google LLC | ||
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
* @link https://sitekit.withgoogle.com | ||
*/ | ||
|
||
namespace Google\Site_Kit\Modules\AdSense; | ||
|
||
use Google\Site_Kit\Core\Tags\Tag; | ||
use Google\Site_Kit\Core\Util\Method_Proxy_Trait; | ||
use Google\Site_Kit\Core\Tags\Tag_With_DNS_Prefetch_Trait; | ||
|
||
/** | ||
* Class for Ad Blocking Recovery tag. | ||
* | ||
* @since n.e.x.t | ||
* @access private | ||
* @ignore | ||
*/ | ||
class Ad_Blocking_Recovery_Web_Tag extends Tag { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
use Method_Proxy_Trait; | ||
use Tag_With_DNS_Prefetch_Trait; | ||
|
||
/** | ||
* Ad_Blocking_Recovery_Tag instance. | ||
* | ||
* @since n.e.x.t | ||
* @var Ad_Blocking_Recovery_Tag | ||
*/ | ||
protected $ad_blocking_recovery_tag; | ||
|
||
/** | ||
* Use Error Protection Snippet. | ||
* | ||
* @since n.e.x.t | ||
* @var bool | ||
*/ | ||
protected $use_error_protection_snippet; | ||
|
||
/** | ||
* Constructor. | ||
* | ||
* @since n.e.x.t | ||
* | ||
* @param Ad_Blocking_Recovery_Tag $ad_blocking_recovery_tag Ad_Blocking_Recovery_Tag instance. | ||
* @param bool $use_error_protection_snippet Use Error Protection Snippet. | ||
*/ | ||
public function __construct( Ad_Blocking_Recovery_Tag $ad_blocking_recovery_tag, $use_error_protection_snippet ) { | ||
$this->ad_blocking_recovery_tag = $ad_blocking_recovery_tag; | ||
$this->use_error_protection_snippet = $use_error_protection_snippet; | ||
} | ||
|
||
/** | ||
* Registers tag hooks. | ||
* | ||
* @since n.e.x.t | ||
*/ | ||
public function register() { | ||
add_action( 'wp_head', $this->get_method_proxy_once( 'render' ) ); | ||
|
||
add_filter( | ||
'wp_resource_hints', | ||
$this->get_dns_prefetch_hints_callback( '//fundingchoicesmessages.google.com' ), | ||
10, | ||
2 | ||
); | ||
} | ||
|
||
/** | ||
* Outputs the AdSense script tag. | ||
* | ||
* @since n.e.x.t | ||
*/ | ||
protected function render() { | ||
$tags = $this->ad_blocking_recovery_tag->get(); | ||
|
||
if ( empty( $tags['tag'] ) || empty( $tags['error_protection_code'] ) ) { | ||
return; | ||
} | ||
|
||
printf( "\n<!-- %s -->\n", esc_html__( 'Google AdSense Ad Blocking Recovery snippet added by Site Kit', 'google-site-kit' ) ); | ||
echo $tags['tag']; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | ||
printf( "\n<!-- %s -->\n", esc_html__( 'End Google AdSense Ad Blocking Recovery snippet added by Site Kit', 'google-site-kit' ) ); | ||
if ( $this->use_error_protection_snippet ) { | ||
printf( "\n<!-- %s -->\n", esc_html__( 'Google AdSense Ad Blocking Recovery Error Protection snippet added by Site Kit', 'google-site-kit' ) ); | ||
echo $tags['error_protection_code']; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | ||
printf( "\n<!-- %s -->\n", esc_html__( 'End Google AdSense Ad Blocking Recovery Error Protection snippet added by Site Kit', 'google-site-kit' ) ); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
tests/phpunit/integration/Core/Tags/Guards/WP_Query_404_GuardTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<?php | ||
/** | ||
* WP_Query_404_GuardTest | ||
* | ||
* @package Google\Site_Kit\Tests\Core\Tags\Guards | ||
* @copyright 2023 Google LLC | ||
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
* @link https://sitekit.withgoogle.com | ||
*/ | ||
|
||
namespace Google\Site_Kit\Tests\Core\Tags\Guards; | ||
|
||
use Google\Site_Kit\Core\Tags\Guards\WP_Query_404_Guard; | ||
use Google\Site_Kit\Tests\TestCase; | ||
|
||
class WP_Query_404_GuardTest extends TestCase { | ||
|
||
public function test_can_activate() { | ||
$guard = new WP_Query_404_Guard(); | ||
|
||
$this->go_to( '/' ); | ||
|
||
$this->assertQueryTrue( 'is_home', 'is_front_page' ); | ||
$this->assertTrue( $guard->can_activate(), 'Should return TRUE when the current page exists (is_home).' ); | ||
} | ||
|
||
public function test_cant_activate_on_404() { | ||
$guard = new WP_Query_404_Guard(); | ||
|
||
$this->go_to( '/?p=123456789' ); | ||
|
||
$this->assertQueryTrue( 'is_404' ); | ||
$this->assertFalse( $guard->can_activate(), 'Should return FALSE when the current page doesnt exist (is_404).' ); | ||
} | ||
|
||
} |
72 changes: 72 additions & 0 deletions
72
tests/phpunit/integration/Modules/AdSense/Ad_Blocking_Recovery_Tag_GuardTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
<?php | ||
/** | ||
* Class Google\Site_Kit\Tests\Modules\AdSense\Ad_Blocking_Recovery_Tag_GuardTest | ||
* | ||
* @package Google\Site_Kit\Tests\Modules\AdSense | ||
* @copyright 2023 Google LLC | ||
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
* @link https://sitekit.withgoogle.com | ||
*/ | ||
|
||
namespace Google\Site_Kit\Tests\Modules\AdSense; | ||
|
||
use Google\Site_Kit\Context; | ||
use Google\Site_Kit\Core\Storage\Options; | ||
use Google\Site_Kit\Modules\AdSense\Settings; | ||
use Google\Site_Kit\Modules\AdSense\Ad_Blocking_Recovery_Tag_Guard; | ||
use Google\Site_Kit\Tests\TestCase; | ||
|
||
/** | ||
* @group Modules | ||
* @group AdSense | ||
*/ | ||
class Ad_Blocking_Recovery_Tag_GuardTest extends TestCase { | ||
|
||
/** | ||
* Settings object. | ||
* | ||
* @var Settings | ||
*/ | ||
private $settings; | ||
|
||
/** | ||
* Ad_Blocking_Recovery_Tag_Guard object. | ||
* | ||
* @var Ad_Blocking_Recovery_Tag_Guard | ||
*/ | ||
private $guard; | ||
|
||
public function set_up() { | ||
parent::set_up(); | ||
|
||
update_option( | ||
Settings::OPTION, | ||
array( | ||
'adBlockingRecoverySetupStatus' => 'tag-placed', | ||
'useAdBlockerDetectionSnippet' => true, | ||
) | ||
); | ||
|
||
$this->settings = new Settings( new Options( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ) ); | ||
$this->guard = new Ad_Blocking_Recovery_Tag_Guard( $this->settings ); | ||
} | ||
|
||
public function test_can_activate() { | ||
$this->assertTrue( $this->guard->can_activate() ); | ||
} | ||
|
||
/** | ||
* @dataProvider data_can_not_activate | ||
*/ | ||
public function test_can_not_activate( $settings ) { | ||
$this->settings->merge( $settings ); | ||
$this->assertFalse( $this->guard->can_activate() ); | ||
} | ||
|
||
public function data_can_not_activate() { | ||
return array( | ||
'when adBlockingRecoverySetupStatus is empty' => array( array( 'adBlockingRecoverySetupStatus' => '' ) ), | ||
'when useAdBlockerDetectionSnippet is falsy' => array( array( 'useAdBlockerDetectionSnippet' => false ) ), | ||
); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works but we can make it easier to test by providing a
WP_Query
to this via the constructor. The global function is a wrapper for calling this onWP_Query
anyways so it's almost the same thing but using dependency injection.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have access to the
WP_Query
object inside theAdSense
module without usingglobal
? If not and we need to use global there, then I think the current solution makes more sense and looks cleaner. Also, the current tests are pretty simple as well, we don't need to specifically make it easier.What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! That's ok though. The module classes are already very interconnected with many other parts, so it wouldn't be a big change there.
I don't feel super strongly about it, and I agree that it's simple in this form too, my thought is that it's better to make the class work as a pure function when possible.