Update Consent Mode once the Ads Conversion ID setting has been migrated to the Ads module.
Feature Description
Ensure that the relevant parts of Consent Mode are updated once the Ads Conversion ID setting has been migrated to the Ads module.
Some of this will almost certainly be updated as part of the Ads Module work, this issue is to ensure nothing is missed.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Once the Ads Conversion ID has been migrated to the Ads Module:
- All references to it in the Consent Mode feature should be updated to use the value from the Ads Module.
- The confirmation dialog shown when disabling Consent Mode on the Settings screen should be updated to include the Ads module when it's connected.
Implementation Brief
An initial IB has been drafted, to be completed.
- [ ] Ensure the following code snippets are updated to get the
adsConversionIDfrom theadsmodule:
https://github.com/google/site-kit-wp/blob/26e6a4318b09e47796fa5c02adf263ab756cb383/assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js#L72-L75
https://github.com/google/site-kit-wp/blob/26e6a4318b09e47796fa5c02adf263ab756cb383/assets/js/components/settings/SettingsCardConsentMode.js#L50-L53
https://github.com/google/site-kit-wp/blob/26e6a4318b09e47796fa5c02adf263ab756cb383/assets/js/components/consent-mode/ConfirmDisableConsentModeDialog.js#L45-L47
Test Coverage
- Update the
assets/js/components/consent-mode/ConsentModeSetupCTAWidget.stories.jsstory to use the ads store.
QA Brief
Changelog entry
I have reviewed the current consent mode for any use of the adsConversionID and all cases are covered in the IB.
IB ✔️
Hi @zutigrm & @techanvil.
It looks like this change in the PR has stopped the consent mode banner from appearing if the Ads module is not connected.
The above change implies that the consent mode banner should only show up if both the Analytics & Ads modules are connected. However, it is possible for Google Ads to be connected even without one of the modules disconnected. Examples:
- Analytics module is not connected, but Ads module is connected with an Ads conversion tracking ID.
- Ads module is not connected, but Analytics module is connected and:
- It is linked with Google Ads (
adsLinked) and/or - The connected Google Tag has an Ads conversion ID as one of its destinations.
- It is linked with Google Ads (
However, with the above linked change, the banner will only appear if both Ads and Analytics modules are connected, which I think is not right. Thoughts?
Thanks @nfmohit, great spot. That one totally slipped by me.
@zutigrm, please can you file a followup PR to update the condition that Nahid's highlighted? It should be returning false if neither of the modules are connected, e.g. along these lines:
if (
! (
isModuleConnected( 'analytics-4' ) || isModuleConnected( 'ads' )
)
) {
return false;
}
With this being a release issue we should address this as a priority. Thanks!
@mohitwp, I've moved this back from QA to Execution and assigned it back to Aleksej to address the above.
Apologies, I hadn't spotted @zutigrm is off today. @nfmohit has kindly offered to raise a PR to address this.
Thanks @nfmohit, great spot. That one totally slipped by me.
@zutigrm, please can you file a followup PR to update the condition that Nahid's highlighted? It should be returning false if neither of the modules are connected, e.g. along these lines:
if ( ! ( isModuleConnected( 'analytics-4' ) || isModuleConnected( 'ads' ) ) ) { return false; }With this being a release issue we should address this as a priority. Thanks!
@mohitwp, I've moved this back from QA to Execution and assigned it back to Aleksej to address the above.
Thank you @techanvil . This is in CR now with a follow-up PR.
Top stuff, thanks @nfmohit. The PR is now merged, back to you @mohitwp to continue the QA for this one.
QA Update ⚠️
- Tested on main environment.
- Hey @nfmohit , according to the QAB Consent mode, the banner only appears if the Ads module is connected, regardless of whether analytics is connected or not. I just want to confirm if this information is correct, especially if the QAB hasn't been updated after recent changes. Could you please review and confirm?
If only Analytics is connected
Both Analytics and Ads Module connected-
Only Ads module is connected -
Ads Module connected with Analytics
Ads Module connected without Analytics
Recommended badge -
Recommended badge not showing if analytics is not connected-
- Hey @nfmohit , according to the QAB Consent mode, the banner only appears if the Ads module is connected, regardless of whether analytics is connected or not. I just want to confirm if this information is correct, especially if the QAB hasn't been updated after recent changes. Could you please review and confirm?
Hi @mohitwp. That is correct. If the Ads module is connected, we don't need the Analytics module to also be connected in this context. Thanks!
QA Update ✅
- Tested on main environment.
- Verified according to the QAB Consent mode, the banner only appears if the Ads module is connected, regardless of whether analytics is connected or not.
- Verified Recommended badge is appearing only if Ads module is connected.
- Verified Copy under disconnect modal is getting update as per which module is active.
If only Analytics is connected
Both Analytics and Ads Module connected-
Only Ads module is connected -
Ads Module connected with Analytics
Ads Module connected without Analytics
Recommended badge -
Recommended badge not showing if analytics is not connected-