From 531dbc5e6a68c9490042a8082d014065e28cead0 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Fri, 30 Jul 2021 10:43:09 +0300
Subject: [PATCH] FIX: Do not offer to save draft if invalid (#13863)

An invalid draft is the draft of a topic with a short title or body.
The client does not save these, but it will ask the client if they want
to save it. Even if the answer is 'yes', the draft is discarded. This
commit skips Save button for small drafts.
---
 .../discourse/app/controllers/composer.js     | 13 ++++---
 .../discourse/app/models/composer.js          | 36 +++++++++++++++----
 .../app/templates/modal/discard-draft.hbs     |  4 ++-
 .../tests/acceptance/composer-test.js         | 11 ++++++
 4 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js
index e9fa6a2c80f..5f0f114b095 100644
--- a/app/assets/javascripts/discourse/app/controllers/composer.js
+++ b/app/assets/javascripts/discourse/app/controllers/composer.js
@@ -1189,9 +1189,6 @@ export default Controller.extend({
           },
           onSaveDraft: () => {
             this._saveDraft();
-            if (this.model.draftKey === Composer.NEW_TOPIC_KEY) {
-              this.currentUser.set("has_topic_draft", true);
-            }
             this.model.clearState();
             this.close();
             resolve();
@@ -1242,10 +1239,12 @@ export default Controller.extend({
           );
         }
       } else {
-        this._saveDraftPromise = model.saveDraft().finally(() => {
-          this._lastDraftSaved = Date.now();
-          this._saveDraftPromise = null;
-        });
+        this._saveDraftPromise = model
+          .saveDraft(this.currentUser)
+          .finally(() => {
+            this._lastDraftSaved = Date.now();
+            this._saveDraftPromise = null;
+          });
       }
     }
   },
diff --git a/app/assets/javascripts/discourse/app/models/composer.js b/app/assets/javascripts/discourse/app/models/composer.js
index 36349ed1210..b93ab8c6e8a 100644
--- a/app/assets/javascripts/discourse/app/models/composer.js
+++ b/app/assets/javascripts/discourse/app/models/composer.js
@@ -1165,38 +1165,56 @@ const Composer = RestModel.extend({
     return "";
   },
 
-  saveDraft() {
+  @discourseComputed(
+    "draftSaving",
+    "disableDrafts",
+    "canEditTitle",
+    "title",
+    "reply",
+    "titleLengthValid",
+    "replyLength",
+    "minimumPostLength"
+  )
+  canSaveDraft() {
     if (this.draftSaving) {
-      return Promise.resolve();
+      return false;
     }
 
     // Do not save when drafts are disabled
     if (this.disableDrafts) {
-      return Promise.resolve();
+      return false;
     }
 
     if (this.canEditTitle) {
       // Save title and/or post body
       if (isEmpty(this.title) && isEmpty(this.reply)) {
-        return Promise.resolve();
+        return false;
       }
 
       // Do not save when both title and reply's length are too small
       if (!this.titleLengthValid && this.replyLength < this.minimumPostLength) {
-        return Promise.resolve();
+        return false;
       }
     } else {
       // Do not save when there is no reply
       if (isEmpty(this.reply)) {
-        return Promise.resolve();
+        return false;
       }
 
       // Do not save when the reply's length is too small
       if (this.replyLength < this.minimumPostLength) {
-        return Promise.resolve();
+        return false;
       }
     }
 
+    return true;
+  },
+
+  saveDraft(user) {
+    if (!this.canSaveDraft) {
+      return Promise.resolve();
+    }
+
     this.setProperties({
       draftSaving: true,
       draftConflictUser: null,
@@ -1225,6 +1243,10 @@ const Composer = RestModel.extend({
             draftConflictUser: result.conflict_user,
           });
         } else {
+          if (this.draftKey === NEW_TOPIC_KEY && user) {
+            user.set("has_topic_draft", true);
+          }
+
           this.setProperties({
             draftStatus: null,
             draftConflictUser: null,
diff --git a/app/assets/javascripts/discourse/app/templates/modal/discard-draft.hbs b/app/assets/javascripts/discourse/app/templates/modal/discard-draft.hbs
index 46c706cbc48..207cf43ccc7 100644
--- a/app/assets/javascripts/discourse/app/templates/modal/discard-draft.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/discard-draft.hbs
@@ -6,6 +6,8 @@
 
 <div class="modal-footer">
   {{d-button icon="far-trash-alt" label="post.cancel_composer.discard" class="btn-danger discard-draft" action=(action "destroyDraft")}}
-  {{d-button label="post.cancel_composer.save_draft" class="save-draft" action=(action "saveDraftAndClose")}}
+  {{#if model.canSaveDraft}}
+    {{d-button label="post.cancel_composer.save_draft" class="save-draft" action=(action "saveDraftAndClose")}}
+  {{/if}}
   {{d-button label="post.cancel_composer.keep_editing" class="keep-editing" action=(action "dismissModal")}}
 </div>
diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js
index 4f8aee38cf6..5cbdf0bc912 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js
@@ -999,4 +999,15 @@ acceptance("Composer", function (needs) {
     await fillIn(".d-editor-input", "@staff");
     assert.ok(exists(".composer-popup"), "Shows the 'group_mentioned' notice");
   });
+
+  test("Does not save invalid draft", async function (assert) {
+    this.siteSettings.min_first_post_length = 20;
+
+    await visit("/");
+    await click("#create-topic");
+    await fillIn("#reply-title", "Something");
+    await fillIn(".d-editor-input", "Something");
+    await click(".save-or-cancel .cancel");
+    assert.notOk(exists(".discard-draft-modal .save-draft"));
+  });
 });