Introduced PostEvent classes to make event handling less generic

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

- When handling a single generic event the code becomes riddled with if statements to detect correct "data" that is being passed with the event. Switching to have a domain event per model event helps solving this problem and makes code more readable.

ghost/collections/src/CollectionResourceChangeEvent.ts

@@ -1,21 +1,19 @@
-type CollectionResourceChangeEventData = {
-    id: string;
-    resource: 'post' | 'tag' | 'author';
-    [any: string]: any;
-};
-
 export class CollectionResourceChangeEvent {
     name: string;
-    data: CollectionResourceChangeEventData;
+    resourceType: 'post' | 'tag' | 'author';
+    data: {
+        id: string;
+    };
     timestamp: Date;
 
-    constructor(name: string, data: CollectionResourceChangeEventData, timestamp: Date) {
+    constructor(name: string, data: {id: string}, timestamp: Date) {
         this.name = name;
+        this.resourceType = name.split('.')[0] as 'post' | 'tag' | 'author';
         this.data = data;
         this.timestamp = timestamp;
     }
 
-    static create(name: string, data: CollectionResourceChangeEventData, timestamp = new Date()) {
+    static create(name: string, data: {id: string}, timestamp = new Date()) {
         return new CollectionResourceChangeEvent(name, data, timestamp);
     }
 }

ghost/collections/src/CollectionsService.ts

@@ -4,6 +4,8 @@ import {CollectionRepository} from './CollectionRepository';
 import tpl from '@tryghost/tpl';
 import {MethodNotAllowedError, NotFoundError} from '@tryghost/errors';
 import DomainEvents from '@tryghost/domain-events';
+import {PostDeletedEvent} from './events/PostDeletedEvent';
+import {PostAddedEvent} from './events/PostAddedEvent';
 
 const messages = {
     cannotDeleteBuiltInCollectionError: {
@@ -32,6 +34,7 @@ type CollectionPostListItemDTO = {
     title: string;
     featured: boolean;
     featured_image?: string;
+    published_at: Date
 }
 
 type ManualCollection = {
@@ -69,12 +72,6 @@ type CollectionDTO = {
     posts: CollectionPostDTO[];
 };
 
-type CollectionPostInputDTO = {
-    id: string;
-    featured: boolean;
-    published_at: Date;
-};
-
 type QueryOptions = {
     filter?: string;
     include?: string;
@@ -137,8 +134,18 @@ export class CollectionsService {
      * @description Subscribes to Domain events to update collections when posts are added, updated or deleted
      */
     subscribeToEvents() {
-        DomainEvents.subscribe(CollectionResourceChangeEvent, async (event: CollectionResourceChangeEvent) => {
+        // generic handler for all events that are not handled optimally yet
-            await this.updateCollections(event);
+        // this handler should go away once we have logic fo reach event
+        DomainEvents.subscribe(CollectionResourceChangeEvent, async () => {
+            await this.updateCollections();
+        });
+
+        DomainEvents.subscribe(PostDeletedEvent, async (event: PostDeletedEvent) => {
+            await this.removePostFromAllCollections(event.id);
+        });
+
+        DomainEvents.subscribe(PostAddedEvent, async (event: PostAddedEvent) => {
+            await this.addPostToMatchingCollections(event.data);
         });
     }
 
@@ -168,7 +175,7 @@ export class CollectionsService {
         return this.toDTO(collection);
     }
 
-    async addPostToCollection(collectionId: string, post: CollectionPostInputDTO): Promise<CollectionDTO | null> {
+    async addPostToCollection(collectionId: string, post: CollectionPostListItemDTO): Promise<CollectionDTO | null> {
         const collection = await this.collectionsRepository.getById(collectionId);
 
         if (!collection) {
@@ -208,19 +215,32 @@ export class CollectionsService {
         }
     }
 
-    async updateCollections(event: CollectionResourceChangeEvent) {
+    private async addPostToMatchingCollections(post: {id: string, featured: boolean, published_at: Date}) {
-        if (event.name === 'post.deleted') {
+        const collections = await this.collectionsRepository.getAll({
-            // NOTE: 'delete' works the same for both manual and automatic collections
+            filter: 'type:automatic'
-            await this.removePostFromAllCollections(event.data.id);
+        });
-        } else {
-            const collections = await this.collectionsRepository.getAll({
-                filter: 'type:automatic'
-            });
 
-            for (const collection of collections) {
+        for (const collection of collections) {
-                await this.updateAutomaticCollectionItems(collection);
+            await collection.addPost(post);
-                await this.collectionsRepository.save(collection);
+            // const added = await collection.addPost(post);
-            }
+            // if (added) {
+            await this.collectionsRepository.save(collection);
+            // }
+        }
+    }
+
+    /**
+     * @description Updates all automatic collections. Can be time intensive and is a temporary solution
+     * while all of the events are mapped out and handled optimally
+     */
+    async updateCollections() {
+        const collections = await this.collectionsRepository.getAll({
+            filter: 'type:automatic'
+        });
+
+        for (const collection of collections) {
+            await this.updateAutomaticCollectionItems(collection);
+            await this.collectionsRepository.save(collection);
         }
     }
 

ghost/collections/src/events/PostAddedEvent.ts

@@ -0,0 +1,22 @@
+type PostData = {
+    id: string;
+    featured: boolean;
+    published_at: Date;
+    timestamp: Date;
+};
+
+export class PostAddedEvent {
+    id: string;
+    data: PostData;
+    timestamp: Date;
+
+    constructor(data: PostAddedEvent, timestamp: Date) {
+        this.id = data.id;
+        this.data = data.data;
+        this.timestamp = timestamp;
+    }
+
+    static create(data: any, timestamp = new Date()) {
+        return new PostAddedEvent(data, timestamp);
+    }
+}

ghost/collections/src/events/PostDeletedEvent.ts

@@ -0,0 +1,15 @@
+export class PostDeletedEvent {
+    id: string;
+    data: any;
+    timestamp: Date;
+
+    constructor(data: PostDeletedEvent, timestamp: Date) {
+        this.id = data.id;
+        this.data = data.data;
+        this.timestamp = timestamp;
+    }
+
+    static create(data: any, timestamp = new Date()) {
+        return new PostDeletedEvent(data, timestamp);
+    }
+}

ghost/collections/src/index.ts

@@ -2,3 +2,5 @@ export * from './CollectionsService';
 export * from './CollectionsRepositoryInMemory';
 export * from './Collection';
 export * from './CollectionResourceChangeEvent';
+export * from './events/PostDeletedEvent';
+export * from './events/PostAddedEvent';

ghost/collections/test/collections.test.ts

@@ -4,7 +4,8 @@ import DomainEvents from '@tryghost/domain-events';
 import {
     CollectionsService,
     CollectionsRepositoryInMemory,
-    CollectionResourceChangeEvent
+    CollectionResourceChangeEvent,
+    PostDeletedEvent
 } from '../src/index';
 import {PostsRepositoryInMemory} from './fixtures/PostsRepositoryInMemory';
 import {posts} from './fixtures/posts';
@@ -262,19 +263,21 @@ describe('CollectionsService', function () {
     });
 
     describe('subscribeToEvents', function () {
-        it('Subscribes to Domain Events', function () {
+        it('Subscribes to Domain Events', async function () {
             const updateCollectionsSpy = sinon.spy(collectionsService, 'updateCollections');
-            const collectionChangeEvent = CollectionResourceChangeEvent.create('post.added', {
+            const collectionChangeEvent = CollectionResourceChangeEvent.create('tag.added', {
-                id: 'test-id',
+                id: 'test-id'
-                resource: 'post'
             });
 
             DomainEvents.dispatch(collectionChangeEvent);
+            await DomainEvents.allSettled();
             assert.equal(updateCollectionsSpy.calledOnce, false, 'updateCollections should not be called yet');
 
             collectionsService.subscribeToEvents();
 
             DomainEvents.dispatch(collectionChangeEvent);
+            await DomainEvents.allSettled();
+
             assert.equal(updateCollectionsSpy.calledOnce, true, 'updateCollections should be called');
         });
     });
@@ -377,12 +380,13 @@ describe('CollectionsService', function () {
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 2);
                 assert.equal((await collectionsService.getById(manualCollection.id))?.posts.length, 2);
 
-                const updateCollectionEvent = CollectionResourceChangeEvent.create('post.deleted', {
+                collectionsService.subscribeToEvents();
-                    id: posts[0].id,
+                const postDeletedEvent = PostDeletedEvent.create({
-                    resource: 'post'
+                    id: posts[0].id
                 });
 
-                await collectionsService.updateCollections(updateCollectionEvent);
+                DomainEvents.dispatch(postDeletedEvent);
+                await DomainEvents.allSettled();
 
                 assert.equal((await collectionsService.getById(automaticFeaturedCollection.id))?.posts?.length, 2);
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 1);
@@ -400,12 +404,13 @@ describe('CollectionsService', function () {
                 };
                 await postsRepository.save(newPost);
 
+                collectionsService.subscribeToEvents();
                 const updateCollectionEvent = CollectionResourceChangeEvent.create('post.published', {
-                    id: newPost.id,
+                    id: newPost.id
-                    resource: 'post'
                 });
 
-                await collectionsService.updateCollections(updateCollectionEvent);
+                DomainEvents.dispatch(updateCollectionEvent);
+                await DomainEvents.allSettled();
 
                 assert.equal((await collectionsService.getById(automaticFeaturedCollection.id))?.posts?.length, 3);
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 2);

ghost/core/core/server/services/collections/model-to-domain-events-bridge.js

@@ -1,6 +1,8 @@
 const DomainEvents = require('@tryghost/domain-events');
 const {
-    CollectionResourceChangeEvent
+    CollectionResourceChangeEvent,
+    PostDeletedEvent,
+    PostAddedEvent
 } = require('@tryghost/collections');
 
 const domainEventDispatcher = (modelEventName, data) => {
@@ -8,22 +10,36 @@ const domainEventDispatcher = (modelEventName, data) => {
         id: data.id,
         resource: modelEventName.split('.')[0]
     }, data._changed);
-    const collectionResourceChangeEvent = CollectionResourceChangeEvent.create(modelEventName, change);
 
-    DomainEvents.dispatch(collectionResourceChangeEvent);
+    let event;
+    if (modelEventName === 'post.deleted') {
+        event = PostDeletedEvent.create({id: data.id});
+    } if (modelEventName === 'post.added') {
+        event = PostAddedEvent.create({
+            id: data.id,
+            featured: data.featured,
+            published_at: data.published_at
+        });
+    } else {
+        event = CollectionResourceChangeEvent.create(modelEventName, change);
+    }
+
+    DomainEvents.dispatch(event);
 };
 
 const translateModelEventsToDomainEvents = () => {
     const events = require('../../lib/common/events');
     const ghostModelUpdateEvents = [
-        'post.published',
+        'post.added',
-        'post.published.edited',
+        'post.deleted',
-        'post.unpublished',
+        'post.edited',
+
         'tag.added',
         'tag.edited',
         'tag.attached',
         'tag.detached',
         'tag.deleted',
+
         'user.activated',
         'user.activated.edited',
         'user.attached',