Update cross domain analytics code and extend tests #1832
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andysellick
force-pushed
the
improve-cross-domain-analytics-code
branch
from
488762b
to
b984573
Compare
andysellick
force-pushed
the
improve-cross-domain-analytics-code
branch
from
b984573
to
453efb9
Compare
chao-xian
approved these changes
Sep 2, 2019
spec/javascripts/analytics_toolkit/google-analytics-universal-tracker.spec.js
Show resolved
Hide resolved
- fairly sure that calls to GA for linked domains should pass domains as an array of strings - addLinkedTrackerDomain was accepting domain (an array of one or more strings) and wrapping it in another array - working assumption is because that was assumed to be a string, when GA requires an array of strings - tests extended to include tests for when more than one linked domain is needed - some unexplained behaviour: passing one domain results in a call containing that string in an array, passing more than one results in the strings inside an array inside another array, this does not look right but currently unable to figure it out - looks like tests are not calling the addLinkedTrackerDomain code in app/assets/javascripts/analytics_toolkit/, or at least log statements briefly added to that code are not triggered by the tests, also unexplained behaviour
andysellick
force-pushed
the
improve-cross-domain-analytics-code
branch
from
453efb9
to
859d998
Compare
- update removes duplicate analytics code, should allow tests to pass
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Still trying to figure out how all this analytics code works by adding new tests.
This PR has required an updated version of
govuk_frontend_toolkit. The toolkit is deprecated, and analytics code from it was recently copied into this application, but it turns out that the code in toolkit was still executing, overriding any changes made here. The updated version of toolkit removes all of the analytics code.Trello card: https://trello.com/c/ZRP8jWmd/75-understand-fix-ga-cross-domain-tracking-code-in-static