From 83b771234cfa558413c820ce69d05d61bd579661 Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 2 Nov 2017 12:34:54 +0000 Subject: [PATCH 1/6] Remove ecommerce spec from static-analytics-spec.js The one test in static-analytics-spec.js does not cover behaviour that is not already covered by the specs in ecommerce.spec.js so there's no reason to keep it in here. --- .../analytics/static-analytics-spec.js | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/spec/javascripts/analytics/static-analytics-spec.js b/spec/javascripts/analytics/static-analytics-spec.js index 49e4fdbcd..a511e7668 100644 --- a/spec/javascripts/analytics/static-analytics-spec.js +++ b/spec/javascripts/analytics/static-analytics-spec.js @@ -46,33 +46,6 @@ describe("GOVUK.StaticAnalytics", function() { expect(GOVUK.analyticsPlugins.error).toHaveBeenCalled(); }); - describe('when ecommerce results are present', function() { - beforeEach(function() { - window.ga.calls.reset(); - }); - - afterEach(function() { - $('.test-fixture').remove(); - }); - - it('sends the ecommerce fields', function() { - $('body').append('\ -
\ -
\ -
\ -
\ -
\ - ') - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(window.ga).toHaveBeenCalledWith('send', 'pageview', pageViewObject); - }); - }); - describe('when there are govuk: meta tags', function() { beforeEach(function() { window.ga.calls.reset(); From 27ff71b1bc81713e7b16661fd9a56dfe972236df Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 2 Nov 2017 12:45:31 +0000 Subject: [PATCH 2/6] Set the language custom dimension in customDimensionsFromDom It used to be done in customDimensionsFromMetaTags, but the value is not coming from a meta tag so that's not really where it belongs. We also do a similar reorganisation of the tests and create explicit tests for setting the custom dimension when the dom element is or isn't present. --- .../analytics/custom-dimensions.js | 5 ++-- .../analytics/static-analytics-spec.js | 30 +++++++++++++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/analytics/custom-dimensions.js b/app/assets/javascripts/analytics/custom-dimensions.js index d1c16c7da..866bcc5de 100644 --- a/app/assets/javascripts/analytics/custom-dimensions.js +++ b/app/assets/javascripts/analytics/custom-dimensions.js @@ -82,15 +82,14 @@ } }); - customDimensions['dimension23'] = $('main[id="content"]').attr('lang') || 'unknown'; - return customDimensions; } function customDimensionsFromDom() { return { dimension26: GOVUK.PageContent.getNumberOfSections(), - dimension27: GOVUK.PageContent.getNumberOfLinks() + dimension27: GOVUK.PageContent.getNumberOfLinks(), + dimension23: $('main[id="content"]').attr('lang') || 'unknown' }; } diff --git a/spec/javascripts/analytics/static-analytics-spec.js b/spec/javascripts/analytics/static-analytics-spec.js index a511e7668..ff8f4e038 100644 --- a/spec/javascripts/analytics/static-analytics-spec.js +++ b/spec/javascripts/analytics/static-analytics-spec.js @@ -56,11 +56,6 @@ describe("GOVUK.StaticAnalytics", function() { }); it('sets them as dimensions', function() { - $('body').append('\ -
\ -
\ -
\ - '); $('head').append('\ \ \ @@ -85,7 +80,6 @@ describe("GOVUK.StaticAnalytics", function() { expect(pageViewObject.dimension10).toEqual(''); expect(pageViewObject.dimension12).toEqual('withdrawn'); expect(pageViewObject.dimension17).toEqual('schema-name'); - expect(pageViewObject.dimension23).toEqual('fr'); expect(pageViewObject.dimension30).toEqual('education') }); @@ -889,6 +883,30 @@ describe("GOVUK.StaticAnalytics", function() { expect(pageViewObject.dimension27).toEqual('3'); }); }); + + it('sets the page language from the main element as a custom dimension', function () { + $('.test-fixture').remove(); + $('body').append('\ +
\ +
\ +
\ + '); + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + expect(pageViewObject.dimension23).toEqual('fr'); + }); + + it('sets the page language as "unknown" if the main element has no lang attribute as a custom dimension', function () { + $('.test-fixture').remove(); + $('body').append('\ +
\ +
\ +
\ + '); + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + expect(pageViewObject.dimension23).toEqual('unknown'); + }); }); }); From 219563523ef1ec1d17fac5e91f0f945909c0a324 Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 2 Nov 2017 12:49:12 +0000 Subject: [PATCH 3/6] Reorganise specs for custom dimensions based on the dom These lived in the "when govuk: meta tags are present" describe block, but the presence of meta tags has no bearing on the outcome of these specs so we move them out of that describe block into their own one. --- .../analytics/static-analytics-spec.js | 440 +++++++++--------- 1 file changed, 220 insertions(+), 220 deletions(-) diff --git a/spec/javascripts/analytics/static-analytics-spec.js b/spec/javascripts/analytics/static-analytics-spec.js index ff8f4e038..236588061 100644 --- a/spec/javascripts/analytics/static-analytics-spec.js +++ b/spec/javascripts/analytics/static-analytics-spec.js @@ -189,271 +189,271 @@ describe("GOVUK.StaticAnalytics", function() { expect(pageViewObject['dimension' + dimension.number]).toEqual(dimension.defaultValue); }); }); + }); - describe('when tracking the number of sections and links on a page', function() { - describe('on a page with a normal sidebar', function() { - beforeEach(function() { - $('head').append('\ -
\ - \ - \ -
\ - ') - $('body').append('\ -
\ -
\ + \ +
\ + '); }); - describe('on a page with a taxon sidebar', function() { - beforeEach(function() { - $('head').append('\ -
\ - \ - \ -
\ - ') - $('body').append('\ -
\ -
\ -
\ -

Section 2

\ - \ + \ +
\ \ - \ + \ \ - '); - }); - - afterEach(function() { - $('.test-meta-tags').remove(); - $('.test-fixture').remove(); - }); - - it('tracks the number of sidebar sections', function() { - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(pageViewObject.dimension26).toEqual('2'); - }); - - it('tracks the total number of related links, including headers', function() { - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(pageViewObject.dimension27).toEqual('5'); - }); + \ + '); }); - describe('on a page with an accordion', function() { - beforeEach(function() { - $('head').append('\ -
\ - \ - \ - \ -
\ - ') - $('body').append('\ -
\ -
\ -
\ -
\ -
\ -

Section 1

\ -
\ -
\ -
    \ -
  1. \ - \ - Link 1.1\ - \ -
    2. \ -
  2. \ - \ - Link 1.2\ - \ -
    3. \ -
\ -
\ -
\ -
\ -
\ -

Section 2

\ -
\ -
\ -
    \ -
  1. \ - \ - Link 2.1\ - \ -
    2. \ -
\ -
\ + afterEach(function() { + $('.test-meta-tags').remove(); + $('.test-fixture').remove(); + }); + + it('tracks the number of accordion sections', function() { + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + expect(pageViewObject.dimension26).toEqual('2'); + }); + + it('tracks the total number of accordion section links', function() { + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + expect(pageViewObject.dimension27).toEqual('3'); + }); + }); + + describe('on a page with a grid', function() { + beforeEach(function() { + $('head').append('\ +
\ + \ + \ + \ +
\ + ') + $('body').append('\ +
\ +
\ + \ +
\ +
\ +
\ +

Grid leaves

\ +
    \ +

  1. \ + \ + Leaf 1\ + \ +

    2. \ + \

  2. \ + \ + Leaf 2\ + \ +

    3. \ +
\
\
\
\ -
\ - '); - }); - - afterEach(function() { - $('.test-meta-tags').remove(); - $('.test-fixture').remove(); - }); - - it('tracks the number of accordion sections', function() { - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(pageViewObject.dimension26).toEqual('2'); - }); - - it('tracks the total number of accordion section links', function() { - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(pageViewObject.dimension27).toEqual('3'); - }); + \ +
\ + '); }); - describe('on a page with a grid', function() { - beforeEach(function() { - $('head').append('\ -
\ - \ - \ - \ -
\ - ') - $('body').append('\ -
\ -
\ - \ -
\ -
\ -
\ -

Grid leaves

\ -
    \ -

  1. \ - \ - Leaf 1\ - \ -

    2. \ - \

  2. \ - \ - Leaf 2\ - \ -

    3. \ -
\ -
\ -
\ -
\ -
\ -
\ - '); - }); - - afterEach(function() { - $('.test-meta-tags').remove(); - $('.test-fixture').remove(); - }); - - it('tracks the number of sections', function() { - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(pageViewObject.dimension26).toEqual('2'); - }); - - it('tracks the total number of grid links and leaf links', function() { - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - expect(pageViewObject.dimension27).toEqual('5'); - }); + afterEach(function() { + $('.test-meta-tags').remove(); + $('.test-fixture').remove(); + }); + + it('tracks the number of sections', function() { + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + expect(pageViewObject.dimension26).toEqual('2'); + }); + + it('tracks the total number of grid links and leaf links', function() { + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + expect(pageViewObject.dimension27).toEqual('5'); }); }); From 0b5f5e22e192caec2526cf8cd9e7dc6ae37707ad Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 2 Nov 2017 12:52:55 +0000 Subject: [PATCH 4/6] Make meta tag custom dimension specs more explicit We already had explicit tests for the custom dimensions that had default values, so we add explicit tests for the ones that don't have default values too. We also add a couple of missing default value tests. This allows us to remove one of the tests that relies on the length of the keys in the `pageViewObject` which is fairly brittle. --- .../analytics/static-analytics-spec.js | 110 +++++++++++------- 1 file changed, 69 insertions(+), 41 deletions(-) diff --git a/spec/javascripts/analytics/static-analytics-spec.js b/spec/javascripts/analytics/static-analytics-spec.js index 236588061..bbbbf328c 100644 --- a/spec/javascripts/analytics/static-analytics-spec.js +++ b/spec/javascripts/analytics/static-analytics-spec.js @@ -55,44 +55,6 @@ describe("GOVUK.StaticAnalytics", function() { $('head').find('meta[name^="govuk:"]').remove(); }); - it('sets them as dimensions', function() { - $('head').append('\ - \ - \ - \ - \ - \ - \ - \ - \ - \ - \ - '); - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - - expect(pageViewObject.dimension1).toEqual('section'); - expect(pageViewObject.dimension2).toEqual('format'); - expect(pageViewObject.dimension5).toEqual('1000'); - expect(pageViewObject.dimension6).toEqual('2005-to-2010-labour-government'); - expect(pageViewObject.dimension7).toEqual('historic'); - expect(pageViewObject.dimension9).toEqual(''); - expect(pageViewObject.dimension10).toEqual(''); - expect(pageViewObject.dimension12).toEqual('withdrawn'); - expect(pageViewObject.dimension17).toEqual('schema-name'); - expect(pageViewObject.dimension30).toEqual('education') - }); - - it('ignores meta tags not set', function() { - $('head').append(''); - - analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); - pageViewObject = getPageViewObject(); - - expect(Object.keys(pageViewObject).length).toEqual(1 + numberOfDimensionsWithDefaultValues); - expect(pageViewObject.dimension1).toEqual('section'); - }); - it('sets A/B meta tags as dimensions', function() { $('head').append('\ \ @@ -106,7 +68,6 @@ describe("GOVUK.StaticAnalytics", function() { expect(pageViewObject.dimension48).toEqual('name-of-other-test:name-of-other-ab-bucket'); }); - it('ignores A/B meta tags with invalid dimensions', function () { $('head').append('\ \ @@ -169,12 +130,22 @@ describe("GOVUK.StaticAnalytics", function() { name: 'navigation-legacy', number: 30, defaultValue: 'none' + }, + { + name: 'withdrawn', + number: 12, + defaultValue: 'not withdrawn' + }, + { + name: 'content-has-history', + number: 39, + defaultValue: 'false' } ].forEach(function (dimension) { it('sets the ' + dimension.name + ' dimension from a meta tag if present', function () { $('head').append('\ - \ - '); + \ + '); analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); pageViewObject = getPageViewObject(); @@ -189,6 +160,63 @@ describe("GOVUK.StaticAnalytics", function() { expect(pageViewObject['dimension' + dimension.number]).toEqual(dimension.defaultValue); }); }); + + [ + { + name: 'section', + number: 1 + }, + { + name: 'format', + number: 2 + }, + { + name: 'search-result-count', + number: 5 + }, + { + name: 'publishing-government', + number: 6 + }, + { + name: 'political-status', + number: 7 + }, + { + name: 'analytics:organisations', + number: 9 + }, + { + name: 'analytics:world-locations', + number: 10 + }, + { + name: 'schema-name', + number: 17 + }, + { + name: 'rendering-application', + number: 20 + } + ].forEach(function (dimension) { + it('sets the ' + dimension.name + ' dimension from a meta tag if present', function () { + $('head').append('\ + \ + '); + + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + + expect(pageViewObject['dimension' + dimension.number]).toEqual('some-' + dimension.name + '-value'); + }); + + it('does not send the dimension if no ' + dimension.name + ' meta tag is present', function () { + analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); + pageViewObject = getPageViewObject(); + + expect(pageViewObject.hasOwnProperty('dimension' + dimension.number)).toEqual(false); + }); + }); }); describe('setting custom dimensions based on elements of the page', function () { From e5168fed785c26d2cb85472f1911e6838c70f06f Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 2 Nov 2017 12:56:18 +0000 Subject: [PATCH 5/6] Make a-b test meta-tag test more robust This test also relied on the length of the keys in the pageViewObject so if that changes this test breaks. We can be more explicit for checking that badly configured a-b test meta tags don't create custom dimensions so we do that instead of relying on counts. --- .../javascripts/analytics/static-analytics-spec.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/analytics/static-analytics-spec.js b/spec/javascripts/analytics/static-analytics-spec.js index bbbbf328c..06c03cafc 100644 --- a/spec/javascripts/analytics/static-analytics-spec.js +++ b/spec/javascripts/analytics/static-analytics-spec.js @@ -71,13 +71,23 @@ describe("GOVUK.StaticAnalytics", function() { it('ignores A/B meta tags with invalid dimensions', function () { $('head').append('\ \ - \ + \ '); analytics = new GOVUK.StaticAnalytics({universalId: 'universal-id'}); pageViewObject = getPageViewObject(); - expect(Object.keys(pageViewObject).length).toEqual(numberOfDimensionsWithDefaultValues); + // 1. check the dimensions haven't been created by using the "value" of data-analytics-dimension + expect(pageViewObject.hasOwnProperty('dimensionnot a number')).toEqual(false); + expect(pageViewObject.hasOwnProperty('dimensionundefined')).toEqual(false); + + // 2. check the values haven't been used at all + var values = []; + for (key in pageViewObject) { + values.push(pageViewObject[key]); + } + expect(values).not.toContain('name-of-test:some-bucket'); + expect(values).not.toContain('name-of-test:some-other-bucket'); }); [ From 68d9921103a767ee814ffa5c86a8ca5ec6fd100d Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 2 Nov 2017 12:59:19 +0000 Subject: [PATCH 6/6] Clarify what getPageViewObject() is doing There's a bunch of magic numbers and assumptions in this method about how many ga() functions have been called, in what order, and for what reason. This is fine, but it's hard to work out so we leave some more explicit comments, and introduce an expectation that will fail if our assumptions are false. --- .../analytics/static-analytics-spec.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/spec/javascripts/analytics/static-analytics-spec.js b/spec/javascripts/analytics/static-analytics-spec.js index 06c03cafc..ac5bb3739 100644 --- a/spec/javascripts/analytics/static-analytics-spec.js +++ b/spec/javascripts/analytics/static-analytics-spec.js @@ -1067,13 +1067,34 @@ describe("GOVUK.StaticAnalytics", function() { // needs to be called manually. We do this below and reload the arguments sent // to window.ga in order to obtain the trackPageView args as well. + // 1. get all arguments to all calls to the ga() function universalSetupArguments = window.ga.calls.allArgs() + // 2. get the ga(function(tracker) { ...}) call - it's the 4th one: + // 1st is the call to create "universal-id" + // 2nd is the call to set "anonymizeIp" + // 3rd is the call to set "displayFeaturesTask" + // 4th is the call that sets the tracker function callback bound = universalSetupArguments[3][0]; + + // 3. trigger the callback with a canned tracker object that has a stubbed + // get method that always retuns the same client id. This is the only + // method we expect to call on the tracker object - calling anything + // else should cause these specs to fail. bound({get: function () { return '12345.67890' }}); + // 4. get all the calls to ga() again as executing the callback will add a + // 5th call. This time it's the one to to send "pageview" which is the + // initial page view tracking we're interested in. universalSetupArguments = window.ga.calls.allArgs(); lastArgumentSet = universalSetupArguments.pop(); + + // 5. make sure this final set of arguments is the one we're looking for + // and fail the test otherwise. + expect(lastArgumentSet[0]).toEqual('send'); + expect(lastArgumentSet[1]).toEqual('pageview'); + + // 6. extract the arguments to that last call and return them pageViewObject = lastArgumentSet[2]; return pageViewObject;