-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor how GA shared ID is added to the pages #787
base: 11.x
Are you sure you want to change the base?
Conversation
48386ca
to
0165a30
Compare
0165a30
to
dd56420
Compare
// Only add Google Analytics for anonymous users or users without editing | ||
// access while on Acquia environment. Being able to view the toolbar is a | ||
// good indicator that they can edit some part of the site. |
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.
@pookmish thanks for these changes.
@buttonwillowsix, does this logic match how we are using our GA tracking code?
Would this opt-out most users on the intranet sites?
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.
@buttonwillowsix ping.
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.
Clearly I need to change my github notification settings.
It would need to track authenticated users on intranets. I wonder if we should revisit this later since we may be changing the method used for adding GA to the site.
gentle poke. no rush |
Code works and makes sense to me. Just looking to @buttonwillowsix to confirm intent or business case alignment. |
READY FOR REVIEW
Summary
Setup tasks and/or behavior to test
$config['google_analytics.settings']['account'] = '';
in thegoogle_analytics.settings.php
file/admin/config/system/basic-site-settings