-
Notifications
You must be signed in to change notification settings - Fork 247
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
[Browser] [GA4] Convert Consent Values to Lower Case #2535
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2535 +/- ##
===========================================
- Coverage 78.51% 33.18% -45.33%
===========================================
Files 1044 14 -1030
Lines 20029 693 -19336
Branches 3988 115 -3873
===========================================
- Hits 15726 230 -15496
+ Misses 3209 463 -2746
+ Partials 1094 0 -1094 ☔ View full report in Codecov by Sentry. |
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.
@harsh-joshi99 , Changes and testing screenshots look good to me!
but lets ensure backwards compatibility, some existing destinations have values like $.properties.ads_storage_consent_state
, as well as GRANTED in upper case.
Could you just do a round of testing by adding these values?
4b12ca8
to
16c5384
Compare
Tested by adding "GRANTED" in the mapping value. GA4 will ignore the all caps value, and the consent will remain unchanged. So the existing destinations that have the value in anything other that all lower caps will have to update it. |
Do we need to do any customer communication here? CC: @smultani |
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.
LGTM 🎉
This PR adds dropdown for the
Ads Storage Consent State
andAnalytics Storage Consent State
mapping, to keep it consistent with other privacy mappings, and also to send the privacy setting (denied, granted) in all lower case as that is required by google.It also converts the privacy mapping value to lowercase to keep it compatible for users who were using GRANTED in all caps.
JIRA -> STRATCONN-4266
Testing
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.
Testing successfully completed in staging via Tag Assistant.
Stage Testing Document