Refactored TiersAPI to use core slug generation

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

This removes the burden from the Tier object, and allows us to reuse
the existing slug generation implementation we have in our bookshelf
models.

ghost/tiers/lib/InMemoryTierRepository.js

@@ -47,16 +47,6 @@ class InMemoryTierRepository {
         });
     }
 
-    /**
-     * @param {string} slug
-     * @returns {Promise<Tier>}
-     */
-    async getBySlug(slug) {
-        return this.#store.find((item) => {
-            return item.slug === slug;
-        });
-    }
-
     /**
      * @param {object} [options]
      * @param {string} [options.filter]

ghost/tiers/lib/Tier.js

@@ -162,10 +162,9 @@ module.exports = class Tier {
 
     /**
      * @param {any} data
-     * @param {ISlugService} slugService
      * @returns {Promise<Tier>}
      */
-    static async create(data, slugService) {
+    static async create(data) {
         let id;
         if (!data.id) {
             id = new ObjectID();
@@ -181,13 +180,7 @@ module.exports = class Tier {
 
         let name = validateName(data.name);
 
-        let slug;
-        if (data.slug) {
-            slug = await slugService.validate(data.slug);
-        } else {
-            slug = await slugService.generate(name);
-        }
-
+        let slug = validateSlug(data.slug);
         let description = validateDescription(data.description);
         let welcomePageURL = validateWelcomePageURL(data.welcome_page_url);
         let status = validateStatus(data.status || 'active');
@@ -221,6 +214,15 @@ module.exports = class Tier {
     }
 };
 
+function validateSlug(value) {
+    if (!value || typeof value !== 'string' || value.length > 191) {
+        throw new ValidationError({
+            message: 'Tier slug must be a string with a maximum of 191 characters'
+        });
+    }
+    return value;
+}
+
 function validateName(value) {
     if (typeof value !== 'string') {
         throw new ValidationError({

ghost/tiers/lib/TierSlugService.js

@@ -1,33 +0,0 @@
-const {ValidationError} = require('@tryghost/errors');
-const {slugify} = require('@tryghost/string');
-
-module.exports = class TierSlugService {
-    /** @type {import('./TiersAPI').ITierRepository} */
-    #repository;
-
-    constructor(deps) {
-        this.#repository = deps.repository;
-    }
-
-    async validate(slug) {
-        const exists = !!(await this.#repository.getBySlug(slug));
-
-        if (!exists) {
-            return slug;
-        }
-
-        throw new ValidationError({
-            message: 'Slug already exists'
-        });
-    }
-
-    async generate(input, n = 0) {
-        const slug = slugify(input + (n ? n : ''));
-
-        try {
-            return await this.validate(slug);
-        } catch (err) {
-            return this.generate(input, n + 1);
-        }
-    }
-};

ghost/tiers/lib/TiersAPI.js

@@ -1,16 +1,19 @@
 const ObjectID = require('bson-objectid').default;
 const {BadRequestError} = require('@tryghost/errors');
 const Tier = require('./Tier');
-const TierSlugService = require('./TierSlugService');
 
 /**
  * @typedef {object} ITierRepository
  * @prop {(id: ObjectID) => Promise<Tier>} getById
- * @prop {(slug: string) => Promise<Tier>} getBySlug
  * @prop {(tier: Tier) => Promise<void>} save
  * @prop {(options?: {filter?: string}) => Promise<Tier[]>} getAll
  */
 
+/**
+ * @typedef {object} ISlugService
+ * @prop {(input: string) => Promise<string>} generate
+ */
+
 /**
  * @template {Model}
  * @typedef {object} Page<Model>
@@ -29,14 +32,12 @@ module.exports = class TiersAPI {
     /** @type {ITierRepository} */
     #repository;
 
-    /** @type {TierSlugService} */
+    /** @type {ISlugService} */
     #slugService;
 
     constructor(deps) {
         this.#repository = deps.repository;
-        this.#slugService = new TierSlugService({
-            repository: deps.repository
-        });
+        this.#slugService = deps.slugService;
     }
 
     /**
@@ -116,7 +117,10 @@ module.exports = class TiersAPI {
                 message: 'Cannot create free Tier'
             });
         }
+
+        const slug = await this.#slugService.generate(data.slug || data.name);
         const tier = await Tier.create({
+            slug,
             type: 'paid',
             status: 'active',
             visibility: data.visibility,
@@ -128,7 +132,7 @@ module.exports = class TiersAPI {
             yearly_price: data.yearly_price,
             currency: data.currency,
             trial_days: data.trial_days
-        }, this.#slugService);
+        });
 
         await this.#repository.save(tier);
 

ghost/tiers/test/Tier.test.js

@@ -35,6 +35,7 @@ const invalidInputs = [
     {id: [100]},
     {name: 100},
     {name: ('a').repeat(200)},
+    {slug: ('slug').repeat(50)},
     {description: ['whatever?']},
     {description: ('b').repeat(200)},
     {welcome_page_url: 'hello world'},
@@ -77,7 +78,7 @@ describe('Tier', function () {
                 let input = {};
                 Object.assign(input, validInput, invalidInput);
                 await assertError(async function () {
-                    await Tier.create(input, {validate: x => x, generate: x => x});
+                    await Tier.create(input);
                 });
             }
         });
@@ -86,12 +87,12 @@ describe('Tier', function () {
             for (const validInputItem of validInputs) {
                 let input = {};
                 Object.assign(input, validInput, validInputItem);
-                await Tier.create(input, {validate: x => x, generate: x => x});
+                await Tier.create(input);
             }
         });
 
         it('Can create a Tier with valid input', async function () {
-            const tier = await Tier.create(validInput, {validate: x => x, generate: x => x});
+            const tier = await Tier.create(validInput);
 
             const expectedProps = [
                 'id',
@@ -117,7 +118,7 @@ describe('Tier', function () {
         });
 
         it('Errors when attempting to set invalid properties', async function () {
-            const tier = await Tier.create(validInput, {validate: x => x, generate: x => x});
+            const tier = await Tier.create(validInput);
 
             assertError(() => {
                 tier.name = 20;

ghost/tiers/test/TiersAPI.test.js

@@ -12,7 +12,12 @@ describe('TiersAPI', function () {
     before(function () {
         repository = new InMemoryTierRepository();
         api = new TiersAPI({
-            repository
+            repository,
+            slugService: {
+                async generate(input) {
+                    return input;
+                }
+            }
         });
     });