MR-4: Ad Suppression Logic (Raptive Ads/AdThrive)#6
Open
Conversation
…to use new action
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an ad-provider subsystem: manager, base provider, Raptive and MediaVine providers, settings storage/sanitisation, admin UI/tab and styles, URL helper, and plugin bootstrap inclusion; version bumped to 1.78.0. Changes
Sequence Diagram(s)sequenceDiagram
participant Visitor as Visitor/User
participant WP as WordPress
participant Manager as Ad Provider Manager
participant Provider as Ad Provider
participant Settings as Settings Store
Visitor->>WP: Request page
WP->>Manager: apply_ad_controls_for_user()
Manager->>Manager: get_detected_providers()
Manager->>Provider: apply_ad_controls_for_user(user_id)
Provider->>Settings: get_ad_provider_settings()
Settings-->>Provider: provider settings
Provider->>Provider: should_disable_ads_for_user(user_id)
alt Ads should be disabled
Provider->>Provider: disable_ads_for_user(user_id)
Provider->>WP: attach filter/action (e.g. body_class or dequeue script)
end
WP->>WP: Render page with modifications
WP-->>Visitor: Serve page (ads disabled/enabled)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/raptive-ads.php`:
- Around line 19-21: The is_installed() method calls is_plugin_active() which
isn't available on front-end and causes a fatal; in the raptive-ads provider's
is_installed() implement a guard that checks function_exists('is_plugin_active')
and, if missing, require_once the WP plugin helper
(wp-admin/includes/plugin.php) before calling is_plugin_active(); this change
will prevent get_detected_providers() (called from init()) from invoking an
undefined function during front-end init. Ensure you update the is_installed()
method to perform function_exists('is_plugin_active') and load the helper only
when necessary.
🧹 Nitpick comments (3)
wordpress/wp-content/plugins/memberful-wp/stylesheets/admin.css (1)
267-272: Consider using consistent units and more specific selectors.The existing stylesheet uses
pxandemunits, whilst this new block introducesrem. Additionally, the.memberful-ad-provider-settings divselector is quite broad and could inadvertently affect deeply nested elements.♻️ Suggested refinement
.memberful-ad-provider-settings { - margin-bottom: 1rem; + margin-bottom: 1em; } -.memberful-ad-provider-settings div { - margin-left: 1rem; +.memberful-ad-provider-settings > div { + margin-left: 1em; }wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers.php (1)
24-28: Typo in variable name.The variable
$ad_provider_mangeris missing an 'a' — should be$ad_provider_manager.♻️ Suggested fix
function memberful_wp_ad_providers_register_providers() { - $ad_provider_manger = Memberful_Wp_Integration_Ad_Provider_Manager::instance(); + $ad_provider_manager = Memberful_Wp_Integration_Ad_Provider_Manager::instance(); // Raptive Ads (AdThrive Ads). - $ad_provider_manger->register_provider( new Memberful_Wp_Integration_Ad_Provider_Raptive() ); + $ad_provider_manager->register_provider( new Memberful_Wp_Integration_Ad_Provider_Raptive() ); }wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/raptive-ads.php (1)
31-33: Optional: silence the unused parameter warning.
PHPMD flags$user_idas unused; renaming it (or unsetting it) keeps CI quiet. Based on static analysis, PHPMD reports an unused parameter here.♻️ Possible tidy‑up
- public function disable_ads_for_user($user_id) { + public function disable_ads_for_user($_user_id) { add_filter( 'body_class', array( $this, 'disable_ads_body_class' ) ); }
wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/raptive-ads.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/raptive-ads.php`:
- Around line 35-37: The method disable_ads_for_user currently has an unused
parameter $user_id which triggers PHPMD; to silence the warning without changing
the signature, explicitly mark the parameter as unused inside the method (e.g.
add a line such as (void)$user_id; or unset($user_id);) and keep the existing
add_filter call in disable_ads_for_user so the signature remains intact and
PHPMD no longer flags UnusedFormalParameter.
Comment on lines
+35
to
+37
| public function disable_ads_for_user($user_id) { | ||
| add_filter( 'body_class', array( $this, 'disable_ads_body_class' ) ); | ||
| } |
There was a problem hiding this comment.
Silence PHPMD unused-parameter warning in disable_ads_for_user()
Line 35 triggers PHPMD; suppress it to keep linting clean while preserving the required signature.
🔧 Proposed fix
- public function disable_ads_for_user($user_id) {
+ /**
+ * Disable ads for a user.
+ *
+ * `@param` int $user_id The ID of the user to disable ads for.
+ * `@SuppressWarnings`(PHPMD.UnusedFormalParameter)
+ */
+ public function disable_ads_for_user($user_id) {
add_filter( 'body_class', array( $this, 'disable_ads_body_class' ) );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function disable_ads_for_user($user_id) { | |
| add_filter( 'body_class', array( $this, 'disable_ads_body_class' ) ); | |
| } | |
| /** | |
| * Disable ads for a user. | |
| * | |
| * `@param` int $user_id The ID of the user to disable ads for. | |
| * `@SuppressWarnings`(PHPMD.UnusedFormalParameter) | |
| */ | |
| public function disable_ads_for_user($user_id) { | |
| add_filter( 'body_class', array( $this, 'disable_ads_body_class' ) ); | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
35-35: Avoid unused parameters such as '$user_id'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/raptive-ads.php`
around lines 35 - 37, The method disable_ads_for_user currently has an unused
parameter $user_id which triggers PHPMD; to silence the warning without changing
the signature, explicitly mark the parameter as unused inside the method (e.g.
add a line such as (void)$user_id; or unset($user_id);) and keep the existing
add_filter call in disable_ads_for_user so the signature remains intact and
PHPMD no longer flags UnusedFormalParameter.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@wordpress/wp-content/plugins/memberful-wp/readme.txt`:
- Around line 115-123: Update the changelog entry under "= 1.78.0 =" to remove
the duplicated word "users" in the sentence "Add ad suppression settings to
allow users to disable Raptive Ads for users"; change it to a concise phrase
such as "Add ad suppression settings to allow users to disable Raptive Ads" (or
"allow site admins to disable Raptive Ads for users" if you mean admins) so the
intent is clear and the repetition is eliminated.
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/base-ad-provider.php`:
- Around line 45-78: The base stub disable_ads_for_user currently accepts
$user_id but is empty; silence the unused-parameter warning by explicitly
marking $user_id as unused inside the method (e.g., add a (void)$user_id; or
unset($user_id); statement) in the disable_ads_for_user($user_id) implementation
so PHPMD/linters stop reporting an unused parameter while preserving the empty
stub behavior.
- Around line 99-135: The call to array_intersect in should_disable_ads_for_user
can receive non-array inputs; cast both operands defensively to arrays: coerce
the result of memberful_wp_user_plans_subscribed_to($user_id) and
$settings['disabled_plans'] to (array) before passing them into array_intersect
so the expression count(array_intersect(...)) is safe and consistent with other
code (refer to function should_disable_ads_for_user, array_intersect,
memberful_wp_user_plans_subscribed_to, and $settings['disabled_plans']).
Comment on lines
115
to
123
| = 1.78.0 = | ||
|
|
||
| * Add ad suppression settings to allow users to disable Raptive Ads for users | ||
|
|
||
| = 1.77.0 = | ||
|
|
||
| * Add per-plan role mappings | ||
| * Add block editor visibility controls | ||
| * Add an option to include protected content in search |
There was a problem hiding this comment.
Tidy the changelog wording.
“users” is repeated; a small copy tweak keeps it cleaner.
✏️ Suggested copy tweak
-* Add ad suppression settings to allow users to disable Raptive Ads for users
+* Add ad suppression settings to allow members to disable Raptive Ads📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| = 1.78.0 = | |
| * Add ad suppression settings to allow users to disable Raptive Ads for users | |
| = 1.77.0 = | |
| * Add per-plan role mappings | |
| * Add block editor visibility controls | |
| * Add an option to include protected content in search | |
| = 1.78.0 = | |
| * Add ad suppression settings to allow members to disable Raptive Ads | |
| = 1.77.0 = | |
| * Add per-plan role mappings | |
| * Add block editor visibility controls | |
| * Add an option to include protected content in search |
🤖 Prompt for AI Agents
In `@wordpress/wp-content/plugins/memberful-wp/readme.txt` around lines 115 - 123,
Update the changelog entry under "= 1.78.0 =" to remove the duplicated word
"users" in the sentence "Add ad suppression settings to allow users to disable
Raptive Ads for users"; change it to a concise phrase such as "Add ad
suppression settings to allow users to disable Raptive Ads" (or "allow site
admins to disable Raptive Ads for users" if you mean admins) so the intent is
clear and the repetition is eliminated.
Comment on lines
+45
to
+78
| /** | ||
| * Check if the ad provider is installed. | ||
| * | ||
| * @return bool True if the ad provider is installed, false otherwise. | ||
| */ | ||
| public function is_installed() {} | ||
|
|
||
| /** | ||
| * Get the human-readable name of the ad provider. | ||
| * | ||
| * @return string The human-readable name of the ad provider. | ||
| */ | ||
| public function get_name() {} | ||
|
|
||
| /** | ||
| * Get the identifier of the ad provider. | ||
| * | ||
| * @return string The identifier of the ad provider. | ||
| */ | ||
| public function get_identifier() {} | ||
|
|
||
| /** | ||
| * Disable ads for a user. | ||
| * | ||
| * @param int $user_id The ID of the user to disable ads for. | ||
| */ | ||
| public function disable_ads_for_user($user_id) {} | ||
|
|
||
| /** | ||
| * Get the detection methods for the ad provider. | ||
| * | ||
| * @return array An array of detection methods. | ||
| */ | ||
| public function get_detection_methods() {} |
There was a problem hiding this comment.
Silence the unused‑parameter warning in the base stub.
disable_ads_for_user() is intentionally empty, so $user_id should be explicitly marked unused to avoid PHPMD noise.
🧹 Minimal lint‑fix
public function disable_ads_for_user($user_id) {}
+// or
+ public function disable_ads_for_user($user_id) {
+ (void) $user_id;
+ }🧰 Tools
🪛 PHPMD (2.15.0)
71-71: Avoid unused parameters such as '$user_id'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/base-ad-provider.php`
around lines 45 - 78, The base stub disable_ads_for_user currently accepts
$user_id but is empty; silence the unused-parameter warning by explicitly
marking $user_id as unused inside the method (e.g., add a (void)$user_id; or
unset($user_id); statement) in the disable_ads_for_user($user_id) implementation
so PHPMD/linters stop reporting an unused parameter while preserving the empty
stub behavior.
Comment on lines
99
to
135
| public function should_disable_ads_for_user($user_id) { | ||
| /** | ||
| * Filter if ads should be disabled for a user. | ||
| * | ||
| * @param bool|null $should_disable Whether to disable ads for the user, or null to use default logic. | ||
| * @param int $user_id The ID of the user to check. | ||
| * @param string $provider_id The identifier of the ad provider. | ||
| * @return bool|null True if ads should be disabled, false otherwise, or null to use default logic. | ||
| */ | ||
| $should_disable = apply_filters( 'memberful_ad_provider_should_disable_ads_for_user', null, $user_id, $this->get_identifier() ); | ||
|
|
||
| if ( $should_disable !== null ) { | ||
| return (bool) $should_disable; | ||
| } | ||
|
|
||
| $settings = $this->get_ad_provider_settings(); | ||
|
|
||
| // Provider is not enabled. | ||
| if ( empty( $settings ) || ! isset( $settings['enabled'] ) || $settings['enabled'] !== true ) { | ||
| return false; | ||
| } | ||
|
|
||
| // Disable for all subscribers. | ||
| if ( isset( $settings['disable_for_all_subscribers'] ) && $settings['disable_for_all_subscribers'] === true && is_subscribed_to_any_memberful_plan( $user_id ) ) { | ||
| return true; | ||
| } | ||
|
|
||
| // Disable for logged in users. | ||
| if ( isset( $settings['disable_for_logged_in'] ) && $settings['disable_for_logged_in'] === true && is_user_logged_in() ) { | ||
| return true; | ||
| } | ||
|
|
||
| // Disable for specific plans. | ||
| if ( isset( $settings['disabled_plans'] ) && ! empty( $settings['disabled_plans'] ) ) { | ||
| $user_plans = memberful_wp_user_plans_subscribed_to( $user_id ); | ||
| return count( array_intersect( $user_plans, $settings['disabled_plans'] ) ) > 0; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the function definition of memberful_wp_user_plans_subscribed_to
rg -n "function memberful_wp_user_plans_subscribed_to" -A 20 --type phpRepository: TheCodeCompany/memberful-wp
Length of output: 1871
🏁 Script executed:
# Search for where disabled_plans is set in settings
rg -n "disabled_plans" -B 3 -A 3 --type phpRepository: TheCodeCompany/memberful-wp
Length of output: 6930
Guard array_intersect() inputs with defensive casts.
memberful_wp_user_plans_subscribed_to() lacks an explicit type guarantee and could theoretically return a non-array value. Similarly, whilst $settings['disabled_plans'] is sanitised during settings processing, the codebase elsewhere (see ad-control.php:77) already uses defensive (array) casting for such values. Adding explicit casts here prevents potential TypeErrors and aligns with existing defensive practices.
🛡️ Defensive cast
- $user_plans = memberful_wp_user_plans_subscribed_to( $user_id );
- return count( array_intersect( $user_plans, $settings['disabled_plans'] ) ) > 0;
+ $user_plans = (array) memberful_wp_user_plans_subscribed_to( $user_id );
+ $disabled_plans = (array) $settings['disabled_plans'];
+ return count( array_intersect( $user_plans, $disabled_plans ) ) > 0;🤖 Prompt for AI Agents
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/base-ad-provider.php`
around lines 99 - 135, The call to array_intersect in
should_disable_ads_for_user can receive non-array inputs; cast both operands
defensively to arrays: coerce the result of
memberful_wp_user_plans_subscribed_to($user_id) and $settings['disabled_plans']
to (array) before passing them into array_intersect so the expression
count(array_intersect(...)) is safe and consistent with other code (refer to
function should_disable_ads_for_user, array_intersect,
memberful_wp_user_plans_subscribed_to, and $settings['disabled_plans']).
Co-authored-by: Patrik Bóna <mrhead@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/mediavine-ads.php (2)
60-62: Suppress or document the unused$user_idparameter to silence the PHPMD warning
$user_idis required by the base interface but not forwarded to thewp_enqueue_scriptscallback. Add an inline suppression annotation or a brief note:✏️ Proposed fix
+ // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.Found -- required by interface; suppression is applied globally for current user. public function disable_ads_for_user( $user_id ) { add_action( 'wp_enqueue_scripts', array( $this, 'disable_mediavine_ads' ), 99 ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/mediavine-ads.php` around lines 60 - 62, The $user_id parameter in disable_ads_for_user is unused and triggers PHPMD; to silence it, explicitly mark it as used (e.g., add a single line like (void)$user_id; at the start of the disable_ads_for_user method) or add a docblock suppression for PHPMD (e.g., `@SuppressWarnings` for UnusedFormalParameter) above the disable_ads_for_user declaration so the signature stays intact while the linter is satisfied; the wp_enqueue_scripts hook should remain calling $this->disable_mediavine_ads as currently written.
69-72: Reorder to the conventional WordPress pattern:wp_dequeue_scriptbeforewp_deregister_scriptThe WordPress best practice (when using both) is to dequeue first, then deregister. The current reversed order (deregister → dequeue) is functionally safe, but dequeuing should come first to ensure the script is removed from the page's render queue before deregistering from the registry. Both calls are necessary here and neither is redundant.
♻️ Suggested reorder
public function disable_mediavine_ads() { - wp_deregister_script( 'mv-script-wrapper' ); wp_dequeue_script( 'mv-script-wrapper' ); + wp_deregister_script( 'mv-script-wrapper' ); }The
mv-script-wrapperhandle is confirmed correct in the mediavine-control-panel plugin, and the priority 99 onwp_enqueue_scriptsensures this runs after the plugin's original enqueue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/mediavine-ads.php` around lines 69 - 72, In disable_mediavine_ads, swap the calls so wp_dequeue_script('mv-script-wrapper') is executed before wp_deregister_script('mv-script-wrapper'); locate the disable_mediavine_ads method and reorder the two function calls (keeping the same handle 'mv-script-wrapper') so the script is removed from the queue prior to being deregistered.wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers.php (1)
26-35: Use the$managerinstance passed bydo_actionrather than callinginstance()againThe
memberful_ad_provider_register_providersaction explicitly passes the manager as its first argument (visible inad-provider-manager.phpline 63:do_action( 'memberful_ad_provider_register_providers', $this )). Consuming it directly is more idiomatic, avoids a redundant static call, and sets the right example for any third-party callbacks hooking the same action.♻️ Proposed refactor
-function memberful_wp_ad_providers_register_providers() { - $ad_provider_manager = Memberful_Wp_Integration_Ad_Provider_Manager::instance(); - +function memberful_wp_ad_providers_register_providers( $ad_provider_manager ) { // Raptive Ads (AdThrive Ads). $ad_provider_manager->register_provider( new Memberful_Wp_Integration_Ad_Provider_Raptive() ); // MediaVine Ads. $ad_provider_manager->register_provider( new Memberful_Wp_Integration_Ad_Provider_Mediavine() ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers.php` around lines 26 - 35, The function memberful_wp_ad_providers_register_providers currently calls Memberful_Wp_Integration_Ad_Provider_Manager::instance() instead of using the manager passed by the action; change the function signature to accept the manager parameter (e.g. function memberful_wp_ad_providers_register_providers($manager)) and replace the instance() call with the passed $manager->register_provider(...) calls for Memberful_Wp_Integration_Ad_Provider_Raptive and Memberful_Wp_Integration_Ad_Provider_Mediavine; ensure the add_action registration still allows one argument (accepted args) so the passed manager is received.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/mediavine-ads.php`:
- Around line 33-39: is_installed() currently only checks that the Mediavine
Control Panel plugin is active, not whether its "Include Script Wrapper" option
is enabled, so add a defensive check: after verifying is_plugin_active in
is_installed(), read the MCP settings (e.g., via
get_option('mediavine_control_panel') or the appropriate Mediavine option array)
and confirm the include_script_wrapper (or equivalent) flag is truthy; if that
option is not present or false, return false. Reference the is_installed()
method, the MCP enqueue_scripts() behavior that registers mv-script-wrapper only
when $site_id && $use_wrapper, and the mv-script-wrapper handle when adding this
option check, and include a brief docblock note on the limitation/fallback if
the option can't be read.
---
Nitpick comments:
In `@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers.php`:
- Around line 26-35: The function memberful_wp_ad_providers_register_providers
currently calls Memberful_Wp_Integration_Ad_Provider_Manager::instance() instead
of using the manager passed by the action; change the function signature to
accept the manager parameter (e.g. function
memberful_wp_ad_providers_register_providers($manager)) and replace the
instance() call with the passed $manager->register_provider(...) calls for
Memberful_Wp_Integration_Ad_Provider_Raptive and
Memberful_Wp_Integration_Ad_Provider_Mediavine; ensure the add_action
registration still allows one argument (accepted args) so the passed manager is
received.
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/mediavine-ads.php`:
- Around line 60-62: The $user_id parameter in disable_ads_for_user is unused
and triggers PHPMD; to silence it, explicitly mark it as used (e.g., add a
single line like (void)$user_id; at the start of the disable_ads_for_user
method) or add a docblock suppression for PHPMD (e.g., `@SuppressWarnings` for
UnusedFormalParameter) above the disable_ads_for_user declaration so the
signature stays intact while the linter is satisfied; the wp_enqueue_scripts
hook should remain calling $this->disable_mediavine_ads as currently written.
- Around line 69-72: In disable_mediavine_ads, swap the calls so
wp_dequeue_script('mv-script-wrapper') is executed before
wp_deregister_script('mv-script-wrapper'); locate the disable_mediavine_ads
method and reorder the two function calls (keeping the same handle
'mv-script-wrapper') so the script is removed from the queue prior to being
deregistered.
Comment on lines
+33
to
+39
| public function is_installed() { | ||
| if ( ! function_exists( 'is_plugin_active' ) ) { | ||
| require_once ABSPATH . 'wp-admin/includes/plugin.php'; | ||
| } | ||
|
|
||
| return is_plugin_active( 'mediavine-control-panel/mediavine-control-panel.php' ); | ||
| } |
There was a problem hiding this comment.
is_installed() returns true even when MCP's "Include Script Wrapper" option is disabled
The detection check confirms the MCP plugin is active, but not whether it's actually managing the script wrapper. The MCP's enqueue_scripts() method only registers mv-script-wrapper when $site_id && $use_wrapper are both set. A site with MCP active but the "Include Script Wrapper" option disabled (or the script injected via theme/header plugin) will have this provider marked as detected, causing disable_ads_for_user() to run a hook that silently has no effect.
Consider documenting this limitation in the class docblock, or adding a more defensive check (e.g., verifying the MCP include_script_wrapper option is set):
📝 Proposed docblock addition
/**
* MediaVine Ads provider class.
*
* Disables ads by preventing the MediaVine script wrapper from being enqueued.
*
+ * Note: This suppression only works when the MediaVine Control Panel plugin
+ * is configured to manage the script wrapper via the "Include Script Wrapper"
+ * option. Sites with the script injected manually (e.g. via theme or a header
+ * plugin) will not be suppressed by this provider.
+ *
* `@see` Memberful_Wp_Integration_Ad_Provider_Base
* `@see` \Mediavine\MCP\MV_Control_Panel::enqueue_scripts()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@wordpress/wp-content/plugins/memberful-wp/src/contrib/ad-providers/mediavine-ads.php`
around lines 33 - 39, is_installed() currently only checks that the Mediavine
Control Panel plugin is active, not whether its "Include Script Wrapper" option
is enabled, so add a defensive check: after verifying is_plugin_active in
is_installed(), read the MCP settings (e.g., via
get_option('mediavine_control_panel') or the appropriate Mediavine option array)
and confirm the include_script_wrapper (or equivalent) flag is truthy; if that
option is not present or false, return false. Reference the is_installed()
method, the MCP enqueue_scripts() behavior that registers mv-script-wrapper only
when $site_id && $use_wrapper, and the mv-script-wrapper handle when adding this
option check, and include a brief docblock note on the limitation/fallback if
the option can't be read.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adds initial Raptive Ads ad suppression logic.
Summary by CodeRabbit
New Features
Style
Documentation