From a5dff4207e35d594fafeaa8e72d9c6f62b8f88a5 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Mon, 20 Mar 2023 16:05:21 +0100 Subject: [PATCH] Moved posts service to its own package fixes https://github.com/TryGhost/Team/issues/2778 It is easier to add extra classes using the latest patterns if it has its own package. --- .../server/services/posts/posts-service.js | 120 +--------------- .../services/posts/posts-service.test.js | 25 ---- ghost/posts-service/.eslintrc.js | 6 + ghost/posts-service/README.md | 2 + ghost/posts-service/index.js | 3 + ghost/posts-service/lib/PostsService.js | 119 ++++++++++++++++ ghost/posts-service/package.json | 30 ++++ ghost/posts-service/test/.eslintrc.js | 6 + ghost/posts-service/test/PostsService.test.js | 42 ++++++ ghost/posts-service/test/utils/index.js | 133 ++++++++++++++++++ 10 files changed, 343 insertions(+), 143 deletions(-) delete mode 100644 ghost/core/test/unit/server/services/posts/posts-service.test.js create mode 100644 ghost/posts-service/.eslintrc.js create mode 100644 ghost/posts-service/README.md create mode 100644 ghost/posts-service/index.js create mode 100644 ghost/posts-service/lib/PostsService.js create mode 100644 ghost/posts-service/package.json create mode 100644 ghost/posts-service/test/.eslintrc.js create mode 100644 ghost/posts-service/test/PostsService.test.js create mode 100644 ghost/posts-service/test/utils/index.js diff --git a/ghost/core/core/server/services/posts/posts-service.js b/ghost/core/core/server/services/posts/posts-service.js index d4aa28d221..841da29624 100644 --- a/ghost/core/core/server/services/posts/posts-service.js +++ b/ghost/core/core/server/services/posts/posts-service.js @@ -1,123 +1,7 @@ -const nql = require('@tryghost/nql'); -const {BadRequestError} = require('@tryghost/errors'); -const tpl = require('@tryghost/tpl'); - -const messages = { - invalidVisibilityFilter: 'Invalid visibility filter.', - invalidEmailSegment: 'The email segment parameter doesn\'t contain a valid filter' -}; - -class PostsService { - constructor({urlUtils, models, isSet, stats, emailService}) { - this.urlUtils = urlUtils; - this.models = models; - this.isSet = isSet; - this.stats = stats; - this.emailService = emailService; - } - - async editPost(frame) { - // Make sure the newsletter is matching an active newsletter - // Note that this option is simply ignored if the post isn't published or scheduled - if (frame.options.newsletter && frame.options.email_segment) { - if (frame.options.email_segment !== 'all') { - // check filter is valid - try { - await this.models.Member.findPage({filter: frame.options.email_segment, limit: 1}); - } catch (err) { - return Promise.reject(new BadRequestError({ - message: tpl(messages.invalidEmailSegment), - context: err.message - })); - } - } - } - - const model = await this.models.Post.edit(frame.data.posts[0], frame.options); - - /**Handle newsletter email */ - if (model.get('newsletter_id')) { - const sendEmail = model.wasChanged() && this.shouldSendEmail(model.get('status'), model.previous('status')); - - if (sendEmail) { - let postEmail = model.relations.email; - let email; - - if (!postEmail) { - email = await this.emailService.createEmail(model); - } else if (postEmail && postEmail.get('status') === 'failed') { - email = await this.emailService.retryEmail(postEmail); - } - if (email) { - model.set('email', email); - } - } - } - - return model; - } - - async getProductsFromVisibilityFilter(visibilityFilter) { - try { - const allProducts = await this.models.Product.findAll(); - const visibilityFilterJson = nql(visibilityFilter).toJSON(); - const productsData = (visibilityFilterJson.product ? [visibilityFilterJson] : visibilityFilterJson.$or) || []; - const tiers = productsData - .map((data) => { - return allProducts.find((p) => { - return p.get('slug') === data.product; - }); - }).filter(p => !!p).map((d) => { - return d.toJSON(); - }); - return tiers; - } catch (err) { - return Promise.reject(new BadRequestError({ - message: tpl(messages.invalidVisibilityFilter), - context: err.message - })); - } - } - - /** - * Calculates if the email should be tried to be sent out - * @private - * @param {String} currentStatus current status from the post model - * @param {String} previousStatus previous status from the post model - * @returns {Boolean} - */ - shouldSendEmail(currentStatus, previousStatus) { - return (['published', 'sent'].includes(currentStatus)) - && (!['published', 'sent'].includes(previousStatus)); - } - - handleCacheInvalidation(model) { - let cacheInvalidate; - - if ( - model.get('status') === 'published' && model.wasChanged() || - model.get('status') === 'draft' && model.previous('status') === 'published' - ) { - cacheInvalidate = true; - } else if ( - model.get('status') === 'draft' && model.previous('status') !== 'published' || - model.get('status') === 'scheduled' && model.wasChanged() - ) { - cacheInvalidate = { - value: this.urlUtils.urlFor({ - relativeUrl: this.urlUtils.urlJoin('/p', model.get('uuid'), '/') - }) - }; - } else { - cacheInvalidate = false; - } - - return cacheInvalidate; - } -} +const {PostsService} = require('@tryghost/posts-service'); /** - * @returns {PostsService} instance of the PostsService + * @returns {InstanceType} instance of the PostsService */ const getPostServiceInstance = () => { const urlUtils = require('../../../shared/url-utils'); diff --git a/ghost/core/test/unit/server/services/posts/posts-service.test.js b/ghost/core/test/unit/server/services/posts/posts-service.test.js deleted file mode 100644 index c3d9bbe9d0..0000000000 --- a/ghost/core/test/unit/server/services/posts/posts-service.test.js +++ /dev/null @@ -1,25 +0,0 @@ -const should = require('should'); - -const {PostsService} = require('../../../../../core/server/services/posts/posts-service'); - -describe('PostsService', function () { - describe('shouldSendEmail', function () { - it('calculates if an email should be sent', async function () { - const postsService = new PostsService({}); - - postsService.shouldSendEmail('published', 'draft').should.be.true(); - postsService.shouldSendEmail('published', 'scheduled').should.be.true(); - postsService.shouldSendEmail('sent', 'draft').should.be.true(); - postsService.shouldSendEmail('sent', 'scheduled').should.be.true(); - - postsService.shouldSendEmail('published', 'published').should.be.false(); - postsService.shouldSendEmail('published', 'sent').should.be.false(); - postsService.shouldSendEmail('published', 'published').should.be.false(); - postsService.shouldSendEmail('published', 'sent').should.be.false(); - postsService.shouldSendEmail('sent', 'published').should.be.false(); - postsService.shouldSendEmail('sent', 'sent').should.be.false(); - - postsService.shouldSendEmail().should.be.false(); - }); - }); -}); diff --git a/ghost/posts-service/.eslintrc.js b/ghost/posts-service/.eslintrc.js new file mode 100644 index 0000000000..c9c1bcb522 --- /dev/null +++ b/ghost/posts-service/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + plugins: ['ghost'], + extends: [ + 'plugin:ghost/node' + ] +}; diff --git a/ghost/posts-service/README.md b/ghost/posts-service/README.md new file mode 100644 index 0000000000..4e15015f0b --- /dev/null +++ b/ghost/posts-service/README.md @@ -0,0 +1,2 @@ +# Posts Service + diff --git a/ghost/posts-service/index.js b/ghost/posts-service/index.js new file mode 100644 index 0000000000..21b85b949e --- /dev/null +++ b/ghost/posts-service/index.js @@ -0,0 +1,3 @@ +module.exports = { + PostsService: require('./lib/PostsService') +}; diff --git a/ghost/posts-service/lib/PostsService.js b/ghost/posts-service/lib/PostsService.js new file mode 100644 index 0000000000..ac0c12edd1 --- /dev/null +++ b/ghost/posts-service/lib/PostsService.js @@ -0,0 +1,119 @@ +const nql = require('@tryghost/nql'); +const {BadRequestError} = require('@tryghost/errors'); +const tpl = require('@tryghost/tpl'); + +const messages = { + invalidVisibilityFilter: 'Invalid visibility filter.', + invalidEmailSegment: 'The email segment parameter doesn\'t contain a valid filter' +}; + +class PostsService { + constructor({urlUtils, models, isSet, stats, emailService}) { + this.urlUtils = urlUtils; + this.models = models; + this.isSet = isSet; + this.stats = stats; + this.emailService = emailService; + } + + async editPost(frame) { + // Make sure the newsletter is matching an active newsletter + // Note that this option is simply ignored if the post isn't published or scheduled + if (frame.options.newsletter && frame.options.email_segment) { + if (frame.options.email_segment !== 'all') { + // check filter is valid + try { + await this.models.Member.findPage({filter: frame.options.email_segment, limit: 1}); + } catch (err) { + return Promise.reject(new BadRequestError({ + message: tpl(messages.invalidEmailSegment), + context: err.message + })); + } + } + } + + const model = await this.models.Post.edit(frame.data.posts[0], frame.options); + + /**Handle newsletter email */ + if (model.get('newsletter_id')) { + const sendEmail = model.wasChanged() && this.shouldSendEmail(model.get('status'), model.previous('status')); + + if (sendEmail) { + let postEmail = model.relations.email; + let email; + + if (!postEmail) { + email = await this.emailService.createEmail(model); + } else if (postEmail && postEmail.get('status') === 'failed') { + email = await this.emailService.retryEmail(postEmail); + } + if (email) { + model.set('email', email); + } + } + } + + return model; + } + + async getProductsFromVisibilityFilter(visibilityFilter) { + try { + const allProducts = await this.models.Product.findAll(); + const visibilityFilterJson = nql(visibilityFilter).toJSON(); + const productsData = (visibilityFilterJson.product ? [visibilityFilterJson] : visibilityFilterJson.$or) || []; + const tiers = productsData + .map((data) => { + return allProducts.find((p) => { + return p.get('slug') === data.product; + }); + }).filter(p => !!p).map((d) => { + return d.toJSON(); + }); + return tiers; + } catch (err) { + return Promise.reject(new BadRequestError({ + message: tpl(messages.invalidVisibilityFilter), + context: err.message + })); + } + } + + /** + * Calculates if the email should be tried to be sent out + * @private + * @param {String} currentStatus current status from the post model + * @param {String} previousStatus previous status from the post model + * @returns {Boolean} + */ + shouldSendEmail(currentStatus, previousStatus) { + return (['published', 'sent'].includes(currentStatus)) + && (!['published', 'sent'].includes(previousStatus)); + } + + handleCacheInvalidation(model) { + let cacheInvalidate; + + if ( + model.get('status') === 'published' && model.wasChanged() || + model.get('status') === 'draft' && model.previous('status') === 'published' + ) { + cacheInvalidate = true; + } else if ( + model.get('status') === 'draft' && model.previous('status') !== 'published' || + model.get('status') === 'scheduled' && model.wasChanged() + ) { + cacheInvalidate = { + value: this.urlUtils.urlFor({ + relativeUrl: this.urlUtils.urlJoin('/p', model.get('uuid'), '/') + }) + }; + } else { + cacheInvalidate = false; + } + + return cacheInvalidate; + } +} + +module.exports = PostsService; diff --git a/ghost/posts-service/package.json b/ghost/posts-service/package.json new file mode 100644 index 0000000000..2be3578058 --- /dev/null +++ b/ghost/posts-service/package.json @@ -0,0 +1,30 @@ +{ + "name": "@tryghost/posts-service", + "version": "0.0.0", + "repository": "https://github.com/TryGhost/Ghost/tree/main/packages/posts-service", + "author": "Ghost Foundation", + "private": true, + "main": "index.js", + "scripts": { + "dev": "echo \"Implement me!\"", + "test:unit": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura mocha './test/**/*.test.js'", + "test": "yarn test:unit", + "lint:code": "eslint *.js lib/ --ext .js --cache", + "lint": "yarn lint:code && yarn lint:test", + "lint:test": "eslint -c test/.eslintrc.js test/ --ext .js --cache" + }, + "files": [ + "index.js", + "lib" + ], + "devDependencies": { + "c8": "7.13.0", + "mocha": "10.2.0", + "sinon": "15.0.2" + }, + "dependencies": { + "@tryghost/errors": "1.2.21", + "@tryghost/nql": "0.11.0", + "@tryghost/tpl": "0.1.22" + } +} diff --git a/ghost/posts-service/test/.eslintrc.js b/ghost/posts-service/test/.eslintrc.js new file mode 100644 index 0000000000..829b601eb0 --- /dev/null +++ b/ghost/posts-service/test/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + plugins: ['ghost'], + extends: [ + 'plugin:ghost/test' + ] +}; diff --git a/ghost/posts-service/test/PostsService.test.js b/ghost/posts-service/test/PostsService.test.js new file mode 100644 index 0000000000..fcfd55d5b5 --- /dev/null +++ b/ghost/posts-service/test/PostsService.test.js @@ -0,0 +1,42 @@ +const {PostsService} = require('../index'); +const assert = require('assert'); + +describe('Posts Service', function () { + it('Can construct class', function () { + new PostsService({}); + }); + + describe('shouldSendEmail', function () { + it('calculates if an email should be sent', async function () { + const postsService = new PostsService({}); + + assert.deepEqual([ + postsService.shouldSendEmail('published', 'draft'), + postsService.shouldSendEmail('published', 'scheduled'), + postsService.shouldSendEmail('sent', 'draft'), + postsService.shouldSendEmail('sent', 'scheduled'), + + postsService.shouldSendEmail('published', 'published'), + postsService.shouldSendEmail('published', 'sent'), + postsService.shouldSendEmail('published', 'published'), + postsService.shouldSendEmail('published', 'sent'), + postsService.shouldSendEmail('sent', 'published'), + postsService.shouldSendEmail('sent', 'sent'), + postsService.shouldSendEmail() + ], [ + true, + true, + true, + true, + + false, + false, + false, + false, + false, + false, + false + ]); + }); + }); +}); diff --git a/ghost/posts-service/test/utils/index.js b/ghost/posts-service/test/utils/index.js new file mode 100644 index 0000000000..f3d689a041 --- /dev/null +++ b/ghost/posts-service/test/utils/index.js @@ -0,0 +1,133 @@ +const ObjectId = require('bson-objectid').default; +const sinon = require('sinon'); + +const createModel = (propertiesAndRelations) => { + const id = propertiesAndRelations.id ?? ObjectId().toHexString(); + return { + id, + getLazyRelation: (relation) => { + propertiesAndRelations.loaded = propertiesAndRelations.loaded ?? []; + if (!propertiesAndRelations.loaded.includes(relation)) { + propertiesAndRelations.loaded.push(relation); + } + if (Array.isArray(propertiesAndRelations[relation])) { + return Promise.resolve({ + models: propertiesAndRelations[relation] + }); + } + return Promise.resolve(propertiesAndRelations[relation]); + }, + related: (relation) => { + if (!Object.keys(propertiesAndRelations).includes('loaded')) { + throw new Error(`Model.related('${relation}'): When creating a test model via createModel you must include 'loaded' to specify which relations are already loaded and useable via Model.related.`); + } + if (!propertiesAndRelations.loaded.includes(relation)) { + throw new Error(`Model.related('${relation}') was used on a test model that didn't explicitly loaded that relation.`); + } + return propertiesAndRelations[relation]; + }, + get: (property) => { + return propertiesAndRelations[property]; + }, + save: (properties) => { + Object.assign(propertiesAndRelations, properties); + return Promise.resolve(); + }, + toJSON: () => { + return { + id, + ...propertiesAndRelations + }; + } + }; +}; + +const createModelClass = (options = {}) => { + return { + ...options, + add: async (properties) => { + return Promise.resolve(createModel(properties)); + }, + findOne: async (data, o) => { + if (options.findOne === null && o.require) { + return Promise.reject(new Error('NotFound')); + } + if (options.findOne === null) { + return Promise.resolve(null); + } + return Promise.resolve( + createModel({...options.findOne, ...data}) + ); + }, + findAll: async (data) => { + return Promise.resolve( + (options.findAll ?? []).map(f => createModel({...f, ...data})) + ); + }, + transaction: async (callback) => { + const transacting = {transacting: 'transacting'}; + return await callback(transacting); + }, + where: function () { + return this; + }, + save: async function () { + return Promise.resolve(); + } + }; +}; + +const createDb = ({first, all} = {}) => { + let a = all; + const db = { + knex: function () { + return this; + }, + where: function () { + return this; + }, + whereNull: function () { + return this; + }, + select: function () { + return this; + }, + limit: function (n) { + a = all.slice(0, n); + return this; + }, + update: sinon.stub().resolves(), + orderByRaw: function () { + return this; + }, + insert: function () { + return this; + }, + first: () => { + return Promise.resolve(first); + }, + then: function (resolve) { + resolve(a); + }, + transacting: function () { + return this; + } + }; + db.knex.raw = function () { + return this; + }; + return db; +}; + +const sleep = (ms) => { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +}; + +module.exports = { + createModel, + createModelClass, + createDb, + sleep +};