Wired up LimitService to NewsletterService (#14602)

refs https://github.com/TryGhost/Team/issues/1549

This allows us to restrict certain sites to a single newsletter

core/server/services/newsletters/index.js

@@ -3,6 +3,7 @@ const SingleUseTokenProvider = require('../members/SingleUseTokenProvider');
 const mail = require('../mail');
 const models = require('../../models');
 const urlUtils = require('../../../shared/url-utils');
+const limitService = require('../limits');
 
 const MAGIC_LINK_TOKEN_VALIDITY = 24 * 60 * 60 * 1000;
 
@@ -11,5 +12,6 @@ module.exports = new NewslettersService({
     MemberModel: models.Member,
     mail,
     singleUseTokenProvider: new SingleUseTokenProvider(models.SingleUseToken, MAGIC_LINK_TOKEN_VALIDITY),
-    urlUtils
+    urlUtils,
+    limitService
 });

core/server/services/newsletters/service.js

@@ -13,11 +13,14 @@ class NewslettersService {
      * @param {Object} options.mail
      * @param {Object} options.singleUseTokenProvider
      * @param {Object} options.urlUtils
+     * @param {ILimitService} options.limitService
      */
-    constructor({NewsletterModel, MemberModel, mail, singleUseTokenProvider, urlUtils}) {
+    constructor({NewsletterModel, MemberModel, mail, singleUseTokenProvider, urlUtils, limitService}) {
         this.NewsletterModel = NewsletterModel;
         this.MemberModel = MemberModel;
         this.urlUtils = urlUtils;
+        /** @private */
+        this.limitService = limitService;
 
         /* email verification setup */
 
@@ -96,6 +99,8 @@ class NewslettersService {
             });
         }
 
+        await this.limitService.errorIfWouldGoOverLimit('newsletters');
+
         // remove any email properties that are not allowed to be set without verification
         const {cleanedAttrs, emailsToVerify} = await this.prepAttrsForEmailVerification(attrs);
 
@@ -242,4 +247,9 @@ class NewslettersService {
     }
 }
 
+/**
+ * @typedef {object} ILimitService
+ * @prop {(name: string) => Promise<void>} errorIfWouldGoOverLimit
+ **/
+
 module.exports = NewslettersService;

package.json

@@ -80,7 +80,7 @@
     "@tryghost/kg-default-cards": "5.16.2",
     "@tryghost/kg-markdown-html-renderer": "5.1.5",
     "@tryghost/kg-mobiledoc-html-renderer": "5.3.5",
-    "@tryghost/limit-service": "1.0.11",
+    "@tryghost/limit-service": "1.1.0",
     "@tryghost/logging": "2.1.8",
     "@tryghost/magic-link": "1.0.21",
     "@tryghost/member-events": "0.4.1",

test/e2e-api/admin/__snapshots__/newsletters.test.js.snap

@@ -654,3 +654,22 @@ Object {
   ],
 }
 `;
+
+exports[`Newsletters API Host Settings: newsletter limits Request fails when newsletter limit is in place 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": "Nuh uh",
+      "details": Object {
+        "name": "newsletters",
+      },
+      "help": "https://ghost.org/help/",
+      "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+      "message": "Host Limit error, cannot save newsletter.",
+      "property": null,
+      "type": "HostLimitError",
+    },
+  ],
+}
+`;

test/e2e-api/admin/newsletters.test.js

@@ -1,5 +1,6 @@
 const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
 const {anyEtag, anyObjectId, anyUuid, anyISODateTime, anyLocationFor} = matchers;
+const configUtils = require('../../utils/configUtils');
 const uuid = require('uuid');
 
 const assert = require('assert');
@@ -261,4 +262,37 @@ describe('Newsletters API', function () {
                 newsletters: [newsletterSnapshot]
             });
     });
+
+    describe('Host Settings: newsletter limits', function () {
+        afterEach(function () {
+            configUtils.set('hostSettings:limits', undefined);
+        });
+
+        it('Request fails when newsletter limit is in place', async function () {
+            configUtils.set('hostSettings:limits', {
+                newsletters: {
+                    disabled: true,
+                    error: 'Nuh uh'
+                }
+            });
+
+            agent = await agentProvider.getAdminAPIAgent();
+            await fixtureManager.init('newsletters', 'members:newsletters');
+            await agent.loginAsOwner();
+
+            const newsletter = {
+                name: 'Naughty newsletter'
+            };
+
+            await agent
+                .post(`newsletters/?opt_in_existing=true`)
+                .body({newsletters: [newsletter]})
+                .expectStatus(403)
+                .matchBodySnapshot({
+                    errors: [{
+                        id: anyUuid
+                    }]
+                });
+        });
+    });
 });

test/unit/server/services/newsletters/service.test.js

@@ -23,18 +23,25 @@ class TestTokenProvider {
 
 describe('NewslettersService', function () {
     let newsletterService, getStub, tokenProvider;
+    /** @type {NewslettersService.ILimitService} */
+    let limitService;
 
     before(function () {
         models.init();
 
         tokenProvider = new TestTokenProvider();
 
+        limitService = {
+            async errorIfWouldGoOverLimit() {}
+        };
+
         newsletterService = new NewslettersService({
             NewsletterModel: models.Newsletter,
             MemberModel: models.Member,
             mail,
             singleUseTokenProvider: tokenProvider,
-            urlUtils: urlUtils.stubUrlUtilsFromConfig()
+            urlUtils: urlUtils.stubUrlUtilsFromConfig(),
+            limitService
         });
     });
 
@@ -74,6 +81,22 @@ describe('NewslettersService', function () {
             getNextAvailableSortOrderStub = sinon.stub(models.Newsletter, 'getNextAvailableSortOrder').returns(1);
         });
 
+        it('rejects if the limit services determines it would be over the limit', async function () {
+            const error = new Error('No way, Jose!');
+            sinon.stub(limitService, 'errorIfWouldGoOverLimit').rejects(error);
+
+            let thrownError;
+            try {
+                await newsletterService.add({
+                    name: 'Newsletter Name'
+                });
+            } catch (err) {
+                thrownError = err;
+            }
+            assert(thrownError, 'It should have thrown an error');
+            assert(thrownError === error, 'It should have rethrown the error from limit service');
+        });
+
         it('rejects if called with no data', async function () {
             assert.rejects(await newsletterService.add, {name: 'TypeError'});
             sinon.assert.notCalled(addStub);

yarn.lock

@@ -2091,10 +2091,10 @@
   dependencies:
     semver "^7.3.5"
 
-"@tryghost/limit-service@1.0.11":
-  version "1.0.11"
-  resolved "https://registry.yarnpkg.com/@tryghost/limit-service/-/limit-service-1.0.11.tgz#93424eac034dacd6bcf8ab661f121eae55bf14ea"
-  integrity sha512-Xuh7rUlmy/nLwDuLAJQOBPTnLHWKZoyHsEnFT7SgA4ra2ZqSDcWECpFW6QyARdB1AXpqCXkT8QGhXkTyytAtUg==
+"@tryghost/limit-service@1.1.0":
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/@tryghost/limit-service/-/limit-service-1.1.0.tgz#12af2d0b8242212871be8cf893e2563466c25ec2"
+  integrity sha512-CC0/Ly1KIVIiYMKwqWfgKPkFoGssyyPjwcdS4XzJ3KyqdwEckzPX1e2jZMzptZlMqDuAa4uZ0xSf5VH4YZ71+Q==
   dependencies:
     "@tryghost/errors" "^1.2.1"
     lodash "^4.17.21"