Skip to content

Commit

Permalink
Merge pull request #451 from alphagov/send-path-not-url-to-trackers-a…
Browse files Browse the repository at this point in the history
…s-default-page-view

Send a path, not full url, to trackers for pageviews
  • Loading branch information
h-lame committed Feb 15, 2018
2 parents 6ae9bdd + c1ab5de commit 33bba7a
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 33bba7a

Please sign in to comment.