Ad blocker warning message is inconsistent in different locations

[Pratheep-lab]

It says " Ad blocker detected, you need to disable it in order to set up AdSense."

However, the AdSense setup has already been done and I can see that AdSense is working properly.

Maybe the message should say "Ad blocker detected, you need to disable it in order for AdSense to work properly."

Steps to reproduce

1. Setup AdSense on Site Kit without any Ad Blockers
2. Enable an adblocker. In this instance, I used https://adblockplus.org/
3. Open the AdSense details page.
4. You can see the above error. Also seen on the AdSense module on the dashboard.

Screenshots

---

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

Acceptance criteria

• The ad blocker warning should be consistent in all locations under the same conditions. Currently the locations are as follows:
  • Setup CTA (from SK success notification on main dashboard)
  • Setup module (on Connect More Services tab of settings)
  • AdSense settings view (when setup is incomplete)
  • AdSense setup
  • Dashboard summary widget
  • Dashboard top earning pages widget (only shown when Analytics is also activated and linked with the connected AdSense account)
  • The module page will be covered separately in AdSense Module Page no longer shows Adblocker warning message. #4178
• A new getAdBlockerWarningMessage selector should be added to the AdSense datastore to be used in both places
• The logic in the AdBlockerWarning component for determining which message to use should be simplified to come from the new getAdBlockerWarningMessage selector rather than using its own logic based on site and account setup completion.

Implementation Brief

Inside assets/js/modules/adsense/datastore/adblocker.js

• Add a new selector, getAdBlockerWarningMessage with the following logic:
  • if there is no ad blocker, return null
  • if the AdSense module is connected and there is an ad blocker, return 'Ad blocker detected, you need to disable it to get the AdSense latest data.'
  • if the AdSense module is not connected and there is an ad blocker, return 'Ad blocker detected, you need to disable it in order to set up AdSense.'
  • Make sure to use i18n for the messages, i.e.
    __( 'Ad blocker detected, you need to disable it in order to set up AdSense.','google-site-kit' )
  • You can use the AdSense module's isAdBlockerActive to determine if the Ad Blocker is active.
  • You can use the select( CORE_MODULES ).isModuleConnected to determine if the AdSense module is connected.
  • Add test coverage to assets/js/modules/adsense/datastore/adblocker.test.js for this new selector.

Inside assets/js/modules/adsense/index.js:

• Refactor the checkRequirements logic to use the new selector instead of isAdBlockerActive
• If the selector returns null / falsey, return (as it does currently).
• Otherwise throw (as it does currently), but pass the message returned by the selector.

Inside assets/js/modules/adsense/components/common/AdBlockerWarning.js:

• Refactor the logic by removing the current use of isAccountSetupComplete and isSiteSetupComplete and replacing them with a call to the new selector.
• If the selector returns null / falsey, return null from the component.
• Otherwise, render the message returned by the selector.

Test Coverage

• See above: coverage will be needed for the new selector.

Visual Regression Changes

• Images relating to the ad blocker messages may need updating.

QA Brief

• Check for the correct adblock warning message when adsense module is connected and not connected and adblock is active.
• The warning messages should match the one in the AC.
• Newly added and update unit tests should also pass.

Changelog entry

• Fix wording of ad blocker warning to always reflect the current state of the AdSense module.

[aaemnnosttv]

@felixarntz how does ^ sound to you?

[felixarntz]

@aaemnnosttv Makes sense. Another nit-pick enhancement/simplification we could make as part of this potentially: Right now the condition to decide between the two messages in AdBlockerWarning is unnecessarily complicated. A simple isAdSenseModuleConnected would work and be more decoupled from the AdSense setup details currently used.

[aaemnnosttv]

IB ✅

[aaemnnosttv]

@felixarntz would you please confirm the proposed changes to the wording of the messages?

See https://github.com/google/site-kit-wp/pull/4065/files#r714314465, https://github.com/google/site-kit-wp/pull/4065/files#r714315039

[wpdarren]

QA Update ⚠️

@kuasha420 I would like some clarification on the QAB please.

According to the ticket description, there's an Adblock message when you are on the AdSense module. When AdSense was set up no Adblock was activated. Then, an Adblock was activated after Adsense was connected, and the message appeared.

This is not stated in the AC or QAB, but what message should I expect to see in the scenario above?

At the moment, going through this scenario, no message appears on the AdSense module with an Adblock enabled.

[eugene-manuilov]

@wpdarren I assume you should see the Ad blocker detected, you need to disable it to get the latest AdSense data. on the AdSense module page if the AdSense module is already connected.

[aaemnnosttv]

@wpdarren @eugene-manuilov It seems the warning on the module page was lost in the migration to using the widget-based dashboard in 1.38.0, although the same can be seen with the feature flag enabled in 1.37.0 (without it the warning shows). The ModuleSettingsWarning component is no longer shown on the module dashboard which is where this would have been shown before. I don't recall if this was intentional but given that it's not a regression from this issue I don't think it needs to block it. We can raise this in a new issue to fix it if it should indeed be restored.

@felixarntz can you clarify if this removal was intentional?

[wpdarren]

QA Update: ⚠️

@aaemnnosttv @eugene-manuilov @felixarntz

I want to make sure I have not missed anything here.

1. When AdSense is not set up, but Adblocker is enabled, I see the message Ad blocker detected, you need to disable it in order to set up AdSense. on the 'Connect more Services' and Site Kit Dashboard CTA banner when you connect Site Kit. - Screenshot 1 - Screenshot 2

2. When AdSense is set up with Adblocker disabled. Then, it is enabled, when I go to the AdSense module, no message appears at the top, which is what we were discussing in previous comments. Due to this I am unable to move this ticket into approval because I cannot confirm the correct message appears. Ad blocker detected, you need to disable it to get the AdSense latest data.

What are your thoughts?

[eugene-manuilov]

@wpdarren, yes, both statements are correct. We need to figure out why we don't see the second message on the AdSense module page. I think we can consider this ticket as such that is not passed QA.

[kuasha420]

Created a new issue to track this issue. I think we can add that as a blocker for this one.

#4178

[aaemnnosttv]

Thanks @kuasha420. While this issue was originally defined referring to the module page, the issue is more about the message being consistent in all locations, under the same conditions. IMO, we should redefine this issue to not be specific to the module page and fix that in the follow-up issue, it shouldn't block this one. Assigning to @felixarntz to give the final word.

[felixarntz]

@aaemnnosttv @kuasha420 Not sure if I understand correctly - did the warning on the module page get broken by the changes in this very issue? If so, I'd say it shouldn't even be a new issue, but rather go into a follow-up PR of this one - while that point wasn't mentioned in the ACs explicitly, there shouldn't be any breakage of course :)

[felixarntz]

@wpdarren @aaemnnosttv @kuasha420 Per our conversation in the meeting, this can be considered done. Let's prioritize #4178 for next sprint. cc @eclarke1 @fhollis

[aaemnnosttv]

@felixarntz I've updated the title and ACs here to reflect our discussion earlier for this to focus purely on the consistency.

[wpdarren]

QA Update: ✅

As per conversation, this ticket is verified:

Checked all areas in the AC, i.e. when AdSense is not set up, but Adblocker is enabled, I see the message Ad blocker detected, you need to disable it in order to set up AdSense. on the 'Connect more Services' and Site Kit Dashboard CTA banner when you connect Site Kit. - Screenshot 1 - Screenshot 2

The initial issue reported will be fixed in #4178.