-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add integration with WP Consent API #425
Conversation
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.
I run tests locally - works well. However, initially, I get some quirk that WP Consent API's cookie was not created after allowing in Complianz modal. Restarting new incognito mode helped. I'll take some time to try to reproduce it, not to miss something.
I left some comments regarding the code. The most important are marked with 🚧
Hey @tomalec, thanks for the review 🙌 I've made some adjustments and responded to your comments so this is ready for a second round. |
After adding the fixes I suggested, I tested the branch with the following extensions:
I added the WP Consent API steps to general testing wiki https://github.com/woocommerce/woocommerce-google-analytics-integration/wiki/General-Testing#12-wp-consent-api-integration I also created Setup & Smoke testing instructions for those three extensions in a separate Wiki: https://github.com/woocommerce/woocommerce-google-analytics-integration/wiki/Smoke-Testing-WP-Consent-API-integration-with-other-extensions. I know we don't have the capacity to maintain support for all of them. Still, IMHO, it would be good to at least one integration works. I made a PR with a small README note about WP Consent API and UI for banners.
Do you have anything started, or can I pick that up? |
Hey @tomalec, many thanks for the additional testing and for updating the documentation. I've made some changes based on our discussions so far if you could please take another look. |
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.
Reviewed the code, tested locally. Using Polish IP, and US one, tested with two cookie banner plugins and fiddling manually with API.
LGTM
Changes proposed in this Pull Request:
Closes #424
Adds integration with the WP Consent API plugin to better support different consent banner configurations.
If the plugin is installed on a website then the existing consent state will be sent as an update when tracking is initialised. For on-page updates, we're adding an event listener for
wp_listen_for_consent_change
which is dispatched by WP Consent API when any changes are made.Detailed test instructions:
General test instruction
add/424-integration-with-wp-consent-api
Consent
event is sent with the updated stateOn-page Update
isGranted
(Under the Consent tab)Granted
in Tag AssistantEvent order testing
By default, the WP Consent API plugin JavaScript loads in the footer meaning we are unable to send the consent update event with the current state until after the page loads.
We need to support calls to
gtag
from on the page for Google Listings & Ads and other third-party extensions. To do that we're including thewait_for_update
parameter as recommended by Google so that events sent before the consent update will still be recorded. 57ec41eGranted
Consent
tab and confirm that theOn-page Update
is listed asGranted
Without
wait_for_update
:With
wait_for_update
:Additional details:
ga4w
data object so that immediately after we set the consent defaults the update can be sent. While this would be preferable, caching and minification plugins would cause problems with it which is why we're relying on a purely JS implementation insteadChangelog entry