From 92172aca8ed4eff2f8432d9446becd01801f016a Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Wed, 7 Jun 2023 15:06:15 +0200 Subject: [PATCH] Wired up collections to posts endpoint (#16945) - Added support for `include=collections` to the Posts Admin API behind a flag - Refactored some of the collections work to support it --- .../src/CollectionsRepositoryInMemory.ts | 8 +----- ghost/collections/src/CollectionsService.ts | 28 ++++++++++++------- ghost/collections/test/collections.test.ts | 20 ++++++++----- .../core/server/api/endpoints/collections.js | 12 ++++---- ghost/core/core/server/api/endpoints/posts.js | 17 +---------- .../utils/serializers/output/mappers/posts.js | 14 +++++++++- .../core/server/services/collections/index.js | 15 +++------- .../server/services/posts/posts-service.js | 4 ++- .../CollectionsServiceWrapper.test.js | 11 ++------ ghost/posts-service/lib/PostsService.js | 24 ++++++++++++++-- 10 files changed, 83 insertions(+), 70 deletions(-) diff --git a/ghost/collections/src/CollectionsRepositoryInMemory.ts b/ghost/collections/src/CollectionsRepositoryInMemory.ts index 4c17a872b1..31e4eb8b08 100644 --- a/ghost/collections/src/CollectionsRepositoryInMemory.ts +++ b/ghost/collections/src/CollectionsRepositoryInMemory.ts @@ -3,12 +3,6 @@ import {Collection} from './Collection'; export class CollectionsRepositoryInMemory extends InMemoryRepository { protected toPrimitive(entity: Collection): object { - return { - title: entity.title, - slug: entity.slug, - description: entity.description, - feature_image: entity.featureImage, - type: entity.type - }; + return entity.toJSON(); } } diff --git a/ghost/collections/src/CollectionsService.ts b/ghost/collections/src/CollectionsService.ts index 13944133ea..2d7c8b3fde 100644 --- a/ghost/collections/src/CollectionsService.ts +++ b/ghost/collections/src/CollectionsService.ts @@ -12,7 +12,7 @@ const messages = { type CollectionsServiceDeps = { collectionsRepository: CollectionRepository; - postsRepository: IPostsRepository; + postsRepository: PostsRepository; }; type CollectionPostDTO = { @@ -61,23 +61,23 @@ type CollectionPostInputDTO = { published_at: Date; }; -type IPostsRepository = { +interface PostsRepository { getAll(options: {filter?: string}): Promise; } export class CollectionsService { - collectionsRepository: CollectionRepository; - postsRepository: IPostsRepository; + private collectionsRepository: CollectionRepository; + private postsRepository: PostsRepository; constructor(deps: CollectionsServiceDeps) { this.collectionsRepository = deps.collectionsRepository; this.postsRepository = deps.postsRepository; } - toDTO(collection: Collection): CollectionDTO { + private toDTO(collection: Collection): CollectionDTO { return { id: collection.id, - title: collection.title || null, + title: collection.title, slug: collection.slug, description: collection.description || null, feature_image: collection.featureImage || null, @@ -92,7 +92,7 @@ export class CollectionsService { }; } - fromDTO(data: any): any { + private fromDTO(data: any): any { const mappedDTO: {[index: string]:any} = { title: data.title, slug: data.slug, @@ -151,7 +151,7 @@ export class CollectionsService { return this.toDTO(collection); } - async #updateAutomaticCollectionItems(collection: Collection, filter?:string) { + private async updateAutomaticCollectionItems(collection: Collection, filter?:string) { const collectionFilter = filter || collection.filter; if (collectionFilter) { @@ -173,7 +173,7 @@ export class CollectionsService { }); for (const collection of collections) { - await this.#updateAutomaticCollectionItems(collection); + await this.updateAutomaticCollectionItems(collection); await this.collectionsRepository.save(collection); } } @@ -192,7 +192,7 @@ export class CollectionsService { } if ((collection.type === 'automatic' || data.type === 'automatic') && data.filter) { - await this.#updateAutomaticCollectionItems(collection, data.filter); + await this.updateAutomaticCollectionItems(collection, data.filter); } const collectionData = this.fromDTO(data); @@ -226,6 +226,14 @@ export class CollectionsService { }; } + async getCollectionsForPost(postId: string): Promise { + const collections = await this.collectionsRepository.getAll({ + filter: `posts:${postId}` + }); + + return collections.map(collection => this.toDTO(collection)); + } + async destroy(id: string): Promise { const collection = await this.getById(id); diff --git a/ghost/collections/test/collections.test.ts b/ghost/collections/test/collections.test.ts index 03377ce2e1..c5565ac99d 100644 --- a/ghost/collections/test/collections.test.ts +++ b/ghost/collections/test/collections.test.ts @@ -1,5 +1,5 @@ import assert from 'assert'; -import {CollectionsService, CollectionsRepositoryInMemory, Collection} from '../src/index'; +import {CollectionsService, CollectionsRepositoryInMemory} from '../src/index'; import {PostsRepositoryInMemory} from './fixtures/PostsRepositoryInMemory'; import {posts} from './fixtures/posts'; @@ -80,13 +80,19 @@ describe('CollectionsService', function () { }); }); - describe('toDTO', function () { - it('Can map Collection entity to DTO object', async function () { - const collection = await Collection.create({}); - const dto = collectionsService.toDTO(collection); + describe('getCollectionsForPost', function () { + it('Can get collections for a post', async function () { + const collection = await collectionsService.createCollection({ + title: 'testing collections', + type: 'manual' + }); - assert.equal(dto.id, collection.id, 'DTO should have the same id as the entity'); - assert.equal(dto.title, null, 'DTO should return null if nullable property of the entity is unassigned'); + await collectionsService.addPostToCollection(collection.id, posts[0]); + + const collections = await collectionsService.getCollectionsForPost(posts[0].id); + + assert.equal(collections.length, 1, 'There should be one collection'); + assert.equal(collections[0].id, collection.id, 'Collection should be the correct one'); }); }); diff --git a/ghost/core/core/server/api/endpoints/collections.js b/ghost/core/core/server/api/endpoints/collections.js index 9c57dc1078..32ec8ffdb2 100644 --- a/ghost/core/core/server/api/endpoints/collections.js +++ b/ghost/core/core/server/api/endpoints/collections.js @@ -19,7 +19,7 @@ module.exports = { // @NOTE: should have permissions when moving out of Alpha permissions: false, query(frame) { - return collectionsService.api.browse(frame.options); + return collectionsService.api.getAll(frame.options); } }, @@ -30,7 +30,7 @@ module.exports = { // @NOTE: should have permissions when moving out of Alpha permissions: false, async query(frame) { - const model = await collectionsService.api.read(frame.data.id, frame.options); + const model = await collectionsService.api.getById(frame.data.id); if (!model) { throw new errors.NotFoundError({ @@ -50,7 +50,7 @@ module.exports = { // @NOTE: should have permissions when moving out of Alpha permissions: false, async query(frame) { - return await collectionsService.api.add(frame.data.collections[0], frame.options); + return await collectionsService.api.createCollection(frame.data.collections[0]); } }, @@ -71,7 +71,7 @@ module.exports = { async query(frame) { const model = await collectionsService.api.edit(Object.assign(frame.data.collections[0], { id: frame.options.id - }), frame.options); + })); if (!model) { throw new errors.NotFoundError({ @@ -117,7 +117,7 @@ module.exports = { // @NOTE: should have permissions when moving out of Alpha permissions: false, async query(frame) { - const collectionPost = await collectionsService.api.addPost(frame.options.id, { + const collectionPost = await collectionsService.api.addPostToCollection(frame.options.id, { id: frame.data.posts[0].id }); @@ -176,7 +176,7 @@ module.exports = { // @NOTE: should have permissions when moving out of Alpha permissions: false, async query(frame) { - const collection = await collectionsService.api.destroyCollectionPost(frame.options.id, frame.options.post_id); + const collection = await collectionsService.api.removePostFromCollection(frame.options.id, frame.options.post_id); if (!collection) { throw new errors.NotFoundError({ diff --git a/ghost/core/core/server/api/endpoints/posts.js b/ghost/core/core/server/api/endpoints/posts.js index 8fbaa2e700..69406301cf 100644 --- a/ghost/core/core/server/api/endpoints/posts.js +++ b/ghost/core/core/server/api/endpoints/posts.js @@ -1,6 +1,4 @@ const models = require('../../models'); -const tpl = require('@tryghost/tpl'); -const errors = require('@tryghost/errors'); const getPostServiceInstance = require('../../services/posts/posts-service'); const allowedIncludes = [ 'tags', @@ -21,10 +19,6 @@ const allowedIncludes = [ ]; const unsafeAttrs = ['status', 'authors', 'visibility']; -const messages = { - postNotFound: 'Post not found.' -}; - const postsService = getPostServiceInstance(); module.exports = { @@ -118,16 +112,7 @@ module.exports = { unsafeAttrs: unsafeAttrs }, query(frame) { - return models.Post.findOne(frame.data, frame.options) - .then((model) => { - if (!model) { - throw new errors.NotFoundError({ - message: tpl(messages.postNotFound) - }); - } - - return model; - }); + return postsService.readPost(frame); } }, diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/posts.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/posts.js index 5dda3654c0..ea53960ef5 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/posts.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/posts.js @@ -24,7 +24,19 @@ const labs = require('../../../../../../../shared/labs'); module.exports = async (model, frame, options = {}) => { const {tiers: tiersData} = options || {}; - const jsonModel = model.toJSON(frame.options); + // NOTE: `model` is now overloaded and may be a bookshelf model or a POJO + let jsonModel = model; + if (typeof model.toJSON === 'function') { + jsonModel = model.toJSON(frame.options); + } else { + // This is to satisy the interface which extraAttrs needs + model = { + id: jsonModel.id, + get(attr) { + return jsonModel[attr]; + } + }; + } // Map email_recipient_filter to email_segment jsonModel.email_segment = jsonModel.email_recipient_filter; diff --git a/ghost/core/core/server/services/collections/index.js b/ghost/core/core/server/services/collections/index.js index ee72847a03..32b1b4bee2 100644 --- a/ghost/core/core/server/services/collections/index.js +++ b/ghost/core/core/server/services/collections/index.js @@ -4,6 +4,7 @@ const { } = require('@tryghost/collections'); class CollectionsServiceWrapper { + /** @type {CollectionsService} */ api; constructor() { @@ -34,23 +35,15 @@ class CollectionsServiceWrapper { }); } - this.api = { - browse: collectionsService.getAll.bind(collectionsService), - read: collectionsService.getById.bind(collectionsService), - add: collectionsService.createCollection.bind(collectionsService), - edit: collectionsService.edit.bind(collectionsService), - addPost: collectionsService.addPostToCollection.bind(collectionsService), - destroy: collectionsService.destroy.bind(collectionsService), - destroyCollectionPost: collectionsService.removePostFromCollection.bind(collectionsService) - }; + this.api = collectionsService; } async init() { - const featuredCollections = await this.api.browse({filter: 'slug:featured'}); + const featuredCollections = await this.api.getAll({filter: 'slug:featured'}); if (!featuredCollections.data.length) { require('./built-in-collections').forEach((collection) => { - this.api.add(collection); + this.api.createCollection(collection); }); } } diff --git a/ghost/core/core/server/services/posts/posts-service.js b/ghost/core/core/server/services/posts/posts-service.js index 0dc40e4a3f..0824941ff9 100644 --- a/ghost/core/core/server/services/posts/posts-service.js +++ b/ghost/core/core/server/services/posts/posts-service.js @@ -12,6 +12,7 @@ const getPostServiceInstance = () => { const emailService = require('../email-service'); const settingsCache = require('../../../shared/settings-cache'); const settingsHelpers = require('../settings-helpers'); + const collectionsService = require('../collections'); const postStats = new PostStats(); @@ -37,7 +38,8 @@ const getPostServiceInstance = () => { isSet: flag => labs.isSet(flag), // don't use bind, that breaks test subbing of labs stats: postStats, emailService: emailService.service, - postsExporter + postsExporter, + collectionsService: collectionsService.api }); }; diff --git a/ghost/core/test/unit/server/services/collections/CollectionsServiceWrapper.test.js b/ghost/core/test/unit/server/services/collections/CollectionsServiceWrapper.test.js index a279394fc0..582fef94af 100644 --- a/ghost/core/test/unit/server/services/collections/CollectionsServiceWrapper.test.js +++ b/ghost/core/test/unit/server/services/collections/CollectionsServiceWrapper.test.js @@ -1,18 +1,11 @@ const assert = require('assert'); const collectionsServiceWrapper = require('../../../../../core/server/services/collections'); +const {CollectionsService} = require('@tryghost/collections'); describe('CollectionsServiceWrapper', function () { it('Exposes a valid instance of CollectionsServiceWrapper', async function () { assert.ok(collectionsServiceWrapper); assert.ok(collectionsServiceWrapper.api); - assert.deepEqual(Object.keys(collectionsServiceWrapper.api), [ - 'browse', - 'read', - 'add', - 'edit', - 'addPost', - 'destroy', - 'destroyCollectionPost' - ]); + assert.ok(collectionsServiceWrapper.api instanceof CollectionsService); }); }); diff --git a/ghost/posts-service/lib/PostsService.js b/ghost/posts-service/lib/PostsService.js index ea968f3160..e201fda9be 100644 --- a/ghost/posts-service/lib/PostsService.js +++ b/ghost/posts-service/lib/PostsService.js @@ -11,17 +11,37 @@ const messages = { invalidTiers: 'Invalid tiers value.', invalidTags: 'Invalid tags value.', invalidEmailSegment: 'The email segment parameter doesn\'t contain a valid filter', - unsupportedBulkAction: 'Unsupported bulk action' + unsupportedBulkAction: 'Unsupported bulk action', + postNotFound: 'Post not found.' }; class PostsService { - constructor({urlUtils, models, isSet, stats, emailService, postsExporter}) { + constructor({urlUtils, models, isSet, stats, emailService, postsExporter, collectionsService}) { this.urlUtils = urlUtils; this.models = models; this.isSet = isSet; this.stats = stats; this.emailService = emailService; this.postsExporter = postsExporter; + this.collectionsService = collectionsService; + } + + async readPost(frame) { + const model = await this.models.Post.findOne(frame.data, frame.options); + + if (!model) { + throw new errors.NotFoundError({ + message: tpl(messages.postNotFound) + }); + } + + const dto = model.toJSON(frame.options); + + if (this.isSet('collections') && frame?.original?.query?.include?.includes('collections')) { + dto.collections = await this.collectionsService.getCollectionsForPost(model.id); + } + + return dto; } async editPost(frame) {