Skip to content

Commit

Permalink
feat(aio): should not send in-page navigations to Google Analytics
Browse files Browse the repository at this point in the history
closes angular#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.
  • Loading branch information
wardbell committed May 5, 2017
1 parent 1092292 commit 31d73ed
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 6 deletions.
4 changes: 3 additions & 1 deletion aio/src/app/shared/ga.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
4 changes: 1 addition & 3 deletions aio/src/app/shared/ga.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions aio/src/app/shared/location.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
6 changes: 4 additions & 2 deletions aio/src/app/shared/location.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@ 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,
private location: Location,
private platformLocation: PlatformLocation) {

this.currentUrl.connect();
this.currentPath.connect();
this.urlSubject.next(location.path(true));

this.location.subscribe(state => {
Expand Down

0 comments on commit 31d73ed

Please sign in to comment.