Merge branch 'main' of https://github.com/TryGhost/Ghost into main

.github/workflows/auto-assign.yml

@@ -5,6 +5,7 @@ on:
 jobs:
   automate:
     runs-on: ubuntu-18.04
+    if: github.repository_owner == 'TryGhost'
     env:
       FORCE_COLOR: 1
       GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/label-actions.yml

@@ -9,5 +9,6 @@ on:
 jobs:
   action:
     runs-on: ubuntu-latest
+    if: github.repository_owner == 'TryGhost'
     steps:
       - uses: tryghost/label-actions@main

.github/workflows/migration-review.yml

@@ -8,6 +8,7 @@ on:
 jobs:
   createComment:
     runs-on: ubuntu-latest
+    if: github.repository_owner == 'TryGhost'
     name: Create checklist comment
     steps:
       - uses: peter-evans/create-or-update-comment@26f07868647c398ca3d2042238c8aa620ca5d311

.github/workflows/stale.yml

@@ -4,6 +4,7 @@ on:
     - cron: '0 15 * * *'
 jobs:
   stale:
+    if: github.repository_owner == 'TryGhost'
     runs-on: ubuntu-latest
     steps:
       - uses: actions/stale@v5

.github/workflows/test.yml

@@ -50,11 +50,6 @@ jobs:
             DB_CLIENT: mysql
     env:
       database__client: ${{ matrix.env.DB_CLIENT }}
-      database__connection__filename: /dev/shm/ghost-test.db
-      database__connection__host: 127.0.0.1
-      database__connection__user: root
-      database__connection__password: root
-      database__connection__database: ghost_testing
     name: Migrations (${{ matrix.env.DB }})
     steps:
       - uses: actions/checkout@v2
@@ -87,6 +82,18 @@ jobs:
           mysql database: 'ghost_testing'
           mysql root password: 'root'
 
+      - name: Set env vars (SQLite)
+        if: contains(matrix.env.DB, 'sqlite')
+        run: echo "database__connection__filename=/dev/shm/ghost-test.db" >> $GITHUB_ENV
+
+      - name: Set env vars (MySQL)
+        if: contains(matrix.env.DB, 'mysql')
+        run: |
+          echo "database__connection__host=127.0.0.1" >> $GITHUB_ENV
+          echo "database__connection__user=root" >> $GITHUB_ENV
+          echo "database__connection__password=root" >> $GITHUB_ENV
+          echo "database__connection__database=ghost_testing" >> $GITHUB_ENV
+
       - run: yarn
       - run: |
           node index.js &
@@ -148,7 +155,6 @@ jobs:
     env:
       DB: ${{ matrix.env.DB }}
       NODE_ENV: ${{ matrix.env.NODE_ENV }}
-      database__connection__password: root
     name: Database Tests (Node ${{ matrix.node }}, ${{ matrix.env.DB }})
     steps:
       - uses: actions/checkout@v2
@@ -182,10 +188,16 @@ jobs:
 
       - run: date +%s > ${{ runner.temp }}/startTime # Get start time for test suite
 
+      - name: Set env vars (SQLite)
+        if: contains(matrix.env.DB, 'sqlite')
+        run: echo "database__connection__filename=/dev/shm/ghost-test.db" >> $GITHUB_ENV
+
+      - name: Set env vars (MySQL)
+        if: contains(matrix.env.DB, 'mysql')
+        run: echo "database__connection__password=root" >> $GITHUB_ENV
+
       - name: Run tests
         run: yarn test:ci
-        env:
-          database__connection__filename: /dev/shm/ghost-test.db
 
       # Get runtime in seconds for test suite
       - run: |

core/server/data/importer/importers/data/products.js

@@ -1,19 +1,20 @@
 const _ = require('lodash');
 const BaseImporter = require('./base');
 const models = require('../../../../models');
