diff --git a/core/server/api/canary/posts.js b/core/server/api/canary/posts.js index 26bab92fca..2d198d9d5d 100644 --- a/core/server/api/canary/posts.js +++ b/core/server/api/canary/posts.js @@ -6,8 +6,6 @@ const {mega} = require('../../services/mega'); const membersService = require('../../services/members'); const allowedIncludes = ['tags', 'authors', 'authors.roles', 'email']; const unsafeAttrs = ['status', 'authors', 'visibility']; -const _ = require('lodash'); -const config = require('../../../shared/config'); module.exports = { docName: 'posts', @@ -150,23 +148,9 @@ module.exports = { unsafeAttrs: unsafeAttrs }, async query(frame) { - /**Check host limits for members when send email is true*/ - const membersHostLimit = config.get('host_settings:limits:members'); - if (frame.options.send_email_when_published && membersHostLimit) { - const allowedMembersLimit = membersHostLimit.max; - const hostUpgradeLink = config.get('host_settings:limits').upgrade_url; - const knexOptions = _.pick(frame.options, ['transacting', 'forUpdate']); - const {members} = await membersService.api.members.list(Object.assign(knexOptions, {filter: 'subscribed:true'}, {limit: 'all'})); - if (members.length > allowedMembersLimit) { - throw new errors.HostLimitError({ - message: `Your current plan allows you to send email to up to ${allowedMembersLimit} members, but you currently have ${members.length} members`, - help: hostUpgradeLink, - errorDetails: { - limit: allowedMembersLimit, - total: members.length - } - }); - } + /**Check host limits for members when send email is true**/ + if (frame.options.send_email_when_published) { + await membersService.checkHostLimit(); } let model = await models.Post.edit(frame.data.posts[0], frame.options); diff --git a/core/server/services/mega/mega.js b/core/server/services/mega/mega.js index 9d7c420742..edc9c87de1 100644 --- a/core/server/services/mega/mega.js +++ b/core/server/services/mega/mega.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const debug = require('ghost-ignition').debug('mega'); const url = require('url'); const moment = require('moment'); const errors = require('@tryghost/errors'); @@ -8,7 +9,6 @@ const membersService = require('../members'); const bulkEmailService = require('../bulk-email'); const models = require('../../models'); const postEmailSerializer = require('./post-email-serializer'); -const config = require('../../../shared/config'); const getEmailData = async (postModel, members = []) => { const {emailTmpl, replacements} = await postEmailSerializer.serialize(postModel); @@ -81,10 +81,13 @@ const sendTestEmail = async (postModel, toEmails) => { const addEmail = async (postModel, options) => { const knexOptions = _.pick(options, ['transacting', 'forUpdate']); + // @TODO: improve performance of this members.list call + debug('addEmail: retrieving members list'); const {members} = await membersService.api.members.list(Object.assign(knexOptions, {filter: 'subscribed:true'}, {limit: 'all'})); const membersToSendTo = members.filter((member) => { return membersService.contentGating.checkPostAccess(postModel.toJSON(), member); }); + debug('addEmail: retrieved members list'); // NOTE: don't create email object when there's nobody to send the email to if (!membersToSendTo.length) { @@ -180,24 +183,6 @@ async function handleUnsubscribeRequest(req) { } } -function checkHostLimitForMembers(members = []) { - const membersHostLimit = config.get('host_settings:limits:members'); - if (membersHostLimit) { - const allowedMembersLimit = membersHostLimit.max; - const hostUpgradeLink = config.get('host_settings:limits').upgrade_url; - if (members.length > allowedMembersLimit) { - throw new errors.HostLimitError({ - message: `Your current plan allows you to send email to up to ${allowedMembersLimit} members, but you currently have ${members.length} members`, - help: hostUpgradeLink, - errorDetails: { - limit: allowedMembersLimit, - total: members.length - } - }); - } - } -} - async function pendingEmailHandler(emailModel, options) { // CASE: do not send email if we import a database // TODO: refactor post.published events to never fire on importing @@ -210,24 +195,29 @@ async function pendingEmailHandler(emailModel, options) { return; } - const {members} = await membersService.api.members.list(Object.assign({filter: 'subscribed:true'}, {limit: 'all'})); - - if (!members.length) { - return; - } - - await models.Email.edit({ - status: 'submitting' - }, { - id: emailModel.id - }); - let meta = []; let error = null; try { // Check host limit for allowed member count and throw error if over limit - checkHostLimitForMembers(members); + await membersService.checkHostLimit(); + + // No need to fetch list until after we've passed the check + // @TODO: improve performance of this members.list call + debug('pendingEmailHandler: retrieving members list'); + const {members} = await membersService.api.members.list(Object.assign({filter: 'subscribed:true'}, {limit: 'all'})); + debug('pendingEmailHandler: retrieved members list'); + + if (!members.length) { + return; + } + + await models.Email.edit({ + status: 'submitting' + }, { + id: emailModel.id + }); + // NOTE: meta can contains an array which can be a mix of successful and error responses // needs filtering and saving objects of {error, batchData} form to separate property meta = await sendEmail(postModel, members); diff --git a/core/server/services/members/index.js b/core/server/services/members/index.js index 527ba48774..f3b3e0819f 100644 --- a/core/server/services/members/index.js +++ b/core/server/services/members/index.js @@ -58,6 +58,8 @@ events.on('settings.edited', function updateSettingFromModel(settingModel) { const membersService = { contentGating: require('./content-gating'), + checkHostLimit: require('./limit'), + config: membersConfig, get api() { diff --git a/core/server/services/members/limit.js b/core/server/services/members/limit.js new file mode 100644 index 0000000000..3eb40717a5 --- /dev/null +++ b/core/server/services/members/limit.js @@ -0,0 +1,32 @@ +const config = require('../../../shared/config'); +const db = require('../../data/db'); +const errors = require('@tryghost/errors'); + +// Get total members direct from DB +// @TODO: determine performance difference between this, normal knex, and using the model layer +async function getTotalMembers() { + const isSQLite = config.get('database:client') === 'sqlite3'; + const result = await db.knex.raw('SELECT COUNT(id) AS total FROM members'); + return isSQLite ? result[0].total : result[0][0].total; +} + +module.exports = async () => { + const membersHostLimit = config.get('host_settings:limits:members'); + if (membersHostLimit) { + const allowedMembersLimit = membersHostLimit.max; + const hostUpgradeLink = config.get('host_settings:limits').upgrade_url; + + const totalMembers = await getTotalMembers(); + + if (totalMembers > allowedMembersLimit) { + throw new errors.HostLimitError({ + message: `Your current plan allows you to send email to up to ${allowedMembersLimit} members, but you currently have ${totalMembers} members`, + help: hostUpgradeLink, + errorDetails: { + limit: allowedMembersLimit, + total: totalMembers + } + }); + } + } +};