site-kit-wp: Block module setup on Connect More Services screen if ad blocker is detected
Feature Description
As reported by @bethanylang in Ads Module Bug Bash:
As noted in Unable to setup Ads Module with AdBlocker enabled, users who attempt to set up the Ads module from the Settings > Connect More Services screen while an ad blocker is turned on will receive an Error: You are probably offline error, because the ad blocker is blocking the connection to the ads endpoint.
To address this, we should check for an ad blocker first and not allow users to complete setup when one is detected, similar to the functionality for the AdSense module:
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- When
Ad blockeris detectedAdsmodule should behave likeAdSensemodule is currently:- in connect more services tab - the module box should be greyed out and notice about
Ad blockerdetection should be shown - In setup module screen - if
Ad blockeris detected a warning should be shown instead of the setup form
- in connect more services tab - the module box should be greyed out and notice about
Implementation Brief
- Move
assets/js/modules/ads/components/common/AdBlockerWarning.jsto theassets/js/components/AdBlockerWarning.js- Introduce new props:
helpURLto be used insteadgetDocumentationLinkURLselector, so link will be passed directly and conditionally included if present. AndwarningMessageprop which will replace the current implementation of sourcing it from module usinggetAdBlockerWarningMessage. So message can be passed directly - Update
AdBlockerWarningusage in AdSense module to implement the new import path and props.
- Introduce new props:
- Add
assets/js/googlesitekit/datastore/user/adblocker.js- Move the common actions/selectors from
assets/js/modules/adsense/datastore/adblocker.jshere. Keep onlygetAdBlockerWarningMessagein the AdSense datastore. - Update the references to implement actions/selectors from user datastore. Except for usage of
getAdBlockerWarningMessage
- Move the common actions/selectors from
- Add
assets/js/modules/ads/datastore/adblocker.js- Implement the
getAdBlockerWarningMessagefromassets/js/modules/adsense/datastore/adblocker.jsmodifying the copy to mentionAdsmodule and check for it inisModuleConnected
- Implement the
- In
assets/js/modules/ads/index.js- Update the setup to include
checkRequirementsproperty, you can copy it over fromAdSensemodule https://github.com/google/site-kit-wp/blob/7170f8ec269b26e48db909c0100f1d1ceab7b2b1/assets/js/modules/adsense/index.js#L71-L89 just adapt thegetAdBlockerWarningMessageto be sourced from theAdsdatastore
- Update the setup to include
- Update
assets/js/modules/ads/components/setup/SetupMain.js- Conditionally render the
SetupFormbased onisAdBlockerActiveand render theAdBlockerWarningcomponent (added earlier in the Ads folder structure) above the setup form. You can replicate how it is done inassets/js/modules/adsense/components/setup/SetupMain.js
- Conditionally render the
- Slug for Ads module documentation url should be
ads-ad-blocker-detectedand for the AdSenseadsense-ad-blocker-detected
Test Coverage
- Add tests and stories for new components. You can adapt them from Adsense counterparts
QA Brief
- This PR touches and refactors both, AdSense and Ads module components. Thus, ensure to test the following scenarios for regressions in addition to verifying the ACs:
- Setup flow for AdSense with and without an AdBlocker.
- Setup flow for Ads with and and without an AdBlocker.
- Editing AdSense settings with and without an AdBlocker.
- Editing Ads settings with and without and AdBlocker.
- Monetisation / AdSense widgets on the main dashboard with and without an AdBlocker.
Changelog entry
- Block Ads module setup when ad blocker is detected.
About this issue
- Original URL
- State: closed
- Created 3 months ago
- Comments: 23
Yeah, @eugene-manuilov 's suggestion makes sense to me:
ads-ad-blocker-detectedandadsense-ad-blocker-detected.@eugene-manuilov thanks, added new slug in the IB
Yes that’s a good pint, it will make component more flexible. Updated IB to include that suggestion
@eugene-manuilov Actually yes I like your idea more. Let’s go with
ads-ad-blocker-detectedandadsense-ad-blocker-detected. That makes more sense. In which case as you say we will need to update the current entry for AdSense and create a new entry for the Ads module entry.Would you like me to make these updates in the sheet?
@eugene-manuilov The slug is named module agnostic, true, but the link it leads to https://sitekit.withgoogle.com/documentation/troubleshooting/adsense/#ad-blocker-detected is all about AdSense. This is the tread about new article for Ads module, that support is working on
@jamesozzie @adamdunnage Tagging you here so you can include the support link once it is online.Here is the slack chat for reference. Thanks
cc: @bethanylang