Fixed member webhooks (#394)

refs https://github.com/TryGhost/Team/issues/1577

The call to `edit` was not loading the newsletter relations which is needed
by the serializer used by the webhooks service.

Co-authored-by: Fabien "egg" O'Carroll <fabien@allou.is>

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

@@ -168,6 +168,10 @@ module.exports = class MemberRepository {
     }
 
     async create(data, options) {
+        if (!options) {
+            options = {};
+        }
+
         const {labels} = data;
 
         if (labels) {
@@ -207,11 +211,19 @@ module.exports = class MemberRepository {
             memberData.newsletters = await this.getSubscribeOnSignupNewsletters(browseOptions);
         }
 
+        const withRelated = options.withRelated ? options.withRelated : [];
+        if (!withRelated.includes('labels')) {
+            withRelated.push('labels');
+        }
+        if (!withRelated.includes('newsletters')) {
+            withRelated.push('newsletters');
+        }
+
         const member = await this._Member.add({
             ...memberData,
             ...memberStatusData,
             labels
-        }, options);
+        }, {...options, withRelated});
 
         for (const product of member.related('products').models) {
             await this._MemberProductEvent.add({
@@ -286,6 +298,18 @@ module.exports = class MemberRepository {
             transacting: options.transacting
         };
 
+        if (!options) {
+            options = {};
+        }
+
+        const withRelated = options.withRelated ? options.withRelated : [];
+        if (!withRelated.includes('labels')) {
+            withRelated.push('labels');
+        }
+        if (!withRelated.includes('newsletters')) {
+            withRelated.push('newsletters');
+        }
+
         const initialMember = await this._Member.findOne({
             id: options.id
         }, {...sharedOptions, withRelated: ['products', 'newsletters']});
@@ -301,7 +325,8 @@ module.exports = class MemberRepository {
             'labels',
             'geolocation',
             'products',
-            'newsletters'
+            'newsletters',
+            'last_seen_at'
         ]);
 
         const memberStatusData = {};
@@ -375,7 +400,7 @@ module.exports = class MemberRepository {
         const member = await this._Member.edit({
             ...memberData,
             ...memberStatusData
-        }, options);
+        }, {...options, withRelated});
 
         for (const productToAdd of productsToAdd) {
             await this._MemberProductEvent.add({

ghost/members-events-service/lib/last-seen-at-updater.js

@@ -1,5 +1,6 @@
 const {MemberPageViewEvent} = require('@tryghost/member-events');
 const moment = require('moment-timezone');
+const {IncorrectUsageError} = require('@tryghost/errors');
 
 /**
  * Listen for `MemberViewEvent` to update the `member.last_seen_at` timestamp
@@ -8,22 +9,23 @@ class LastSeenAtUpdater {
     /**
      * Initializes the event subscriber
      * @param {Object} deps dependencies
-     * @param {Object} deps.models The list of model dependencies
-     * @param {any} deps.models.Member The Member model
      * @param {Object} deps.services The list of service dependencies
      * @param {any} deps.services.domainEvents The DomainEvents service
      * @param {any} deps.services.settingsCache The settings service
+     * @param {() => object} deps.getMembersApi - A function which returns an instance of members-api
      */
     constructor({
-        models: {
-            Member
-        },
         services: {
             domainEvents,
             settingsCache
-        }
+        },
+        getMembersApi
     }) {
-        this._memberModel = Member;
+        if (!getMembersApi) {
+            throw new IncorrectUsageError({message: 'Missing option getMembersApi'});
+        }
+
+        this._getMembersApi = getMembersApi;
         this._domainEventsService = domainEvents;
         this._settingsCacheService = settingsCache;
 
@@ -44,7 +46,8 @@ class LastSeenAtUpdater {
     async updateLastSeenAt(memberId, memberLastSeenAt, timestamp) {
         const timezone = this._settingsCacheService.get('timezone');
         if (memberLastSeenAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(memberLastSeenAt)) {
-            await this._memberModel.edit({
+            const membersApi = await this._getMembersApi();
+            await membersApi.members.update({
                 last_seen_at: moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss')
             }, {
                 id: memberId

ghost/members-events-service/test/last-seen-at-updater.test.js

@@ -6,78 +6,81 @@ const assert = require('assert');
 const sinon = require('sinon');
 const {LastSeenAtUpdater} = require('../');
 const DomainEvents = require('@tryghost/domain-events');
-const {MemberPageViewEvent, MemberSubscribeEvent} = require('@tryghost/member-events');
+const {MemberPageViewEvent} = require('@tryghost/member-events');
 const moment = require('moment');
 
 describe('LastSeenAtUpdater', function () {
-    it('Fires on MemberPageViewEvent events', async function () {
+    it('Calls updateLastSeenAt on MemberPageViewEvents', async function () {
         const now = moment('2022-02-28T18:00:00Z').utc();
         const previousLastSeen = moment('2022-02-27T23:00:00Z').toISOString();
-        const spy = sinon.spy();
+        const stub = sinon.stub().resolves();
         const settingsCache = sinon.stub().returns('Etc/UTC');
-        new LastSeenAtUpdater({
-            models: {
-                Member: {
-                    edit: spy
-                }
-            },
+        const updater = new LastSeenAtUpdater({
             services: {
                 settingsCache: {
                     get: settingsCache
                 },
                 domainEvents: DomainEvents
+            },
+            async getMembersApi() {
+                return {
+                    members: {
+                        update: stub
+                    }
+                };
             }
         });
+        sinon.stub(updater, 'updateLastSeenAt');
         DomainEvents.dispatch(MemberPageViewEvent.create({memberId: '1', memberLastSeenAt: previousLastSeen, url: '/'}, now.toDate()));
-        assert(spy.calledOnceWithExactly({
-            last_seen_at: now.format('YYYY-MM-DD HH:mm:ss')
-        }, {
-            id: '1'
-        }), 'The LastSeenAtUpdater should attempt a member update with the current date.');
+        assert(updater.updateLastSeenAt.calledOnceWithExactly('1', previousLastSeen, now.toDate()));
     });
 
     it('works correctly on another timezone (not updating last_seen_at)', async function () {
         const now = moment('2022-02-28T04:00:00Z').utc();
         const previousLastSeen = moment('2022-02-27T20:00:00Z').toISOString();
-        const spy = sinon.spy();
+        const stub = sinon.stub().resolves();
         const settingsCache = sinon.stub().returns('Asia/Bangkok');
-        new LastSeenAtUpdater({
-            models: {
-                Member: {
-                    edit: spy
-                }
-            },
+        const updater = new LastSeenAtUpdater({
             services: {
                 settingsCache: {
                     get: settingsCache
                 },
                 domainEvents: DomainEvents
+            },
+            async getMembersApi() {
+                return {
+                    members: {
+                        update: stub
+                    }
+                };
             }
         });
-        DomainEvents.dispatch(MemberPageViewEvent.create({memberId: '1', memberLastSeenAt: previousLastSeen, url: '/'}, now.toDate()));
-        assert(spy.notCalled, 'The LastSeenAtUpdater should attempt a member update when the new timestamp is within the same day in the publication timezone.');
+        await updater.updateLastSeenAt('1', previousLastSeen, now.toDate());
+        assert(stub.notCalled, 'The LastSeenAtUpdater should attempt a member update when the new timestamp is within the same day in the publication timezone.');
     });
 
     it('works correctly on another timezone (updating last_seen_at)', async function () {
         const now = moment('2022-02-28T04:00:00Z').utc();
         const previousLastSeen = moment('2022-02-27T20:00:00Z').toISOString();
-        const spy = sinon.spy();
+        const stub = sinon.stub().resolves();
         const settingsCache = sinon.stub().returns('Europe/Paris');
-        new LastSeenAtUpdater({
-            models: {
-                Member: {
-                    edit: spy
-                }
-            },
+        const updater = new LastSeenAtUpdater({
             services: {
                 settingsCache: {
                     get: settingsCache
                 },
                 domainEvents: DomainEvents
+            },
+            async getMembersApi() {
+                return {
+                    members: {
+                        update: stub
+                    }
+                };
             }
         });
-        DomainEvents.dispatch(MemberPageViewEvent.create({memberId: '1', memberLastSeenAt: previousLastSeen, url: '/'}, now.toDate()));
-        assert(spy.calledOnceWithExactly({
+        await updater.updateLastSeenAt('1', previousLastSeen, now.toDate());
+        assert(stub.calledOnceWithExactly({
             last_seen_at: now.format('YYYY-MM-DD HH:mm:ss')
         }, {
             id: '1'
@@ -87,43 +90,47 @@ describe('LastSeenAtUpdater', function () {
     it('Doesn\'t update when last_seen_at is too recent', async function () {
         const now = moment('2022-02-28T18:00:00Z');
         const previousLastSeen = moment('2022-02-28T00:00:00Z').toISOString();
-        const spy = sinon.spy();
+        const stub = sinon.stub().resolves();
         const settingsCache = sinon.stub().returns('Etc/UTC');
-        new LastSeenAtUpdater({
-            models: {
-                Member: {
-                    edit: spy
-                }
-            },
+        const updater = new LastSeenAtUpdater({
             services: {
                 settingsCache: {
                     get: settingsCache
                 },
                 domainEvents: DomainEvents
+            },
+            async getMembersApi() {
+                return {
+                    members: {
+                        update: stub
+                    }
+                };
             }
         });
-        DomainEvents.dispatch(MemberPageViewEvent.create({memberId: '1', memberLastSeenAt: previousLastSeen, url: '/'}, now.toDate()));
-        assert(spy.notCalled, 'The LastSeenAtUpdater should\'t update a member when the previous last_seen_at is close to the event timestamp.');
+        await updater.updateLastSeenAt('1', previousLastSeen, now.toDate());
+        assert(stub.notCalled, 'The LastSeenAtUpdater should\'t update a member when the previous last_seen_at is close to the event timestamp.');
     });
 
     it('Doesn\'t fire on other events', async function () {
         const now = moment('2022-02-28T18:00:00Z');
-        const spy = sinon.spy();
+        const stub = sinon.stub().resolves();
         const settingsCache = sinon.stub().returns('Etc/UTC');
-        new LastSeenAtUpdater({
-            models: {
-                Member: {
-                    edit: spy
-                }
-            },
+        const updater = new LastSeenAtUpdater({
             services: {
                 settingsCache: {
                     get: settingsCache
                 },
                 domainEvents: DomainEvents
+            },
+            async getMembersApi() {
+                return {
+                    members: {
+                        update: stub
+                    }
+                };
             }
         });
-        DomainEvents.dispatch(MemberSubscribeEvent.create({memberId: '1', source: 'api'}, now.toDate()));
-        assert(spy.notCalled, 'The LastSeenAtUpdater should never fire on MemberPageViewEvent events.');
+        await updater.updateLastSeenAt('1', undefined, now.toDate());
+        assert(stub.notCalled, 'The LastSeenAtUpdater should never fire on MemberPageViewEvent events.');
     });
 });