Fixed backward compatibility for send_email_when_published (#12357)

no-issue

* Handled send_email_when_published in Posts API

This restores backwards compatibility of the Posts API allowing existing
clients to continue to use the `send_email_when_published` flag. This
change uses two edits, which is unfortunate. The reason being is that
this is an API compatibility issue, not a model issue, so we shouldn't
introduce code to the model layer to handle it. The visibility property
of the model is used to determine how to fall back, and because it can
be left out of the API request, and relies on a default in the settings,
we require that the model decide on the `visibility` before we run our
fallback logic (or we duplicate the `visibility` default at the cost of
maintenance in the future)

* Dropped send_email_when_published column from posts

Since this column is not used any more, we can drop it from the table.
We include an extra migration to repopulate the column in the event of
a rollback

* Updated importer to handle send_email_when_published

Because we currently export this value from Ghost, we should correctly
import it. This follows the same logic as the migrations for this value.

* Included send_email_when_published in API response

As our v3 API documentation includes `send_email_when_published` we must
retain backward compatibility by calculating the property.

* Fixed fields filter with send_email_when_published

* Added safety checks to frame properties

Some parts of the code pass a manually created "frame" which is missing
lots of properties, so we check for the existence of all of them before
using them.

* Fixed 3.1 migration to include columnDefinition

We require that migrations have all the information they need contained
within them as they run in an unknown state of the codebase, which could
be from the commit they are introduced, to any future commit. In this
case the column definition is removed from the schema in 3.38 and the
migration would fail when run in this version or later.

core/server/api/canary/posts.js

@@ -125,6 +125,7 @@ module.exports = {
             'formats',
             'source',
             'email_recipient_filter',
+            'send_email_when_published',
             'force_rerender',
             // NOTE: only for internal context
             'forUpdate',
@@ -143,6 +144,9 @@ module.exports = {
                 },
                 email_recipient_filter: {
                     values: ['none', 'free', 'paid', 'all']
+                },
+                send_email_when_published: {
+                    values: [true, false]
                 }
             }
         },