+const debug = require('@tryghost/debug')('importer:products');
 
 class ProductsImporter extends BaseImporter {
     constructor(allDataFromFile) {
         super(allDataFromFile, {
             modelName: 'Product',
             dataKeyToImport: 'products',
             requiredFromFile: ['stripe_prices'],
-            requiredExistingData: ['stripe_prices']
+            requiredExistingData: ['stripe_prices', 'products']
         });
     }
 
     fetchExisting(modelOptions) {
-        return models.Product.findAll(_.merge({columns: ['products.id as id']}, modelOptions))
+        return models.Product.findAll(_.merge({columns: ['products.id as id', 'name', 'slug']}, modelOptions))
             .then((existingData) => {
                 this.existingData = existingData.toJSON();
             });
@@ -58,8 +59,34 @@ class ProductsImporter extends BaseImporter {
         this.dataToImport = this.dataToImport.filter(item => !invalidProducts.includes(item.id));
     }
 
+    preventDuplicates() {
+        debug('preventDuplicates');
+        let duplicateProducts = [];
+        _.each(this.dataToImport, (objectInFile) => {
+            const existingObject = _.find(
+                this.requiredExistingData.products,
+                {name: objectInFile.name, slug: objectInFile.slug}
+            );
+            // CASE: tier already exists
+            if (existingObject) {
+                debug(`skipping existing product ${objectInFile.name}`);
+                this.problems.push({
+                    message: 'Entry was not imported and ignored. Detected duplicated entry.',
+                    help: this.modelName,
+                    context: JSON.stringify({
+                        product: objectInFile
+                    })
+                });
+                duplicateProducts.push(objectInFile.id);
+            }
+        });
+        // ignore products that already exist
+        this.dataToImport = this.dataToImport.filter(item => !duplicateProducts.includes(item.id));
+    }
+
     replaceIdentifiers() {
         // this has to be in replaceIdentifiers because it's after required* fields are set
+        this.preventDuplicates();
         this.validateStripePrice();
         return super.replaceIdentifiers();
     }

core/server/data/importer/importers/data/stripe-products.js

@@ -8,6 +8,7 @@ class StripeProductsImporter extends BaseImporter {
         super(allDataFromFile, {
             modelName: 'StripeProduct',
             dataKeyToImport: 'stripe_products',
+            requiredFromFile: ['products'],
             requiredImportedData: ['products'],
             requiredExistingData: ['products']
         });
@@ -41,11 +42,32 @@ class StripeProductsImporter extends BaseImporter {
                 objectInFile.product_id = importedObject.id;
                 return;
             }
-            const existingObject = _.find(this.requiredExistingData.products, {id: objectInFile.product_id});
+
+            const existingObjectById = _.find(this.requiredExistingData.products, {id: objectInFile.product_id});
             // CASE: the product exists in the db already
-            if (existingObject) {
+            if (existingObjectById) {
                 return;
             }
+
+            // CASE: we skipped product import because a product with the same name and slug exists in the DB
+            debug('lookup product by name and slug');
+            const productFromFile = _.find(
+                this.requiredFromFile.products,
+                {id: objectInFile.product_id}
+            );
+            if (productFromFile) {
+                // look for the existing product with the same name and slug
+                const existingObjectByNameAndSlug = _.find(
+                    this.requiredExistingData.products,
+                    {name: productFromFile.name, slug: productFromFile.slug}
+                );
+                if (existingObjectByNameAndSlug) {
+                    debug(`resolved ${objectInFile.product_id} to ${existingObjectByNameAndSlug.name}`);
+                    objectInFile.product_id = existingObjectByNameAndSlug.id;
+                    return;
+                }
+            }
+
             // CASE: we don't know what product this is for
             debug(`ignoring stripe product ${objectInFile.stripe_product_id}`);
             invalidProducts.push(objectInFile.id);

core/server/models/base/bookshelf.js

@@ -61,6 +61,8 @@ ghostBookshelf.plugin(require('./plugins/data-manipulation'));
 
 ghostBookshelf.plugin(require('./plugins/overrides'));
 
+ghostBookshelf.plugin(require('./plugins/relations'));
+
 // Manages nested updates (relationships)
 ghostBookshelf.plugin('bookshelf-relations', {
     allowedOptions: ['context', 'importing', 'migrating'],

core/server/models/base/plugins/data-manipulation.js

@@ -31,6 +31,7 @@ module.exports = function (Bookshelf) {
 
             _.each(attrs, function each(value, key) {
                 if (value !== null
+                && Object.prototype.hasOwnProperty.call(schema.tables, self.tableName)
                 && Object.prototype.hasOwnProperty.call(schema.tables[self.tableName], key)
                 && schema.tables[self.tableName][key].type === 'dateTime') {
                     attrs[key] = moment(value).format('YYYY-MM-DD HH:mm:ss');

core/server/models/base/plugins/relations.js

@@ -0,0 +1,30 @@
+/**
+ * @param {import('bookshelf')} Bookshelf
+ */
+module.exports = function (Bookshelf) {
+    Bookshelf.Model = Bookshelf.Model.extend({
+        /**
+         * Return a relation, and load it if it hasn't been loaded already (or force a refresh with the forceRefresh option).
+         * refs https://github.com/TryGhost/Team/issues/1626
+         * @param {string} name Name of the relation to load
+         * @param {Object} [options] Options to pass to the fetch when not yet loaded (or when force refreshing) 
+         * @param {boolean} [options.forceRefresh] If true, the relation will be fetched again even if it has already been loaded.
+         * @returns {Promise<import('bookshelf').Model|import('bookshelf').Collection|null>}
+         */
+        getLazyRelation: async function (name, options = {}) {
+            if (this.relations[name] && !options.forceRefresh) {
+                // Relation was already loaded
+                return this.relations[name];
+            }
+
+            if (!this[name]) {
+                return undefined;
+            }
+            // Not yet loaded, or force refresh
+            // Note that we don't use .refresh on the relation on options.forceRefresh
+            // Because the relation can also be a collection, which doesn't have a refresh method
+            this.relations[name] = this[name]();
+            return this.relations[name].fetch(options);
+        }
+    });
+};

core/server/models/post.js

@@ -645,12 +645,12 @@ Post = ghostBookshelf.Model.extend({
 
         // ### Business logic for published_at and published_by
         // If the current status is 'published' and published_at is not set, set it to now
-        if (newStatus === 'published' && !publishedAt) {
+        if ((newStatus === 'published' || newStatus === 'sent') && !publishedAt) {
             this.set('published_at', new Date());
         }
 
         // If the current status is 'published' and the status has just changed ensure published_by is set correctly
-        if (newStatus === 'published' && this.hasChanged('status')) {
+        if ((newStatus === 'published' || newStatus === 'sent') && this.hasChanged('status')) {
             // unless published_by is set and we're importing, set published_by to contextUser
             if (!(this.get('published_by') && options.importing)) {
                 this.set('published_by', String(this.contextUser(options)));
@@ -666,7 +666,7 @@ Post = ghostBookshelf.Model.extend({
         if (options.newsletter
             && !this.get('newsletter_id')
             && this.hasChanged('status')
-            && (newStatus === 'published' || newStatus === 'scheduled')) {
+            && (newStatus === 'published' || newStatus === 'scheduled' || newStatus === 'sent')) {
             // Map the passed slug to the id + validate the passed newsletter
             ops.push(async () => {
                 const newsletter = await Newsletter.findOne({slug: options.newsletter}, {transacting: options.transacting, filter: 'status:active'});
@@ -691,7 +691,7 @@ Post = ghostBookshelf.Model.extend({
         // ensure draft posts have the email_recipient_filter reset unless an email has already been sent
         if (newStatus === 'draft' && this.hasChanged('status')) {
             ops.push(function ensureSendEmailWhenPublishedIsUnchanged() {
-                return self.related('email').fetch({transacting: options.transacting}).then((email) => {
+                return self.getLazyRelation('email', {transacting: options.transacting}).then((email) => {
                     if (!email) {
                         self.set('email_recipient_filter', 'all');
                         self.set('newsletter_id', null);
@@ -705,6 +705,9 @@ Post = ghostBookshelf.Model.extend({
         const hasEmailOnlyFlag = _.get(attrs, 'posts_meta.email_only') || model.related('posts_meta').get('email_only');
         if (hasEmailOnlyFlag && (newStatus === 'published') && this.hasChanged('status')) {
             this.set('status', 'sent');
+        } else if (!hasEmailOnlyFlag && (newStatus === 'sent') && this.hasChanged('status')) {
+            // Prevent setting status to 'sent' for non email only posts
+            this.set('status', 'published');
         }
 
         // If a title is set, not the same as the old title, a draft post, and has never been published

core/server/models/user.js

@@ -13,11 +13,17 @@ const validatePassword = require('../lib/validate-password');
 const permissions = require('../services/permissions');
 const urlUtils = require('../../shared/url-utils');
 const activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4'];
+const ASSIGNABLE_ROLES = ['Administrator', 'Editor', 'Author', 'Contributor'];
 
 const messages = {
     valueCannotBeBlank: 'Value in [{tableName}.{columnKey}] cannot be blank.',
     onlyOneRolePerUserSupported: 'Only one role per user is supported at the moment.',
     methodDoesNotSupportOwnerRole: 'This method does not support assigning the owner role',
+    invalidRoleValue: {
+        message: 'Role should be an existing role id or a role name',
+        context: 'Invalid role assigned to the user',
+        help: 'Change the provided role to a role id or use a role name.'
+    },
     userNotFound: 'User not found',
     ownerNotFound: 'Owner not found',
     notEnoughPermission: 'You do not have permission to perform this action',
@@ -507,7 +513,7 @@ User = ghostBookshelf.Model.extend({
         }
 
         ops.push(function update() {
-            return ghostBookshelf.Model.edit.call(self, data, options).then((user) => {
+            return ghostBookshelf.Model.edit.call(self, data, options).then(async (user) => {
                 let roleId;
 
                 if (!data.roles || !data.roles.length) {
@@ -521,17 +527,39 @@ User = ghostBookshelf.Model.extend({
                     if (roles.models[0].id === roleId) {
                         return;
                     }
-                    return ghostBookshelf.model('Role').findOne({id: roleId});
+
+                    if (ASSIGNABLE_ROLES.includes(roleId)) {
+                        // return if the role is already assigned
+                        if (roles.models[0].get('name') === roleId) {
+                            return;
+                        }
+
+                        return ghostBookshelf.model('Role').findOne({
+                            name: roleId
+                        });
+                    } else if (ObjectId.isValid(roleId)){
+                        return ghostBookshelf.model('Role').findOne({
+                            id: roleId
+                        });
+                    } else {
+                        return Promise.reject(
+                            new errors.ValidationError({
+                                message: tpl(messages.invalidRoleValue.message),
+                                context: tpl(messages.invalidRoleValue.context),
+                                help: tpl(messages.invalidRoleValue.help)
+                            })
+                        );
+                    }
                 }).then((roleToAssign) => {
                     if (roleToAssign && roleToAssign.get('name') === 'Owner') {
                         return Promise.reject(
                             new errors.ValidationError({
                                 message: tpl(messages.methodDoesNotSupportOwnerRole)
                             })
                         );
-                    } else {
+                    } else if (roleToAssign) {
                         // assign all other roles
-                        return user.roles().updatePivot({role_id: roleId});
+                        return user.roles().updatePivot({role_id: roleToAssign.id});
                     }
                 }).then(() => {
                     options.status = 'all';

core/server/services/bulk-email/bulk-email-processor.js

@@ -168,7 +168,7 @@ module.exports = {
 
         try {
             // Load newsletter data on email
-            await emailBatchModel.relations.email.related('newsletter').fetch(Object.assign({}, {require: false}, knexOptions));
+            await emailBatchModel.relations.email.getLazyRelation('newsletter', {require: false, ...knexOptions});
 
             // send the email
             const sendResponse = await this.send(emailBatchModel.relations.email.toJSON(), recipientRows, memberSegment);

core/server/services/mega/email-preview.js

@@ -10,7 +10,7 @@ class EmailPreview {
      * @returns {Promise<Object>}
      */
     async generateEmailContent(post, {newsletter, memberSegment} = {}) {
-        let newsletterModel = post.relations.newsletter ?? await post.related('newsletter').fetch();
+        let newsletterModel = await post.getLazyRelation('newsletter');
         if (!newsletterModel) {
             if (newsletter) {
                 newsletterModel = await models.Newsletter.findOne({slug: newsletter});