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

Send a path, not full url, to trackers for pageviews #451

Merged
merged 1 commit into from
Feb 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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