From e3b56dd99f440db85e311a3f11aa5106c5c72139 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Tue, 5 Oct 2021 12:00:52 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20immediately=20sent=20ema?=
 =?UTF-8?q?il=20when=20scheduling=20email-only=20post?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes https://linear.app/tryghost/issue/CORE-78/email-only-scheduling-should-work-the-same-way-as-regular-posts

- The email was going out at the moment of scheduling an email-only post instead of respecting the scheduled time.
---
 core/server/services/posts/posts-service.js   | 19 +++++++++++---
 .../unit/services/posts/posts-service.test.js | 25 +++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)
 create mode 100644 test/unit/services/posts/posts-service.test.js

diff --git a/core/server/services/posts/posts-service.js b/core/server/services/posts/posts-service.js
index 58b5041366..180ad71631 100644
--- a/core/server/services/posts/posts-service.js
+++ b/core/server/services/posts/posts-service.js
@@ -60,10 +60,9 @@ class PostsService {
                 }
             }
 
-            const postPublished = model.wasChanged() && (model.get('status') === 'published') && (model.previous('status') !== 'published');
-            const emailOnlyEnabled = model.related('posts_meta').get('email_only') && this.isSet('emailOnlyPosts');
+            const sendEmail = model.wasChanged() && this.shouldSendEmail(model.get('status'), model.previous('status'));
 
-            if (postPublished || emailOnlyEnabled) {
+            if (sendEmail) {
                 let postEmail = model.relations.email;
 
                 if (!postEmail) {
@@ -79,6 +78,18 @@ class PostsService {
         return model;
     }
 
+    /**
+     * Calculates if the email should be tried to be sent out
+     * @private
+     * @param {String} currentStatus current status from the post model
+     * @param {String} previousStatus previous status from the post model
+     * @returns {Boolean}
+     */
+    shouldSendEmail(currentStatus, previousStatus) {
+        return (['published', 'sent'].includes(currentStatus))
+            && (!['published', 'sent'].includes(previousStatus));
+    }
+
     handleCacheInvalidation(model) {
         let cacheInvalidate;
 
@@ -124,3 +135,5 @@ const getPostServiceInstance = (apiVersion) => {
 };
 
 module.exports = getPostServiceInstance;
+// exposed for testing purposes only
+module.exports.PostsService = PostsService;
diff --git a/test/unit/services/posts/posts-service.test.js b/test/unit/services/posts/posts-service.test.js
new file mode 100644
index 0000000000..0949b2dc55
--- /dev/null
+++ b/test/unit/services/posts/posts-service.test.js
@@ -0,0 +1,25 @@
+const should = require('should');
+
+const {PostsService} = require('../../../../core/server/services/posts/posts-service');
+
+describe('PostsService', function () {
+    describe('shouldSendEmail', function () {
+        it('calculates if an email should be sent', async function () {
+            const postsService = new PostsService({});
+
+            postsService.shouldSendEmail('published', 'draft').should.be.true();
+            postsService.shouldSendEmail('published', 'scheduled').should.be.true();
+            postsService.shouldSendEmail('sent', 'draft').should.be.true();
+            postsService.shouldSendEmail('sent', 'scheduled').should.be.true();
+
+            postsService.shouldSendEmail('published', 'published').should.be.false();
+            postsService.shouldSendEmail('published', 'sent').should.be.false();
+            postsService.shouldSendEmail('published', 'published').should.be.false();
+            postsService.shouldSendEmail('published', 'sent').should.be.false();
+            postsService.shouldSendEmail('sent', 'published').should.be.false();
+            postsService.shouldSendEmail('sent', 'sent').should.be.false();
+
+            postsService.shouldSendEmail().should.be.false();
+        });
+    });
+});