From 2390afc6f1c551b6f92490e2a4e8c5f5e1312244 Mon Sep 17 00:00:00 2001 From: Naz Date: Mon, 5 Jun 2023 18:33:04 +0700 Subject: [PATCH] Added automatic collection recalculation closes https://github.com/TryGhost/Team/issues/3170 - When resources that are related to automatic collection filter are updated the posts in collections should be updated as well. - This change adds a super-basic way to track changes in post/tag/author resources and updated all automatic collections when any of those resources change. In the future we can optimize the update process to be more performant, but it's good enough for current needs --- .../src/CollectionsRepositoryInMemory.ts | 3 +- ghost/collections/src/CollectionsService.ts | 37 ++++++++++++++----- ghost/collections/test/collections.test.ts | 25 +++++++++++++ .../core/server/services/collections/index.js | 12 ++++++ .../services/collections/update-events.js | 15 ++++++++ 5 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 ghost/core/core/server/services/collections/update-events.js diff --git a/ghost/collections/src/CollectionsRepositoryInMemory.ts b/ghost/collections/src/CollectionsRepositoryInMemory.ts index 3f2813bbcf..8246dc75fe 100644 --- a/ghost/collections/src/CollectionsRepositoryInMemory.ts +++ b/ghost/collections/src/CollectionsRepositoryInMemory.ts @@ -6,7 +6,8 @@ export class CollectionsRepositoryInMemory extends InMemoryRepository { const collection = await this.collectionsRepository.getById(data.id); @@ -153,15 +180,7 @@ export class CollectionsService { } if ((collection.type === 'automatic' || data.type === 'automatic') && data.filter) { - const posts = await this.postsRepository.getAll({ - filter: data.filter - }); - - collection.removeAllPosts(); - - for (const post of posts) { - collection.addPost(post); - } + await this.#updateAutomaticCollectionItems(collection, data.filter); } const collectionData = this.fromDTO(data); diff --git a/ghost/collections/test/collections.test.ts b/ghost/collections/test/collections.test.ts index 84d810af64..73a62a1887 100644 --- a/ghost/collections/test/collections.test.ts +++ b/ghost/collections/test/collections.test.ts @@ -225,5 +225,30 @@ describe('CollectionsService', function () { assert.equal(updatedCollection?.posts.length, 1, 'Collection should have one post'); assert.equal(updatedCollection?.posts[0].id, 'post-2', 'Collection should have the correct post'); }); + + // @NOTE: add a more comprehensive test as this one is too basic + it('Updates all automatic collections', async function () { + let collection1 = await collectionsService.createCollection({ + title: 'Featured Collection 1', + description: 'testing automatic collection', + type: 'automatic', + filter: 'featured:true' + }); + + let collection2 = await collectionsService.createCollection({ + title: 'Featured Collection 2', + description: 'testing automatic collection', + type: 'automatic', + filter: 'featured:true' + }); + + assert.equal(collection1.posts.length, 2); + assert.equal(collection2.posts.length, 2); + + await collectionsService.updateAutomaticCollections(); + + assert.equal(collection1.posts.length, 2); + assert.equal(collection2.posts.length, 2); + }); }); }); diff --git a/ghost/core/core/server/services/collections/index.js b/ghost/core/core/server/services/collections/index.js index 90c4cf1580..97e839997c 100644 --- a/ghost/core/core/server/services/collections/index.js +++ b/ghost/core/core/server/services/collections/index.js @@ -8,6 +8,7 @@ class CollectionsServiceWrapper { constructor() { const models = require('../../models'); + const events = require('../../lib/common/events'); const collectionsRepositoryInMemory = new CollectionsRepositoryInMemory(); const collectionsService = new CollectionsService({ @@ -22,6 +23,17 @@ class CollectionsServiceWrapper { } }); + // @NOTE: these should be reworked to use the "Event" classes + // instead of Bookshelf model events + const updateEvents = require('./update-events'); + + // @NOTE: naive update implementation to keep things simple for the first version + for (const event of updateEvents) { + events.on(event, () => { + collectionsService.updateAutomaticCollections(); + }); + } + this.api = { browse: collectionsService.getAll.bind(collectionsService), read: collectionsService.getById.bind(collectionsService), diff --git a/ghost/core/core/server/services/collections/update-events.js b/ghost/core/core/server/services/collections/update-events.js new file mode 100644 index 0000000000..f0d66869c6 --- /dev/null +++ b/ghost/core/core/server/services/collections/update-events.js @@ -0,0 +1,15 @@ +module.exports = [ + 'post.published', + 'post.published.edited', + 'post.unpublished', + 'tag.added', + 'tag.edited', + 'tag.attached', + 'tag.detached', + 'tag.deleted', + 'user.activated', + 'user.activated.edited', + 'user.attached', + 'user.detached', + 'user.deleted' +];