From 3b13300f625cd3f2fcc5e1cac00053c483008e59 Mon Sep 17 00:00:00 2001 From: Ward Bell Date: Sat, 29 Apr 2017 19:04:02 -0700 Subject: [PATCH] =?UTF-8?q?test(aio):=20confirm=20ga=20tracks=20user?= =?UTF-8?q?=E2=80=99s=20#=20fragment=20clicks=20within=20a=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was the behavior and we think we still want to do it. Added comment and test to confirm that is our present intention. --- aio/src/app/shared/ga.service.spec.ts | 61 ++++++++++++++++++--------- aio/src/app/shared/ga.service.ts | 5 ++- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/aio/src/app/shared/ga.service.spec.ts b/aio/src/app/shared/ga.service.spec.ts index c654221316b9c..ca51a617f9155 100644 --- a/aio/src/app/shared/ga.service.spec.ts +++ b/aio/src/app/shared/ga.service.spec.ts @@ -8,6 +8,21 @@ describe('GaService', () => { let gaSpy: jasmine.Spy; let injector: ReflectiveInjector; + // filter for 'send' which communicates with server + // returns the url of the 'send pageview' + function gaSpySendCalls() { + let lastUrl: string; + return gaSpy.calls.all() + .reduce((acc, c) => { + const args = c.args; + if (args[0] === 'set') { + lastUrl = args[2]; + } else if (args[0] === 'send') { + acc.push(lastUrl); + } + return acc; + }, []); + } beforeEach(() => { injector = ReflectiveInjector.resolveAndCreate([ @@ -40,6 +55,16 @@ describe('GaService', () => { expect(first[0]).toBe('create'); }); + describe('#locationChanged(url)', () => { + it('should send page to url w/ leading slash', () => { + gaService.locationChanged('testUrl'); + let args = gaSpy.calls.all()[1].args; + expect(args).toEqual(['set', 'page', '/testUrl']); + args = gaSpy.calls.all()[2].args; + expect(args).toEqual(['send', 'pageview']); + }); + }); + describe('#sendPage(url)', () => { it('should set page to url w/ leading slash', () => { gaService.sendPage('testUrl'); @@ -55,36 +80,32 @@ describe('GaService', () => { it('should not send twice with same URL, back-to-back', () => { gaService.sendPage('testUrl'); - const count1 = gaSpy.calls.count(); - gaService.sendPage('testUrl'); - const count2 = gaSpy.calls.count(); - expect(count2).toEqual(count1); + expect(gaSpySendCalls()).toEqual(['/testUrl']); + }); + + it('should send twice with same URL, back-to-back, when the hash changes', () => { + gaService.sendPage('testUrl#one'); + gaService.sendPage('testUrl#two'); + expect(gaSpySendCalls()).toEqual([ + '/testUrl#one', + '/testUrl#two' + ]); + }); it('should send same URL twice when other intervening URL', () => { gaService.sendPage('testUrl'); - const count1 = gaSpy.calls.count(); - gaService.sendPage('testUrl2'); - const count2 = gaSpy.calls.count(); - expect(count2).toBeGreaterThan(count1, 'testUrl2 was sent'); - gaService.sendPage('testUrl'); - const count3 = gaSpy.calls.count(); - expect(count3).toBeGreaterThan(count1, 'testUrl was sent 2nd time'); + expect(gaSpySendCalls()).toEqual([ + '/testUrl', + '/testUrl2', + '/testUrl' + ]); }); }); - describe('#locationChanged(url)', () => { - it('should send page to url w/ leading slash', () => { - gaService.locationChanged('testUrl'); - let args = gaSpy.calls.all()[1].args; - expect(args).toEqual(['set', 'page', '/testUrl']); - args = gaSpy.calls.all()[2].args; - expect(args).toEqual(['send', 'pageview']); - }); - }); }); describe('when no ambient GA', () => { diff --git a/aio/src/app/shared/ga.service.ts b/aio/src/app/shared/ga.service.ts index 6296614de054f..cef1eeae8b1f7 100644 --- a/aio/src/app/shared/ga.service.ts +++ b/aio/src/app/shared/ga.service.ts @@ -5,7 +5,7 @@ import { Logger } from 'app/shared/logger.service'; @Injectable() /** - * Google Analytics Service - captures app behaviours and sends them to Google Analytics (GA). + * Google Analytics Service - captures app behaviors and sends them to Google Analytics (GA). * Presupposes that GA script has been loaded from a script on the host web page. * Associates data with a GA "property" from the environment (`gaId`). */ @@ -27,6 +27,9 @@ export class GaService { } sendPage(url: string) { + // Won't re-send if the url (including fragment) hasn't changed. + // TODO: Perhaps we don't want to track clicks on in-page links. + // Could easily report only when the page (base url) changes. if (url === this.previousUrl) { return; } this.previousUrl = url; this.ga('set', 'page', '/' + url);