Removed post delete related event handling

closes https://github.com/TryGhost/Arch/issues/91

- We have on cascade delete (a9f9f6121a/ghost/core/core/server/data/schema/schema.js (L1068)) on `post_id` column which handles post deletion logic automatically on DB level.
- The commented out handlers in the long term should be hooked up with public CollectionService methods on the client side.

ghost/collections/src/CollectionsService.ts

@@ -13,10 +13,8 @@ import {Collection} from './Collection';
 import {CollectionRepository} from './CollectionRepository';
 import {CollectionPost} from './CollectionPost';
 import {MethodNotAllowedError} from '@tryghost/errors';
-import {PostDeletedEvent} from './events/PostDeletedEvent';
 import {PostAddedEvent} from './events/PostAddedEvent';
 import {PostEditedEvent} from './events/PostEditedEvent';
-import {PostsBulkDestroyedEvent} from '@tryghost/post-events';
 import {RepositoryUniqueChecker} from './RepositoryUniqueChecker';
 import {TagDeletedEvent} from './events/TagDeletedEvent';
 
@@ -173,15 +171,18 @@ export class CollectionsService {
      * @description Subscribes to Domain events to update collections when posts are added, updated or deleted
      */
     subscribeToEvents() {
-        this.DomainEvents.subscribe(PostDeletedEvent, async (event: PostDeletedEvent) => {
-            logging.info(`PostDeletedEvent received, removing post ${event.id} from all collections`);
-            try {
-                await this.removePostFromAllCollections(event.id);
-                /* c8 ignore next 3 */
-            } catch (err) {
-                logging.error({err, message: 'Error handling PostDeletedEvent'});
-            }
-        });
+        // @NOTE: event handling should be moved to the client - Ghost app
+        //        Leaving commented out handlers here to move them all together
+
+        // this.DomainEvents.subscribe(PostDeletedEvent, async (event: PostDeletedEvent) => {
+        //     logging.info(`PostDeletedEvent received, removing post ${event.id} from all collections`);
+        //     try {
+        //         await this.removePostFromAllCollections(event.id);
+        //         /* c8 ignore next 3 */
+        //     } catch (err) {
+        //         logging.error({err, message: 'Error handling PostDeletedEvent'});
+        //     }
+        // });
 
         this.DomainEvents.subscribe(PostAddedEvent, async (event: PostAddedEvent) => {
             logging.info(`PostAddedEvent received, adding post ${event.data.id} to matching collections`);
@@ -207,15 +208,15 @@ export class CollectionsService {
             }
         });
 
-        this.DomainEvents.subscribe(PostsBulkDestroyedEvent, async (event: PostsBulkDestroyedEvent) => {
-            logging.info(`BulkDestroyEvent received, removing posts ${event.data} from all collections`);
-            try {
-                await this.removePostsFromAllCollections(event.data);
-                /* c8 ignore next 3 */
-            } catch (err) {
-                logging.error({err, message: 'Error handling PostsBulkDestroyedEvent'});
-            }
-        });
+        // this.DomainEvents.subscribe(PostsBulkDestroyedEvent, async (event: PostsBulkDestroyedEvent) => {
+        //     logging.info(`BulkDestroyEvent received, removing posts ${event.data} from all collections`);
+        //     try {
+        //         await this.removePostsFromAllCollections(event.data);
+        //         /* c8 ignore next 3 */
+        //     } catch (err) {
+        //         logging.error({err, message: 'Error handling PostsBulkDestroyedEvent'});
+        //     }
+        // });
 
         this.DomainEvents.subscribe(PostsBulkUnpublishedEvent, async (event: PostsBulkUnpublishedEvent) => {
             logging.info(`PostsBulkUnpublishedEvent received, updating collection posts ${event.data}`);
@@ -356,7 +357,7 @@ export class CollectionsService {
         });
     }
 
-    private async removePostFromAllCollections(postId: string) {
+    async removePostFromAllCollections(postId: string) {
         return await this.collectionsRepository.createTransaction(async (transaction) => {
             // @NOTE: can be optimized by having a "getByPostId" method on the collections repository
             const collections = await this.collectionsRepository.getAll({transaction});
@@ -370,7 +371,7 @@ export class CollectionsService {
         });
     }
 
-    private async removePostsFromAllCollections(postIds: string[]) {
+    async removePostsFromAllCollections(postIds: string[]) {
         return await this.collectionsRepository.createTransaction(async (transaction) => {
             const collections = await this.collectionsRepository.getAll({transaction});
 

ghost/collections/src/index.ts

@@ -1,7 +1,6 @@
 export * from './CollectionsService';
 export * from './CollectionsRepositoryInMemory';
 export * from './Collection';
-export * from './events/PostDeletedEvent';
 export * from './events/PostAddedEvent';
 export * from './events/PostEditedEvent';
 export * from './events/TagDeletedEvent';

ghost/collections/test/collections.test.ts

@@ -4,13 +4,11 @@ import DomainEvents from '@tryghost/domain-events';
 import {
     CollectionsService,
     CollectionsRepositoryInMemory,
-    PostDeletedEvent,
     PostAddedEvent,
     PostEditedEvent,
     TagDeletedEvent
 } from '../src/index';
 import {
-    PostsBulkDestroyedEvent,
     PostsBulkUnpublishedEvent,
     PostsBulkFeaturedEvent,
     PostsBulkUnfeaturedEvent,
@@ -474,13 +472,7 @@ describe('CollectionsService', function () {
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 2);
                 assert.equal((await collectionsService.getById(manualCollection.id))?.posts.length, 2);
 
-                collectionsService.subscribeToEvents();
-                const postDeletedEvent = PostDeletedEvent.create({
-                    id: postFixtures[0].id
-                });
-
-                DomainEvents.dispatch(postDeletedEvent);
-                await DomainEvents.allSettled();
+                await collectionsService.removePostFromAllCollections(postFixtures[0].id);
 
                 assert.equal((await collectionsService.getById(automaticFeaturedCollection.id))?.posts?.length, 2);
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 1);
@@ -492,14 +484,11 @@ describe('CollectionsService', function () {
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 2);
                 assert.equal((await collectionsService.getById(manualCollection.id))?.posts.length, 2);
 
-                collectionsService.subscribeToEvents();
-                const postDeletedEvent = PostsBulkDestroyedEvent.create([
+                const deletedPostIds = [
                     postFixtures[0].id,
                     postFixtures[1].id
-                ]);
-
-                DomainEvents.dispatch(postDeletedEvent);
-                await DomainEvents.allSettled();
+                ];
+                await collectionsService.removePostsFromAllCollections(deletedPostIds);
 
                 assert.equal((await collectionsService.getById(automaticFeaturedCollection.id))?.posts?.length, 2);
                 assert.equal((await collectionsService.getById(automaticNonFeaturedCollection.id))?.posts.length, 0);

ghost/core/test/e2e-api/admin/collections.test.js

@@ -632,7 +632,10 @@ describe('Collections API', function () {
                     await DomainEvents.allSettled();
                 }, this.skip.bind(this));
                 const collectionRelatedQueries = queries.filter(query => query.sql.includes('collection'));
-                assert.equal(collectionRelatedQueries.length, 2);
+
+                // deletion is handled on the DB layer through Cascade Delete,
+                // so collections should not execute any additional queries
+                assert.equal(collectionRelatedQueries.length, 0);
             }
 
             await agent

ghost/model-to-domain-event-interceptor/package.json

@@ -26,7 +26,8 @@
         "sinon": "15.2.0"
     },
     "dependencies": {
-        "@tryghost/collections": "0.0.0"
+        "@tryghost/collections": "0.0.0",
+        "@tryghost/post-events": "0.0.0"
     },
     "c8": {
         "exclude": [

ghost/model-to-domain-event-interceptor/src/ModelToDomainEventInterceptor.ts

@@ -1,4 +1,5 @@
-import {PostDeletedEvent, PostAddedEvent, PostEditedEvent, TagDeletedEvent} from '@tryghost/collections';
+import {PostDeletedEvent} from '@tryghost/post-events';
+import {PostAddedEvent, PostEditedEvent, TagDeletedEvent} from '@tryghost/collections';
 
 type ModelToDomainEventInterceptorDeps = {
     ModelEvents: {

ghost/model-to-domain-event-interceptor/test/model-to-domain-event-interceptor.test.ts

@@ -3,7 +3,9 @@ import events from 'events';
 import sinon from 'sinon';
 import DomainEvents from '@tryghost/domain-events';
 import {
-    PostDeletedEvent,
+    PostDeletedEvent
+} from '@tryghost/post-events';
+import {
     PostEditedEvent,
     PostAddedEvent,
     TagDeletedEvent

ghost/post-events/src/index.ts

@@ -1,3 +1,5 @@
+export * from './PostDeletedEvent';
+
 export * from './PostsBulkDestroyedEvent';
 export * from './PostsBulkUnpublishedEvent';
 export * from './PostsBulkFeaturedEvent';

ghost/post-events/test/post-events.test.ts

@@ -1,5 +1,6 @@
 import assert from 'assert/strict';
 import {
+    PostDeletedEvent,
     PostsBulkDestroyedEvent,
     PostsBulkUnpublishedEvent,
     PostsBulkFeaturedEvent,
@@ -8,6 +9,12 @@ import {
 } from '../src/index';
 
 describe('Post Events', function () {
+    it('Can instantiate PostDeletedEvent', function () {
+        const event = PostDeletedEvent.create({id: 'post-id-1', data: {}});
+        assert.ok(event);
+        assert.equal(event.id, 'post-id-1');
+    });
+
     it('Can instantiate BulkDestroyEvent', function () {
         const event = PostsBulkDestroyedEvent.create(['1', '2', '3']);
         assert.ok(event);