Skip to content

Commit

Permalink
Send a path, not full url, to trackers for pageviews
Browse files Browse the repository at this point in the history
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.

Analytics sent without this (e.g. using v 7.3.0 and 7.4.0) are broken
without this change.
  • Loading branch information
h-lame committed Feb 15, 2018
1 parent f3684c5 commit cc35c96
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
10 changes: 5 additions & 5 deletions javascripts/govuk/analytics/analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
36 changes: 32 additions & 4 deletions spec/unit/analytics/analytics.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down

0 comments on commit cc35c96

Please sign in to comment.