From 04c92d2ca5548050a0ed1fc7ebdbffb23a25805b Mon Sep 17 00:00:00 2001 From: Naz Date: Wed, 19 Jul 2023 20:26:30 +0800 Subject: [PATCH] Removed CollectionResourceChangeEvent handling refs https://github.com/TryGhost/Arch/issues/16 - The generic "UpdateAllCollections" logic should not be used as the mapped Post's delete/add/edit events should be sufficient in maintaining collection's state --- .../src/CollectionResourceChangeEvent.ts | 19 -------- ghost/collections/src/CollectionsService.ts | 26 +++-------- ghost/collections/src/index.ts | 1 - ghost/collections/test/collections.test.ts | 46 ------------------- .../src/ModelToDomainEventInterceptor.ts | 26 ++--------- .../model-to-domain-event-interceptor.test.ts | 19 +++----- 6 files changed, 19 insertions(+), 118 deletions(-) delete mode 100644 ghost/collections/src/CollectionResourceChangeEvent.ts diff --git a/ghost/collections/src/CollectionResourceChangeEvent.ts b/ghost/collections/src/CollectionResourceChangeEvent.ts deleted file mode 100644 index 4b8a2dff1f..0000000000 --- a/ghost/collections/src/CollectionResourceChangeEvent.ts +++ /dev/null @@ -1,19 +0,0 @@ -export class CollectionResourceChangeEvent { - name: string; - resourceType: 'post' | 'tag' | 'author'; - data: { - id: string; - }; - 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: {id: string}, timestamp = new Date()) { - return new CollectionResourceChangeEvent(name, data, timestamp); - } -} diff --git a/ghost/collections/src/CollectionsService.ts b/ghost/collections/src/CollectionsService.ts index e014e4232f..cfc5eb0c64 100644 --- a/ghost/collections/src/CollectionsService.ts +++ b/ghost/collections/src/CollectionsService.ts @@ -2,7 +2,6 @@ import logging from '@tryghost/logging'; import tpl from '@tryghost/tpl'; import ObjectID from 'bson-objectid'; import {Collection} from './Collection'; -import {CollectionResourceChangeEvent} from './CollectionResourceChangeEvent'; import {CollectionRepository} from './CollectionRepository'; import {CollectionPost} from './CollectionPost'; import {MethodNotAllowedError, NotFoundError} from '@tryghost/errors'; @@ -173,13 +172,6 @@ export class CollectionsService { * @description Subscribes to Domain events to update collections when posts are added, updated or deleted */ subscribeToEvents() { - // generic handler for all events that are not handled optimally yet - // this handler should go away once we have logic fo reach event - this.DomainEvents.subscribe(CollectionResourceChangeEvent, async () => { - logging.info('CollectionResourceChangeEvent received, updating all collections'); - await this.updateCollections(); - }); - this.DomainEvents.subscribe(PostDeletedEvent, async (event: PostDeletedEvent) => { logging.info(`PostDeletedEvent received, removing post ${event.id} from all collections`); await this.removePostFromAllCollections(event.id); @@ -237,19 +229,15 @@ export class CollectionsService { return this.toDTO(collection); } - private async updateAutomaticCollectionItems(collection: Collection, filter?:string) { - const collectionFilter = filter || collection.filter; + private async updateAutomaticCollectionItems(collection: Collection, filter:string) { + const posts = await this.postsRepository.getAll({ + filter: filter + }); - if (collectionFilter) { - const posts = await this.postsRepository.getAll({ - filter: collectionFilter - }); + collection.removeAllPosts(); - collection.removeAllPosts(); - - for (const post of posts) { - await collection.addPost(post); - } + for (const post of posts) { + await collection.addPost(post); } } diff --git a/ghost/collections/src/index.ts b/ghost/collections/src/index.ts index 415d139276..205a008490 100644 --- a/ghost/collections/src/index.ts +++ b/ghost/collections/src/index.ts @@ -1,7 +1,6 @@ export * from './CollectionsService'; export * from './CollectionsRepositoryInMemory'; export * from './Collection'; -export * from './CollectionResourceChangeEvent'; export * from './events/PostDeletedEvent'; export * from './events/PostAddedEvent'; export * from './events/PostEditedEvent'; diff --git a/ghost/collections/test/collections.test.ts b/ghost/collections/test/collections.test.ts index 369b770600..126c099acf 100644 --- a/ghost/collections/test/collections.test.ts +++ b/ghost/collections/test/collections.test.ts @@ -1,10 +1,8 @@ import assert from 'assert/strict'; -import sinon from 'sinon'; import DomainEvents from '@tryghost/domain-events'; import { CollectionsService, CollectionsRepositoryInMemory, - CollectionResourceChangeEvent, PostDeletedEvent, PostAddedEvent, PostEditedEvent @@ -313,26 +311,6 @@ describe('CollectionsService', function () { }); }); - describe('subscribeToEvents', function () { - it('Subscribes to Domain Events', async function () { - const updateCollectionsSpy = sinon.spy(collectionsService, 'updateCollections'); - const collectionChangeEvent = CollectionResourceChangeEvent.create('tag.added', { - id: 'test-id' - }); - - 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'); - }); - }); - describe('Automatic Collections', function () { it('Can create an automatic collection', async function () { const collection = await collectionsService.createCollection({ @@ -443,30 +421,6 @@ describe('CollectionsService', function () { assert.equal((await collectionsService.getById(manualCollection.id))?.posts.length, 2); }); - it('Updates automatic collections only when post is published', async function () { - const newPost = { - id: 'post-published', - title: 'Post Published', - slug: 'post-published', - featured: true, - published_at: new Date('2023-03-16T07:19:07.447Z'), - deleted: false - }; - await postsRepository.save(newPost); - - collectionsService.subscribeToEvents(); - const updateCollectionEvent = CollectionResourceChangeEvent.create('post.published', { - id: newPost.id - }); - - 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); - assert.equal((await collectionsService.getById(manualCollection.id))?.posts.length, 2); - }); - it('Moves post form featured to non featured collection when the featured attribute is changed', async function () { collectionsService.subscribeToEvents(); const newFeaturedPost = { diff --git a/ghost/model-to-domain-event-interceptor/src/ModelToDomainEventInterceptor.ts b/ghost/model-to-domain-event-interceptor/src/ModelToDomainEventInterceptor.ts index 7cf496cf3b..3377eacaeb 100644 --- a/ghost/model-to-domain-event-interceptor/src/ModelToDomainEventInterceptor.ts +++ b/ghost/model-to-domain-event-interceptor/src/ModelToDomainEventInterceptor.ts @@ -1,5 +1,4 @@ const { - CollectionResourceChangeEvent, PostDeletedEvent, PostAddedEvent, PostEditedEvent @@ -29,19 +28,8 @@ export class ModelToDomainEventInterceptor { 'post.added', 'post.deleted', 'post.edited', - - // @NOTE: uncomment events below once they have appropriate DomainEvent to map to - // 'tag.added', - // 'tag.edited', - // 'tag.attached', - // 'tag.detached', - // 'tag.deleted', - - // 'user.activated', - 'user.activated.edited' - // 'user.attached', - // 'user.detached', - // 'user.deleted' + // NOTE: currently unmapped and unused event + 'tag.added' ]; for (const modelEventName of ghostModelUpdateEvents) { @@ -58,11 +46,6 @@ export class ModelToDomainEventInterceptor { } domainEventDispatcher(modelEventName: string, data: any) { - const change = Object.assign({}, { - id: data.id, - resource: modelEventName.split('.')[0] - }, data._changed); - let event; switch (modelEventName) { @@ -101,9 +84,10 @@ export class ModelToDomainEventInterceptor { }); break; default: - event = CollectionResourceChangeEvent.create(modelEventName, change); } - this.DomainEvents.dispatch(event); + if (event) { + this.DomainEvents.dispatch(event); + } } } diff --git a/ghost/model-to-domain-event-interceptor/test/model-to-domain-event-interceptor.test.ts b/ghost/model-to-domain-event-interceptor/test/model-to-domain-event-interceptor.test.ts index 24234442a3..1342c130ef 100644 --- a/ghost/model-to-domain-event-interceptor/test/model-to-domain-event-interceptor.test.ts +++ b/ghost/model-to-domain-event-interceptor/test/model-to-domain-event-interceptor.test.ts @@ -1,8 +1,8 @@ import assert from 'assert/strict'; import events from 'events'; +import sinon from 'sinon'; import DomainEvents from '@tryghost/domain-events'; const { - CollectionResourceChangeEvent, PostDeletedEvent, PostEditedEvent, PostAddedEvent @@ -149,28 +149,23 @@ describe('ModelToDomainEventInterceptor', function () { assert.ok(interceptedEvent); }); - it('Intercepts unmapped Model event and dispatches CollectionResourceChangeEvent Domain event', async function () { + it('Intercepts unmapped Model event and dispatches nothing', async function () { let eventRegistry = new EventRegistry(); const modelToDomainEventInterceptor = new ModelToDomainEventInterceptor({ ModelEvents: eventRegistry, DomainEvents: DomainEvents }); + const domainEventsSpy = sinon.spy(DomainEvents, 'dispatch'); + modelToDomainEventInterceptor.init(); - let interceptedEvent; - DomainEvents.subscribe(CollectionResourceChangeEvent, (event: any) => { - assert.equal(event.name, 'user.activated.edited'); - assert.equal(event.data.id, '1234-user-edit'); - interceptedEvent = event; - }); - - eventRegistry.emit('user.activated.edited', { - id: '1234-user-edit' + eventRegistry.emit('tag.added', { + id: '1234-tag' }); await DomainEvents.allSettled(); - assert.ok(interceptedEvent); + assert.equal(domainEventsSpy.called, false); }); });