From c1ab5de39a27257cea274beaf61dc3e12d14f2ac Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 15 Feb 2018 13:03:59 +0000 Subject: [PATCH] Send a path, not full url, to trackers for pageviews When we track a page view that has no arguments, the default behaviour is for the trackers to work out the path by themselves from the window.location. In order for us to strip PII though we need to build an argument ourselves so we have something to strip PII from, and we then inject that into the arguments we send to the trackers. Prior to this change we were sending a full url, but the GA docs say that this argument should be a path, not a full url. In order to test this (stubbing window.location apears to be painful) we make the `defaultPathForTrackPageview` function take a location object, and we pass `window.location` into it at the call-sites. The tests can now pass an object in and we can test the function properly. These tests are a bit brittle because the location object we use only has the properties we use. Pageview analytics sent without this (e.g. using 7.3.0 and 7.4.0) are broken without this change. --- CHANGELOG.md | 4 +++ javascripts/govuk/analytics/analytics.js | 10 +++---- spec/unit/analytics/analytics.spec.js | 36 +++++++++++++++++++++--- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80fba90d..6d44d4ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased + +- Use a path, not a full url, as the `defaultPathForTrackPageview` because our analytics trackers expect the explicit page param to be a path, not a url ([PR #451](https://github.com/alphagov/govuk_frontend_toolkit/pull/451)). Analytics sent with 7.3.0 and 7.4.0 are broken without this fix. + # 7.4.0 - Allow wrapping arguments to analytics as PII safe to tell the analytics code not to attempt to strip PII from the values: ([PR #448](https://github.com/alphagov/govuk_frontend_toolkit/pull/448)) diff --git a/javascripts/govuk/analytics/analytics.js b/javascripts/govuk/analytics/analytics.js index c405abe2..78eb6176 100644 --- a/javascripts/govuk/analytics/analytics.js +++ b/javascripts/govuk/analytics/analytics.js @@ -93,17 +93,17 @@ GOVUK.GOVUKTracker.load() } - Analytics.prototype.defaultPathForTrackPageview = function () { - // Ignore anchor, but keep query string as per default behaviour of - // GA (see: https://developers.google.com/analytics/devguides/collection/analyticsjs/pages#overview) + Analytics.prototype.defaultPathForTrackPageview = function (location) { + // Get the page path including querystring, but ignoring the anchor + // as per default behaviour of GA (see: https://developers.google.com/analytics/devguides/collection/analyticsjs/pages#overview) // we ignore the possibility of there being campaign variables in the // anchor because we wouldn't know how to detect and parse them if they // were present - return this.stripPIIFromString(window.location.href.split('#')[0]) + return this.stripPIIFromString(location.href.substring(location.origin.length).split('#')[0]) } Analytics.prototype.trackPageview = function (path, title, options) { - arguments[0] = arguments[0] || this.defaultPathForTrackPageview() + arguments[0] = arguments[0] || this.defaultPathForTrackPageview(window.location) if (arguments.length === 0) { arguments.length = 1 } this.sendToTrackers('trackPageview', this.stripPII(arguments)) } diff --git a/spec/unit/analytics/analytics.spec.js b/spec/unit/analytics/analytics.spec.js index efdc1f4f..611cb9c2 100644 --- a/spec/unit/analytics/analytics.spec.js +++ b/spec/unit/analytics/analytics.spec.js @@ -49,25 +49,53 @@ describe('GOVUK.Analytics', function () { }) }) + describe('extracting the default path for a page view', function () { + it('returns a path extracted from the location', function () { + var location = { + href: 'https://govuk-frontend-toolkit.example.com/a/path', + origin: 'https://govuk-frontend-toolkit.example.com' + } + expect(analytics.defaultPathForTrackPageview(location)).toEqual('/a/path') + }) + + it('includes the querystring in the path extracted from the location', function () { + var location = { + href: 'https://govuk-frontend-toolkit.example.com/a/path?with=a&query=string', + origin: 'https://govuk-frontend-toolkit.example.com' + } + expect(analytics.defaultPathForTrackPageview(location)).toEqual('/a/path?with=a&query=string') + }) + + it('removes any anchor from the path extracted from the location', function () { + var location = { + href: 'https://govuk-frontend-toolkit.example.com/a/path#with-an-anchor', + origin: 'https://govuk-frontend-toolkit.example.com' + } + expect(analytics.defaultPathForTrackPageview(location)).toEqual('/a/path') + location.href = 'https://govuk-frontend-toolkit.example.com/a/path?with=a&query=string#with-an-anchor' + expect(analytics.defaultPathForTrackPageview(location)).toEqual('/a/path?with=a&query=string') + }) + }) + describe('tracking pageviews', function () { beforeEach(function () { - spyOn(analytics, 'defaultPathForTrackPageview').and.returnValue('https://govuk-frontend-toolkit.example.com/a/page?with=a&query=string') + spyOn(analytics, 'defaultPathForTrackPageview').and.returnValue('/a/page?with=a&query=string') }) it('injects a default path if no args are supplied', function () { analytics.trackPageview() console.log(window.ga.calls.mostRecent().args) - expect(window.ga.calls.mostRecent().args[2].page).toEqual('https://govuk-frontend-toolkit.example.com/a/page?with=a&query=string') + expect(window.ga.calls.mostRecent().args[2].page).toEqual('/a/page?with=a&query=string') }) it('injects a default path if args are supplied, but the path arg is blank', function () { analytics.trackPageview(null) console.log(window.ga.calls.mostRecent().args) - expect(window.ga.calls.mostRecent().args[2].page).toEqual('https://govuk-frontend-toolkit.example.com/a/page?with=a&query=string') + expect(window.ga.calls.mostRecent().args[2].page).toEqual('/a/page?with=a&query=string') analytics.trackPageview(undefined) console.log(window.ga.calls.mostRecent().args) - expect(window.ga.calls.mostRecent().args[2].page).toEqual('https://govuk-frontend-toolkit.example.com/a/page?with=a&query=string') + expect(window.ga.calls.mostRecent().args[2].page).toEqual('/a/page?with=a&query=string') }) it('uses the supplied path', function () {