Skip to content
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

Change linker code calling approach #1829

Merged
merged 1 commit into from
Aug 30, 2019
Merged

Change linker code calling approach #1829

merged 1 commit into from
Aug 30, 2019

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Aug 30, 2019

Seems we might have been calling the linker code incorrectly. Rather than calling it once for each linked domain required, we should call it once but with all the domains.

The previous method was causing duplicate GA events to be fired, one for each call to the linker code.

This PR also adds a test for calling the code this way.

- have added a test, which appears to produce the result we want
- now calling the linker code once, but with multiple domains
@andysellick andysellick changed the title [DO NOT MERGE] Testing the linked tracker code Change linker code calling approach Aug 30, 2019
@@ -40,7 +40,7 @@

// Make interface public for virtual pageviews and events
GOVUK.analytics = analytics;
GOVUK.analytics.addLinkedTrackerDomain(secondaryId, 'govuk', 'design-system.service.gov.uk');
GOVUK.analytics.addLinkedTrackerDomain(secondaryId, 'govuk', 'design-system.service.gov.uk, passport.service.gov.uk, apply-for-eu-settled-status.homeoffice.gov.uk');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
GOVUK.analytics.addLinkedTrackerDomain(secondaryId, 'govuk', 'design-system.service.gov.uk, passport.service.gov.uk, apply-for-eu-settled-status.homeoffice.gov.uk');
GOVUK.analytics.addLinkedTrackerDomain(secondaryId, 'govuk', 'design-system.service.gov.uk', 'passport.service.gov.uk', 'apply-for-eu-settled-status.homeoffice.gov.uk');

Is it missing quotes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe not, no. I think it's supposed to be like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan is to have a revert ready. Unfortunately the only way to test this is working and to be sure of it is to deploy it to live :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants