FIX: Avoid double-counting pageviews when navigating with loading spinner

app/assets/javascripts/discourse/app/instance-initializers/page-tracking.js

@@ -65,10 +65,17 @@ export default {
   },
 
   handleRouteWillChange(transition) {
-    // will be null on initial boot transition, which is already tracked as a pageview via the HTML request
-    if (transition.from) {
-      trackNextAjaxAsPageview();
+    // transition.from will be null on initial boot transition, which is already tracked as a pageview via the HTML request
+    if (!transition.from) {
+      return;
     }
+
+    // Ignore intermediate transitions (e.g. loading substates)
+    if (transition.isIntermediate) {
+      return;
+    }
+
+    trackNextAjaxAsPageview();
   },
 
   teardown() {

app/assets/javascripts/discourse/tests/acceptance/page-tracking-test.js

@@ -2,38 +2,124 @@ import { acceptance } from "discourse/tests/helpers/qunit-helpers";
 import { click, visit } from "@ember/test-helpers";
 import { test } from "qunit";
 import pretender from "discourse/tests/helpers/create-pretender";
+import { getOwner } from "@ember/application";
+import { ajax } from "discourse/lib/ajax";
+
+const trackViewHeaderName = "Discourse-Track-View";
+
+function setupPretender(server, helper) {
+  server.get("/fake-analytics-endpoint", (request) => {
+    if (request.requestHeaders[trackViewHeaderName]) {
+      throw "Fake analytics endpoint was called with track-view header";
+    }
+    return helper.response({});
+  });
+}
+
+function setupFakeAnalytics(ref) {
+  getOwner(ref)
+    .lookup("service:router")
+    .on("routeDidChange", () => ajax("/fake-analytics-endpoint"));
+}
+
+function assertRequests({ assert, tracked, untracked, message }) {
+  let trackedCount = 0,
+    untrackedCount = 0;
+
+  const requests = pretender.handledRequests;
+  requests.forEach((request) => {
+    if (request.requestHeaders[trackViewHeaderName]) {
+      trackedCount++;
+    } else {
+      untrackedCount++;
+    }
+  });
+
+  assert.strictEqual(trackedCount, tracked, `${message} (tracked)`);
+  assert.strictEqual(untrackedCount, untracked, `${message} (untracked)`);
+
+  pretender.handledRequests = [];
+}
+
+acceptance("Page tracking - loading slider", function (needs) {
+  needs.user();
+  needs.pretender(setupPretender);
+
+  test("sets the discourse-track-view header correctly", async function (assert) {
+    setupFakeAnalytics(this);
+
+    assertRequests({
+      assert,
+      tracked: 0,
+      untracked: 0,
+      message: "no requests before app boot",
+    });
+
+    await visit("/");
+    assertRequests({
+      assert,
+      tracked: 0,
+      untracked: 2,
+      message: "no ajax tracked for initial page load",
+    });
+
+    await click("#site-logo");
+    assertRequests({
+      assert,
+      tracked: 1,
+      untracked: 1,
+      message: "tracked one pageview for reloading latest",
+    });
+
+    await visit("/t/-/280");
+    assertRequests({
+      assert,
+      tracked: 1,
+      untracked: 1,
+      message: "tracked one pageview for navigating to topic",
+    });
+  });
+});
+
+acceptance("Page tracking - loading spinner", function (needs) {
+  needs.user();
+  needs.pretender(setupPretender);
+  needs.settings({
+    page_loading_indicator: "spinner",
+  });
 
-acceptance("Page tracking", function () {
   test("sets the discourse-track-view header correctly", async function (assert) {
-    const trackViewHeaderName = "Discourse-Track-View";
-    assert.strictEqual(
-      pretender.handledRequests.length,
-      0,
-      "no requests logged before app boot"
-    );
+    setupFakeAnalytics(this);
+
+    assertRequests({
+      assert,
+      tracked: 0,
+      untracked: 0,
+      message: "no requests before app boot",
+    });
 
     await visit("/");
-    assert.strictEqual(
-      pretender.handledRequests.length,
-      1,
-      "one request logged during app boot"
-    );
-    assert.strictEqual(
-      pretender.handledRequests[0].requestHeaders[trackViewHeaderName],
-      undefined,
-      "does not track view for ajax before a transition has taken place"
-    );
+    assertRequests({
+      assert,
+      tracked: 0,
+      untracked: 2,
+      message: "no ajax tracked for initial page load",
+    });
 
     await click("#site-logo");
-    assert.strictEqual(
-      pretender.handledRequests.length,
-      2,
-      "second request logged during next transition"
-    );
-    assert.strictEqual(
-      pretender.handledRequests[1].requestHeaders[trackViewHeaderName],
-      "true",
-      "sends track-view header for subsequent requests"
-    );
+    assertRequests({
+      assert,
+      tracked: 1,
+      untracked: 1,
+      message: "tracked one pageview for reloading latest",
+    });
+
+    await visit("/t/-/280");
+    assertRequests({
+      assert,
+      tracked: 1,
+      untracked: 1,
+      message: "tracked one pageview for navigating to topic",
+    });
   });
 });