@@ -151,12 +155,17 @@ module.exports = {
         },
         async query(frame) {
             /**Check host limits for members when send email is true**/
-            if (frame.options.email_recipient_filter && frame.options.email_recipient_filter !== 'none') {
+            if ((frame.options.email_recipient_filter && frame.options.email_recipient_filter !== 'none') || frame.options.send_email_when_published) {
                 await membersService.checkHostLimit();
             }
 
             let model = await models.Post.edit(frame.data.posts[0], frame.options);
 
+            if (!frame.options.email_recipient_filter && frame.options.send_email_when_published) {
+                frame.options.email_recipient_filter = model.get('visibility') === 'paid' ? 'paid' : 'all';
+                model = await models.Post.edit(frame.data.posts[0], frame.options);
+            }
+
             /**Handle newsletter email */
             if (model.get('email_recipient_filter') !== 'none') {
                 const postPublished = model.wasChanged() && (model.get('status') === 'published') && (model.previous('status') !== 'published');

core/server/api/canary/utils/serializers/input/posts.js

@@ -108,6 +108,10 @@ module.exports = {
 
         forcePageFilter(frame);
 
+        if (frame.options.columns && frame.options.columns.includes('send_email_when_published')) {
+            frame.options.columns.push('email_recipient_filter');
+        }
+
         /**
          * ## current cases:
          * - context object is empty (functional call, content api access)
@@ -136,6 +140,10 @@ module.exports = {
 
         forcePageFilter(frame);
 
+        if (frame.options.columns && frame.options.columns.includes('send_email_when_published')) {
+            frame.options.columns.push('email_recipient_filter');
+        }
+
         /**
          * ## current cases:
          * - context object is empty (functional call, content api access)

core/server/api/canary/utils/serializers/output/utils/clean.js

@@ -70,6 +70,8 @@ const author = (attrs, frame) => {
 };
 
 const post = (attrs, frame) => {
+    const columns = frame && frame.options && frame.options.columns || null;
+    const fields = frame && frame.original && frame.original.query && frame.original.query.fields || null;
     if (localUtils.isContentAPI(frame)) {
         // @TODO: https://github.com/TryGhost/Ghost/issues/10335
         // delete attrs.page;
@@ -95,13 +97,21 @@ const post = (attrs, frame) => {
             attrs.og_description = null;
         }
         // NOTE: the visibility column has to be always present in Content API response to perform content gating
-        if (frame.options.columns && frame.options.columns.includes('visibility') && !frame.original.query.fields.includes('visibility')) {
+        if (columns && columns.includes('visibility') && fields && !fields.includes('visibility')) {
             delete attrs.visibility;
         }
     } else {
         delete attrs.page;
     }
 
+    if (columns && columns.includes('email_recipient_filter') && fields && !fields.includes('email_recipient_filter')) {
+        delete attrs.email_recipient_filter;
+    }
+
+    if (fields && !fields.includes('send_email_when_published')) {
+        delete attrs.send_email_when_published;
+    }
+
     if (!attrs.tags) {
         delete attrs.primary_tag;
     }

core/server/api/canary/utils/serializers/output/utils/mapper.js

@@ -47,6 +47,14 @@ const mapPost = (model, frame) => {
         gating.forPost(jsonModel, frame);
     }
 
+    if (typeof jsonModel.email_recipient_filter === 'undefined') {
+        jsonModel.send_email_when_published = null;
+    } else if (jsonModel.email_recipient_filter === 'none') {
+        jsonModel.send_email_when_published = false;
+    } else {
+        jsonModel.send_email_when_published = true;
+    }
+
     clean.post(jsonModel, frame);
 
     if (frame.options && frame.options.withRelated) {

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

@@ -33,6 +33,15 @@ class PostsImporter extends BaseImporter {
                 }
                 delete obj.page;
             }
+
+            if (_.has(obj, 'send_email_when_published')) {
+                if (obj.send_email_when_published) {
+                    obj.email_recipient_filter = obj.visibility === 'paid' ? 'paid' : 'all';
+                } else {
+                    obj.email_recipient_filter = 'none';
+                }
+                delete obj.send_email_when_published;
+            }
         });
     }
 

core/server/data/migrations/versions/3.1/01-add-send-email-when-published-to-posts.js

@@ -7,7 +7,12 @@ module.exports.up = commands.createColumnMigration({
         return columnExists === true;
     },
     operation: commands.addColumn,
-    operationVerb: 'Adding'
+    operationVerb: 'Adding',
+    columnDefinition: {
+        type: 'bool',
+        nullable: true,
+        defaultTo: false
+    }
 });
 
 module.exports.down = commands.createColumnMigration({

core/server/data/migrations/versions/3.38/08-repopulate-send-email-when-published-down-migration.js

@@ -0,0 +1,25 @@
+const logging = require('../../../../../shared/logging');
+const {createTransactionalMigration} = require('../../utils');
+
+module.exports = createTransactionalMigration(
+    async function up() {},
+
+    async function down(connection) {
+        logging.info('Setting "send_email_when_published" based on "email_recipient_filter"');
+        await connection('posts')
+            .update({
+                send_email_when_published: true
+            })
+            .whereNot({
+                email_recipient_filter: 'none'
+            });
+
+        await connection('posts')
+            .update({
+                send_email_when_published: false
+            })
+            .where({
+                email_recipient_filter: 'none'
+            });
+    }
+);

core/server/data/migrations/versions/3.38/09-remove-send-email-when-published-column.js

@@ -0,0 +1,28 @@
+const {createColumnMigration, addColumn, dropColumn} = require('../../../schema/commands');
+
+module.exports = {
+    up: createColumnMigration({
+        table: 'posts',
+        column: 'send_email_when_published',
+        dbIsInCorrectState(columnExists) {
+            return columnExists === false;
+        },
+        operation: dropColumn,
+        operationVerb: 'Removing'
+    }),
+
+    down: createColumnMigration({
+        table: 'posts',
+        column: 'send_email_when_published',
+        dbIsInCorrectState(columnExists) {
+            return columnExists === true;
+        },
+        operation: addColumn,
+        operationVerb: 'Adding',
+        columnDefinition: {
+            type: 'bool',
+            nullable: true,
+            defaultTo: false
+        }
+    })
+};

core/server/data/schema/schema.js

@@ -29,7 +29,6 @@ module.exports = {
             defaultTo: 'public',
             validations: {isIn: [['public', 'members', 'paid']]}
         },
-        send_email_when_published: {type: 'bool', nullable: true, defaultTo: false},
         email_recipient_filter: {
             type: 'string',
             maxlength: 50,

core/server/models/post.js

@@ -50,7 +50,6 @@ Post = ghostBookshelf.Model.extend({
         }
 
         return {
-            send_email_when_published: false,
             uuid: uuid.v4(),
             status: 'draft',
             featured: false,

test/api-acceptance/admin/utils.js

@@ -44,6 +44,7 @@ const expectedProperties = {
         .concat(
             ..._(schema.posts_meta).keys().without('post_id', 'id')
         )
+        .concat('send_email_when_published')
     ,
 
     page: _(schema.posts)
@@ -57,7 +58,6 @@ const expectedProperties = {
         // deprecated
         .without('author_id', 'author')
         // pages are not sent as emails
-        .without('send_email_when_published')
         .without('email_recipient_filter')
         // always returns computed properties
         .concat('url', 'primary_tag', 'primary_author', 'excerpt')

test/api-acceptance/content/posts_spec.js

@@ -202,6 +202,27 @@ describe('Posts Content API', function () {
             });
     });
 
+    it('Can request send_email_when_published calculated field of posts', function (done) {
+        request.get(localUtils.API.getApiQuery(`posts/?key=${validKey}&fields=url&fields=send_email_when_published`))
+            .set('Origin', testUtils.API.getURL())
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(200)
+            .end(function (err, res) {
+                if (err) {
+                    return done(err);
+                }
+                const jsonResponse = res.body;
+
+                should.exist(jsonResponse.posts);
+
+                localUtils.API.checkResponse(jsonResponse.posts[0], 'post', false, false, ['url', 'send_email_when_published']);
+                res.body.posts[0].url.should.eql('http://127.0.0.1:2369/welcome/');
+                res.body.posts[0].send_email_when_published.should.eql(false);
+                done();
+            });
+    });
+
     it('Can include relations', function (done) {
         request.get(localUtils.API.getApiQuery(`posts/?key=${validKey}&include=tags,authors`))
             .set('Origin', testUtils.API.getURL())

test/api-acceptance/content/utils.js

@@ -36,6 +36,7 @@ const expectedProperties = {
             ..._(schema.posts_meta).keys().without('post_id', 'id')
         )
         .concat('reading_time')
+        .concat('send_email_when_published')
     ,
     author: _(schema.users)
         .keys()

test/regression/api/canary/admin/utils.js

@@ -36,6 +36,7 @@ const expectedProperties = {
         .concat(
             ..._(schema.posts_meta).keys().without('post_id', 'id')
         )
+        .concat('send_email_when_published')
     ,
     user: _(schema.users)
         .keys()

test/regression/api/canary/content/utils.js

@@ -35,6 +35,7 @@ const expectedProperties = {
             ..._(schema.posts_meta).keys().without('post_id', 'id')
         )
         .concat('reading_time')
+        .concat('send_email_when_published')
     ,
     author: _(schema.users)
         .keys()

test/regression/api/v2/admin/utils.js

@@ -27,7 +27,6 @@ const expectedProperties = {
         .without('page')
         .without('author_id', 'author')
         // emails are not supported in API v2
-        .without('send_email_when_published')
         .without('email_recipient_filter')
         // always returns computed properties
         .concat('url', 'primary_tag', 'primary_author', 'excerpt')

test/regression/api/v2/content/utils.js

@@ -22,7 +22,6 @@ const expectedProperties = {
         // v2 API doesn't return unused fields
         .without('locale', 'visibility')
         // emails are not supported in API v2
-        .without('send_email_when_published')
         .without('email_recipient_filter')
         // These fields aren't useful as they always have known values
         .without('status')

test/regression/api/v3/admin/utils.js

@@ -35,6 +35,7 @@ const expectedProperties = {
         .concat(
             ..._(schema.posts_meta).keys().without('post_id', 'id')
         )
+        .concat('send_email_when_published')
     ,
     user: _(schema.users)
         .keys()

test/regression/api/v3/content/utils.js

@@ -35,6 +35,7 @@ const expectedProperties = {
             ..._(schema.posts_meta).keys().without('post_id', 'id')
         )
         .concat('reading_time')
+        .concat('send_email_when_published')
     ,
     author: _(schema.users)
         .keys()

test/unit/data/schema/integrity_spec.js

@@ -32,7 +32,7 @@ const defaultSettings = require('../../../../core/server/data/schema/default-set
  */
 describe('DB version integrity', function () {
     // Only these variables should need updating
-    const currentSchemaHash = '97705c7f5ae33414fcdb009c143480a8';
+    const currentSchemaHash = '102b04bbd38cd2451fbf0957ffc35b30';
     const currentFixturesHash = 'd46d696c94d03e41a5903500547fea77';
     const currentSettingsHash = '229360069a9c77a945727a3c5869c3c6';
     const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';