From 3e33482386abc77d822b54cd29585840b050743e Mon Sep 17 00:00:00 2001
From: Ward Bell <wardbell@hotmail.com>
Date: Thu, 4 May 2017 23:51:07 -0700
Subject: [PATCH] feat(aio): should not send in-page navigations to Google
 Analytics

closes #16521
`LocationService` sends `GaService` a url stripped of fragment and query strings.
`GaService` already guards against re-send of the prior url so it will only report doc changes.
---
 aio/src/app/shared/ga.service.spec.ts       |  4 +++-
 aio/src/app/shared/ga.service.ts            |  4 +---
 aio/src/app/shared/location.service.spec.ts | 12 ++++++++++++
 aio/src/app/shared/location.service.ts      |  6 ++++--
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/aio/src/app/shared/ga.service.spec.ts b/aio/src/app/shared/ga.service.spec.ts
index ca51a617f9..5281142ced 100644
--- a/aio/src/app/shared/ga.service.spec.ts
+++ b/aio/src/app/shared/ga.service.spec.ts
@@ -84,7 +84,9 @@ describe('GaService', () => {
         expect(gaSpySendCalls()).toEqual(['/testUrl']);
       });
 
-      it('should send twice with same URL, back-to-back, when the hash changes', () => {
+      it('should send twice with same URL, back-to-back, even when the hash changes', () => {
+        // Therefore it is up to caller NOT to call it when hash changes if this is unwanted.
+        // See LocationService and its specs
         gaService.sendPage('testUrl#one');
         gaService.sendPage('testUrl#two');
         expect(gaSpySendCalls()).toEqual([
diff --git a/aio/src/app/shared/ga.service.ts b/aio/src/app/shared/ga.service.ts
index cef1eeae8b..c9563f9cf8 100644
--- a/aio/src/app/shared/ga.service.ts
+++ b/aio/src/app/shared/ga.service.ts
@@ -27,9 +27,7 @@ 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.
+    // Won't re-send if the url hasn't changed.
     if (url === this.previousUrl) { return; }
     this.previousUrl = url;
     this.ga('set', 'page', '/' + url);
diff --git a/aio/src/app/shared/location.service.spec.ts b/aio/src/app/shared/location.service.spec.ts
index d45e5d0078..c48f7eab63 100644
--- a/aio/src/app/shared/location.service.spec.ts
+++ b/aio/src/app/shared/location.service.spec.ts
@@ -517,6 +517,18 @@ describe('LocationService', () => {
       expect(args[0]).toBe('some-new-url');
     });
 
+    it('should call locationChanged with url stripped of hash or query', () => {
+      // Important to keep GA service from sending tracking event when the doc hasn't changed
+      // e.g., when the user navigates within the page via # fragments.
+      service.go('some-new-url#one');
+      service.go('some-new-url#two');
+      service.go('some-new-url/?foo="true"');
+      expect(gaLocationChanged.calls.count()).toBe(4, 'gaService.locationChanged called');
+      const args = gaLocationChanged.calls.allArgs();
+      expect(args[1]).toEqual(args[2], 'same url for hash calls');
+      expect(args[1]).toEqual(args[3], 'same url for query string call');
+    });
+
     it('should call locationChanged when window history changes', () => {
       location.simulatePopState('/next-url');
 
diff --git a/aio/src/app/shared/location.service.ts b/aio/src/app/shared/location.service.ts
index d1c533a5d2..2b7e755285 100644
--- a/aio/src/app/shared/location.service.ts
+++ b/aio/src/app/shared/location.service.ts
@@ -15,11 +15,12 @@ export class LocationService {
   private urlSubject = new Subject<string>();
   currentUrl = this.urlSubject
     .map(url => this.stripSlashes(url))
-    .do(url => this.gaService.locationChanged(url))
     .publishReplay(1);
 
   currentPath = this.currentUrl
-    .map(url => url.match(/[^?#]*/)[0]); // strip query and hash
+    .map(url => url.match(/[^?#]*/)[0]) // strip query and hash
+    .do(url => this.gaService.locationChanged(url))
+    .publishReplay(1);
 
   constructor(
     private gaService: GaService,
@@ -27,6 +28,7 @@ export class LocationService {
     private platformLocation: PlatformLocation) {
 
     this.currentUrl.connect();
+    this.currentPath.connect();
     this.urlSubject.next(location.path(true));
 
     this.location.subscribe(state => {