Average Scroll Depth Metric: Track custom props on pageleaves (#4964)

* improve tracker test util to handle custom props

* track pageleave props

* make test util more powerful

* also test with pageview-props extension

* add test for sending props on hash navigation

* simplify tracker logic around currentPageLeaveURL

* clarify the intent of a timeout by extracting a fn

* bugfix: stop sending pageleaves from ignored pages

+ fix the test to actually fail if this doesn't work.

* refactor: drop expectCustomEvent in favour of the new test util

* add test for pageleave props ingestion

* add query tests

* improve test util readability

test/plausible_web/controllers/api/external_controller_test.exs

@@ -649,6 +649,29 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       assert Map.get(event, :"meta.value") == ["true", "12"]
     end
 
+    test "records custom props for a pageleave event", %{conn: conn, site: site} do
+      post(conn, "/api/event", %{
+        n: "pageview",
+        u: "https://ab.cd",
+        d: site.domain
+      })
+
+      post(conn, "/api/event", %{
+        name: "pageleave",
+        url: "http://ab.cd/",
+        domain: site.domain,
+        props: %{
+          bool_test: true,
+          number_test: 12
+        }
+      })
+
+      pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))
+
+      assert Map.get(pageleave, :"meta.key") == ["bool_test", "number_test"]
+      assert Map.get(pageleave, :"meta.value") == ["true", "12"]
+    end
+
     test "filters out bad props", %{conn: conn, site: site} do
       params = %{
         name: "Signup",

test/plausible_web/controllers/api/external_stats_controller/query_test.exs

@@ -3759,6 +3759,40 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do
              ]
     end
 
+    test "can query scroll_depth with page + custom prop filter", %{conn: conn, site: site} do
+      populate_stats(site, [
+        build(:pageview, user_id: 123, timestamp: ~N[2021-01-01 00:00:00]),
+        build(:pageleave, user_id: 123, timestamp: ~N[2021-01-01 00:00:10], scroll_depth: 40),
+        build(:pageview,
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:00:10]
+        ),
+        build(:pageleave,
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:00:20],
+          scroll_depth: 60
+        ),
+        build(:pageview, user_id: 456, timestamp: ~N[2021-01-01 00:00:00]),
+        build(:pageleave, user_id: 456, timestamp: ~N[2021-01-01 00:00:10], scroll_depth: 80)
+      ])
+
+      conn =
+        post(conn, "/api/v2/query-internal-test", %{
+          "site_id" => site.domain,
+          "filters" => [["is", "event:page", ["/"]], ["is", "event:props:author", ["john"]]],
+          "date_range" => "all",
+          "metrics" => ["scroll_depth"]
+        })
+
+      assert json_response(conn, 200)["results"] == [
+               %{"metrics" => [60], "dimensions" => []}
+             ]
+    end
+
     test "scroll depth is 0 when no pageleave data in range", %{conn: conn, site: site} do
       populate_stats(site, [
         build(:pageview, timestamp: ~N[2021-01-01 00:00:00])
@@ -4048,6 +4082,78 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do
                %{"dimensions" => ["/blog", "2020-01-02"], "metrics" => [20]}
              ]
     end
+
+    test "breakdown by a custom prop with a page filter", %{conn: conn, site: site} do
+      populate_stats(site, [
+        build(:pageview, user_id: 123, timestamp: ~N[2021-01-01 00:00:00], pathname: "/blog"),
+        build(:pageleave,
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:01:00],
+          pathname: "/blog",
+          scroll_depth: 91
+        ),
+        build(:pageview,
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:02:00],
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          pathname: "/blog/john-post"
+        ),
+        build(:pageleave,
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:03:00],
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          pathname: "/blog/john-post",
+          scroll_depth: 40
+        ),
+        build(:pageview,
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:02:00],
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          pathname: "/another-blog/john-post"
+        ),
+        build(:pageleave,
+          user_id: 123,
+          timestamp: ~N[2021-01-01 00:03:00],
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          pathname: "/another-blog/john-post",
+          scroll_depth: 90
+        ),
+        build(:pageview,
+          user_id: 456,
+          timestamp: ~N[2021-01-01 00:02:00],
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          pathname: "/blog/john-post"
+        ),
+        build(:pageleave,
+          user_id: 456,
+          timestamp: ~N[2021-01-01 00:03:00],
+          "meta.key": ["author"],
+          "meta.value": ["john"],
+          pathname: "/blog/john-post",
+          scroll_depth: 46
+        )
+      ])
+
+      conn =
+        post(conn, "/api/v2/query-internal-test", %{
+          "site_id" => site.domain,
+          "metrics" => ["scroll_depth"],
+          "order_by" => [["scroll_depth", "desc"]],
+          "date_range" => "all",
+          "filters" => [["matches", "event:page", ["/blog.*"]]],
+          "dimensions" => ["event:props:author"]
+        })
+
+      assert json_response(conn, 200)["results"] == [
+               %{"dimensions" => ["(none)"], "metrics" => [91]},
+               %{"dimensions" => ["john"], "metrics" => [43]}
+             ]
+    end
   end
 
   test "can filter by utm_medium case insensitively", %{conn: conn, site: site} do

