🎨 Improved the performance of the /members/events/ aggregated_click_e…

…vent endpoint (#20790)

Ref https://linear.app/tryghost/issue/ONC-216/improve-the-performance-of-the-membersevents-aggregated-click-event

.github/workflows/ci.yml

@@ -843,6 +843,13 @@ jobs:
         run: |
           ghost update -f -d $V4_DIR --archive $(pwd)/ghost/core/ghost.tgz
 
+      - name: Save Ghost CLI Debug Logs
+        if: failure()
+        uses: actions/upload-artifact@v3
+        with:
+          name: ghost-cli-debug-logs
+          path: /home/runner/.ghost/logs/
+
       - name: Clean Install
         run: |
           DIR=$(mktemp -d)

ghost/core/core/server/models/base/plugins/crud.js

@@ -120,6 +120,20 @@ module.exports = function (Bookshelf) {
                 });
             }
 
+            if (Array.isArray(options.cte)) {
+                options.cte.forEach((cte) => {
+                    itemCollection.query((qb) => {
+                        qb.with(cte.name, qb.client.raw(cte.query));
+                    });
+                });
+            }
+
+            if (options.from) {
+                itemCollection.query((qb) => {
+                    qb.from(options.from);
+                });
+            }
+
             //option param to skip distinct from count query, distinct adds a lot of latency and in this case the result set will always be unique.
             if (unfilteredOptions.useBasicCount) {
                 options.useBasicCount = unfilteredOptions.useBasicCount;

ghost/core/core/server/models/member-click-event.js

@@ -21,7 +21,10 @@ const MemberClickEvent = ghostBookshelf.Model.extend({
         return expansions;
     },
 
-    filterRelations() {
+    filterRelations(options) {
+        if (options && options.filterRelations === false) {
+            return {};
+        }
         return {
             link: {
                 // Mongo-knex doesn't support belongsTo relations
@@ -47,7 +50,7 @@ const MemberClickEvent = ghostBookshelf.Model.extend({
     permittedOptions(methodName) {
         let options = ghostBookshelf.Model.permittedOptions.call(this, methodName);
         const validOptions = {
-            findPage: ['selectRaw', 'whereRaw']
+            findPage: ['selectRaw', 'whereRaw', 'cte', 'from', 'useCTE', 'filterRelations']
         };
 
         if (validOptions[methodName]) {

ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap

@@ -23567,6 +23567,102 @@ Object {
 }
 `;
 
+exports[`Activity Feed API Returns aggregated_click events in activity feed 1: [body] 1`] = `
+Object {
+  "events": Array [
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+  ],
+  "meta": Object {
+    "pagination": Object {
+      "limit": 10,
+      "next": null,
+      "page": null,
+      "pages": 1,
+      "prev": null,
+      "total": 8,
+    },
+  },
+}
+`;
+
+exports[`Activity Feed API Returns aggregated_click events in activity feed 2: [headers] 1`] = `
+Object {
+  "access-control-allow-origin": "http://127.0.0.1:2369",
+  "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
+  "content-length": "2443",
+  "content-type": "application/json; charset=utf-8",
+  "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
+  "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
+  "vary": "Accept-Version, Origin, Accept-Encoding",
+  "x-powered-by": "Express",
+}
+`;
+
+exports[`Activity Feed API Returns aggregated_click events in activity feed with post_id filter 1: [body] 1`] = `
+Object {
+  "events": Array [
+    Object {
+      "data": Any<Object>,
+      "type": "aggregated_click_event",
+    },
+  ],
+  "meta": Object {
+    "pagination": Object {
+      "limit": 10,
+      "next": null,
+      "page": null,
+      "pages": 1,
+      "prev": null,
+      "total": 1,
+    },
+  },
+}
+`;
+
+exports[`Activity Feed API Returns aggregated_click events in activity feed with post_id filter 2: [headers] 1`] = `
+Object {
+  "access-control-allow-origin": "http://127.0.0.1:2369",
+  "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
+  "content-length": "391",
+  "content-type": "application/json; charset=utf-8",
+  "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
+  "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
+  "vary": "Accept-Version, Origin, Accept-Encoding",
+  "x-powered-by": "Express",
+}
+`;
+
 exports[`Activity Feed API Returns click events in activity feed 1: [body] 1`] = `
 Object {
   "events": Array [

ghost/core/test/e2e-api/admin/activity-feed.test.js

@@ -319,6 +319,48 @@ describe('Activity Feed API', function () {
             });
     });
 
+    it('Returns aggregated_click events in activity feed', async function () {
+        // Check activity feed
+        await agent
+            .get(`/members/events?filter=type:aggregated_click_event`)
+            .expectStatus(200)
+            .matchHeaderSnapshot({
+                etag: anyEtag,
+                'content-version': anyContentVersion
+            })
+            .matchBodySnapshot({
+                events: new Array(8).fill({
+                    type: 'aggregated_click_event',
+                    data: anyObject
+                })
+            })
+            .expect(({body}) => {
+                assert(body.events.find(e => e.type === 'aggregated_click_event'), 'Expected a aggregated_click event');
+                assert(!body.events.find(e => e.type !== 'aggregated_click_event'), 'Expected only aggregated_click events');
+            });
+    });
+
+    it('Returns aggregated_click events in activity feed with post_id filter', async function () {
+        const postId = fixtureManager.get('posts', 0).id;
+        await agent
+            .get(`/members/events?filter=type:aggregated_click_event${encodeURIComponent(`+data.post_id:'${postId}'`)}`)
+            .expectStatus(200)
+            .matchHeaderSnapshot({
+                etag: anyEtag,
+                'content-version': anyContentVersion
+            })
+            .matchBodySnapshot({
+                events: new Array(1).fill({
+                    type: 'aggregated_click_event',
+                    data: anyObject
+                })
+            })
+            .expect(({body}) => {
+                assert(body.events.find(e => e.type === 'aggregated_click_event'), 'Expected a aggregated_click event');
+                assert(!body.events.find(e => e.type !== 'aggregated_click_event'), 'Expected only aggregated_click events');
+            });
+    });
+
     it('Returns signup events in activity feed', async function () {
         // Check activity feed
         await agent

ghost/members-api/lib/repositories/EventRepository.js

@@ -1,7 +1,7 @@
 const errors = require('@tryghost/errors');
 const nql = require('@tryghost/nql');
 const mingo = require('mingo');
-const {replaceFilters, expandFilters, splitFilter, getUsedKeys, chainTransformers, mapKeys} = require('@tryghost/mongo-utils');
+const {replaceFilters, expandFilters, splitFilter, getUsedKeys, chainTransformers, mapKeys, rejectStatements} = require('@tryghost/mongo-utils');
 
 /**
  * This mongo transformer ignores the provided filter option and replaces the filter with a custom filter that was provided to the transformer. Allowing us to set a mongo filter instead of a string based NQL filter.
@@ -489,23 +489,66 @@ module.exports = class EventRepository {
      * This groups click events per member for the same post, and only returns the first actual event, and includes the total clicks per event (for the same member and post)
      */
     async getAggregatedClickEvents(options = {}, filter) {
-        // This counts all clicks for a member for the same post
-        const postClickQuery = `SELECT count(distinct A.redirect_id)
-            FROM members_click_events A
-            LEFT JOIN redirects A_r on A_r.id = A.redirect_id
-            LEFT JOIN redirects B_r on B_r.id = members_click_events.redirect_id
-            WHERE A.member_id = members_click_events.member_id AND A_r.post_id = B_r.post_id`;
-
-        // Counts all clicks for the same member, for the same post, but only preceding events. This should be zero to include the event (so we only include the first events)
-        const postClickQueryPreceding = `SELECT count(distinct A.redirect_id)
-            FROM members_click_events A
-            LEFT JOIN redirects A_r on A_r.id = A.redirect_id
-            LEFT JOIN redirects B_r on B_r.id = members_click_events.redirect_id
-            WHERE A.member_id = members_click_events.member_id AND A_r.post_id = B_r.post_id AND (A.created_at < members_click_events.created_at OR (A.created_at = members_click_events.created_at AND A.id < members_click_events.id))`;
+        let postId = '';
+
+        if (filter && filter.$and) {
+            // Case when there is an $and condition
+            postId = filter.$and.find(condition => condition['data.post_id'])?.['data.post_id'];
+        } else {
+            // Case when there's no $and condition, directly look for data.post_id
+            postId = filter ? filter['data.post_id'] : '';
+        }
 
+        //Remove type filter as we don't need it in the query 
+        const [typeFilter, otherFilter] = this.getNQLSubset(options.filter); // eslint-disable-line
+
+        filter = this.removePostIdFilter(otherFilter); //Remove post_id filter as we don't need it in the query
+
+        let postClicksQuery = postId && postId !== '' ? `SELECT
+                    mce.id,
+                    mce.member_id,
+                    mce.redirect_id,
+                    mce.created_at
+                FROM
+                    members_click_events mce
+                INNER JOIN
+                    redirects r ON mce.redirect_id = r.id
+                WHERE
+                    r.post_id = '${postId}'
+        `
+            : `SELECT
+                        mce.id,
+                        mce.member_id,
+                        mce.redirect_id,
+                        mce.created_at
+                    FROM
+                        members_click_events mce
+                    INNER JOIN
+                        redirects r ON mce.redirect_id = r.id
+            `;
+
+        const firstClicksQuery = `
+            SELECT
+                id,
+                member_id,
+                redirect_id,
+                created_at,
+                ROW_NUMBER() OVER (PARTITION BY member_id ORDER BY created_at, id) AS rn
+            FROM
+                PostClicks
+        `;
+
+        const mainQuery = `SELECT COUNT(DISTINCT redirect_id)
+                    FROM PostClicks AS inner_mce
+                    WHERE inner_mce.member_id = FirstClicks.member_id
+                    AND inner_mce.redirect_id IN (
+                        SELECT redirect_id
+                        FROM PostClicks
+                    )`;
         options = {
             ...options,
             withRelated: ['member'],
+            filterRelations: false,
             filter: 'custom:true',
             useBasicCount: true,
             mongoTransformer: chainTransformers(
@@ -519,11 +562,22 @@ module.exports = class EventRepository {
                     'data.post_id': 'post_id'
                 })
             ),
+            useCTE: true,
             // We need to use MIN to make pagination work correctly
             // Note: we cannot do `count(distinct redirect_id) as count__clicks`, because we don't want the created_at filter to affect that count
             // For pagination to work correctly, we also need to return the id of the first event (or the minimum id if multiple events happend at the same time, but should be the first). Just MIN(id) won't work because that value changes if filter created_at < x is applied.
-            selectRaw: `id, member_id, created_at, (${postClickQuery}) as count__clicks`,
-            whereRaw: `(${postClickQueryPreceding}) = 0`
+            selectRaw: `id, member_id, created_at, (${mainQuery}) as count__clicks`,
+            whereRaw: `rn = 1 ORDER BY created_at DESC, id DESC`,
+            cte: [{
+                name: `PostClicks`,
+                query: postClicksQuery
+            },
+            {
+                name: `FirstClicks`,
+                query: firstClicksQuery
+            }],
+            from: 'FirstClicks',
+            order: ''
         };
 
         const {data: models, meta} = await this._MemberLinkClickEvent.findPage(options);
@@ -856,6 +910,20 @@ module.exports = class EventRepository {
         }
     }
 
+    removePostIdFilter(filter) {
+        if (!filter) {
+            return filter;
+        }
+
+        try {
+            return rejectStatements(filter, key => key === 'data.post_id');
+        } catch (e) {
+            throw new errors.IncorrectUsageError({
+                message: e.message
+            });
+        }
+    }
+
     async getMRR() {
         const results = await this._MemberPaidSubscriptionEvent.findAll({
             aggregateMRRDeltas: true