tracker/.eslintrc.json

@@ -19,8 +19,7 @@
       {
         "assertFunctionNames": [
           "expect",
-          "clickPageElementAndExpectEventRequests",
-          "expectCustomEvent"
+          "expectPlausibleInAction"
         ]
       }
     ]

tracker/src/customEvents.js

@@ -193,6 +193,8 @@ function handleTaggedElementClickEvent(event) {
     var eventAttrs = getTaggedEventAttributes(taggedElement)
 
     if (clickedLink) {
+      // if the clicked tagged element is a link, we attach the `url` property
+      // automatically for user convenience
       eventAttrs.props.url = clickedLink.href
       sendLinkClickEvent(event, clickedLink, eventAttrs)
     } else {

tracker/src/plausible.js

@@ -12,9 +12,15 @@
   var endpoint = scriptEl.getAttribute('data-api') || defaultEndpoint(scriptEl)
   var dataDomain = scriptEl.getAttribute('data-domain')
 
-  function onIgnoredEvent(reason, options) {
+  function onIgnoredEvent(eventName, reason, options) {
     if (reason) console.warn('Ignoring Event: ' + reason);
     options && options.callback && options.callback()
+
+    {{#if pageleave}}
+    if (eventName === 'pageview') {
+      currentPageLeaveIgnored = true
+    }
+    {{/if}}
   }
 
   function defaultEndpoint(el) {
@@ -31,10 +37,9 @@
   {{#if pageleave}}
   // :NOTE: Tracking pageleave events is currently experimental.
 
-  // Keeps track of the URL to be sent in the pageleave event payload.
-  // Should get updated on pageviews triggered manually with a custom
-  // URL, and on SPA navigation.
+  var currentPageLeaveIgnored
   var currentPageLeaveURL = location.href
+  var currentPageLeaveProps = {}
 
   // Multiple pageviews might be sent by the same script when the page
   // uses client-side routing (e.g. hash or history-based). This flag
@@ -92,7 +97,7 @@
   })
 
   function triggerPageLeave() {
-    if (pageLeaveSending) {return}
+    if (pageLeaveSending || currentPageLeaveIgnored) {return}
     pageLeaveSending = true
     setTimeout(function () {pageLeaveSending = false}, 500)
 
@@ -101,6 +106,7 @@
       sd: Math.round((maxScrollDepthPx / currentDocumentHeight) * 100),
       d: dataDomain,
       u: currentPageLeaveURL,
+      p: currentPageLeaveProps
     }
 
     {{#if hash}}
@@ -126,15 +132,15 @@
 
     {{#unless local}}
     if (/^localhost$|^127(\.[0-9]+){0,2}\.[0-9]+$|^\[::1?\]$/.test(location.hostname) || location.protocol === 'file:') {
-      return onIgnoredEvent('localhost', options)
+      return onIgnoredEvent(eventName, 'localhost', options)
     }
     if ((window._phantom || window.__nightmare || window.navigator.webdriver || window.Cypress) && !window.__plausible) {
-      return onIgnoredEvent(null, options)
+      return onIgnoredEvent(eventName, null, options)
     }
     {{/unless}}
     try {
       if (window.localStorage.plausible_ignore === 'true') {
-        return onIgnoredEvent('localStorage flag', options)
+        return onIgnoredEvent(eventName, 'localStorage flag', options)
       }
     } catch (e) {
 
@@ -147,7 +153,7 @@
       var isIncluded = !dataIncludeAttr || (dataIncludeAttr && dataIncludeAttr.split(',').some(pathMatches))
       var isExcluded = dataExcludeAttr && dataExcludeAttr.split(',').some(pathMatches)
 
-      if (!isIncluded || isExcluded) return onIgnoredEvent('exclusion rule', options)
+      if (!isIncluded || isExcluded) return onIgnoredEvent(eventName, 'exclusion rule', options)
     }
 
     function pathMatches(wildcardPath) {
@@ -167,10 +173,6 @@
     {{#if manual}}
     var customURL = options && options.u
 
-    {{#if pageleave}}
-    isPageview && customURL && (currentPageLeaveURL = customURL)
-    {{/if}}
-
     payload.u = customURL ? customURL : location.href
     {{else}}
     payload.u = location.href
@@ -220,6 +222,9 @@
       if (request.readyState === 4) {
         {{#if pageleave}}
         if (isPageview) {
+          currentPageLeaveIgnored = false
+          currentPageLeaveURL = payload.u
+          currentPageLeaveProps = payload.p
           registerPageLeaveListener()
         }
         {{/if}}
@@ -245,7 +250,6 @@
       {{#if pageleave}}
       if (isSPANavigation && listeningPageLeave) {
         triggerPageLeave();
-        currentPageLeaveURL = location.href;
         currentDocumentHeight = getDocumentHeight()
         maxScrollDepthPx = getCurrentScrollDepthPx()
       }

tracker/test/custom-event-edge-cases.spec.js

@@ -1,38 +1,36 @@
-const { mockRequest, mockManyRequests, expectCustomEvent } = require('./support/test-utils')
-const { expect, test } = require('@playwright/test')
+const { expectPlausibleInAction } = require('./support/test-utils')
+const { test } = require('@playwright/test')
 const { LOCAL_SERVER_ADDR } = require('./support/server')
 
 test.describe('script.file-downloads.outbound-links.tagged-events.js', () => {
     test('sends only outbound link event when clicked link is both download and outbound', async ({ page }) => {
         await page.goto('/custom-event-edge-case.html')
         const downloadURL = await page.locator('#outbound-download-link').getAttribute('href')
 
-        const plausibleRequestMockList = mockManyRequests(page, '/api/event', 2)
-        await page.click('#outbound-download-link')
-
-        const requests = await plausibleRequestMockList
-        expect(requests.length).toBe(1)
-        expectCustomEvent(requests[0], 'Outbound Link: Click', {url: downloadURL})
+        await expectPlausibleInAction(page, {
+            action: () => page.click('#outbound-download-link'),
+            expectedRequests: [{n: 'Outbound Link: Click', p: {url: downloadURL}}]
+        })
     })
 
     test('sends file download event when local download link clicked', async ({ page }) => {
         await page.goto('/custom-event-edge-case.html')
         const downloadURL = LOCAL_SERVER_ADDR + '/' + await page.locator('#local-download').getAttribute('href')
 
-        const plausibleRequestMock = mockRequest(page, '/api/event')
-        await page.click('#local-download')
-
-        expectCustomEvent(await plausibleRequestMock, 'File Download', {url: downloadURL})
+        await expectPlausibleInAction(page, {
+            action: () => page.click('#local-download'),
+            expectedRequests: [{n: 'File Download', p: {url: downloadURL}}]
+        })
     })
 
     test('sends only tagged event when clicked link is tagged + outbound + download', async ({ page }) => {
         await page.goto('/custom-event-edge-case.html')
 
-        const plausibleRequestMockList = mockManyRequests(page, '/api/event', 3)
-        await page.click('#tagged-outbound-download-link')
-
-        const requests = await plausibleRequestMockList
-        expect(requests.length).toBe(1)
-        expectCustomEvent(requests[0], 'Foo', {})
+        await expectPlausibleInAction(page, {
+            action: () => page.click('#tagged-outbound-download-link'),
+            expectedRequests: [{n: 'Foo', p: {url: 'https://awesome.website.com/file.pdf'}}],
+            awaitedRequestCount: 3,
+            expectedRequestCount: 1
+        })
     })
 })

tracker/test/file-downloads.spec.js

@@ -1,4 +1,4 @@
-const { mockRequest, expectCustomEvent, mockManyRequests, metaKey } = require('./support/test-utils')
+const { mockRequest, mockManyRequests, metaKey, expectPlausibleInAction } = require('./support/test-utils')
 const { expect, test } = require('@playwright/test')
 const { LOCAL_SERVER_ADDR } = require('./support/server')
 
@@ -7,35 +7,38 @@ test.describe('file-downloads extension', () => {
     await page.goto('/file-download.html')
     const downloadURL = await page.locator('#link').getAttribute('href')
 
-    const plausibleRequestMock = mockRequest(page, '/api/event')
     const downloadRequestMock = mockRequest(page, downloadURL)
-    await page.click('#link', { modifiers: [metaKey()] })
 
-    expectCustomEvent(await plausibleRequestMock, 'File Download', { url: downloadURL })
+    await expectPlausibleInAction(page, {
+      action: () => page.click('#link', { modifiers: [metaKey()] }),
+      expectedRequests: [{n: 'File Download', p: { url: downloadURL }}]
+    })
+
     expect(await downloadRequestMock, "should not make download request").toBeNull()
   })
 
   test('sends event and starts download when link child is clicked', async ({ page }) => {
     await page.goto('/file-download.html')
     const downloadURL = await page.locator('#link').getAttribute('href')
 
-    const plausibleRequestMock = mockRequest(page, '/api/event')
     const downloadRequestMock = mockRequest(page, downloadURL)
-    await page.click('#link-child')
 
-    expectCustomEvent(await plausibleRequestMock, 'File Download', { url: downloadURL })
+    await expectPlausibleInAction(page, {
+      action: () => page.click('#link-child'),
+      expectedRequests: [{n: 'File Download', p: { url: downloadURL }}]
+    })
+
     expect((await downloadRequestMock).url()).toContain(downloadURL)
   })
 
   test('sends File Download event with query-stripped url property', async ({ page }) => {
     await page.goto('/file-download.html')
     const downloadURL = await page.locator('#link-query').getAttribute('href')
 
-    const plausibleRequestMock = mockRequest(page, '/api/event')
-    await page.click('#link-query')
-
-    const expectedURL = downloadURL.split("?")[0]
-    expectCustomEvent(await plausibleRequestMock, 'File Download', { url: expectedURL })
+    await expectPlausibleInAction(page, {
+      action: () => page.click('#link-query'),
+      expectedRequests: [{n: 'File Download', p: { url: downloadURL.split("?")[0] }}]
+    })
   })
 
   test('starts download only once', async ({ page }) => {