FEATURE: Allow users to save draft and close composer (#12439)

We previously included this option conditionally when users were replying or creating a new topic while they had content already in the composer. This makes the dialog always include three buttons: - Close and discard - Close and save draft for later - Keed editing This also changes how the backend notifies the frontend when there is a current draft topic. This is now sent via the `has_topic_draft` property in the current user serializer.
2021-03-19 09:19:15 -04:00 · 2021-03-19 09:19:15 -04:00 · d470e4fade
parent 6eab1e83c3
commit d470e4fade
23 changed files with 155 additions and 148 deletions
--- a/app/assets/javascripts/discourse/app/components/d-modal-body.js
+++ b/app/assets/javascripts/discourse/app/components/d-modal-body.js
@ -56,7 +56,8 @@ export default Component.extend({
        "fixed",
        "subtitle",
        "rawSubtitle",
-        "dismissable"
+        "dismissable",
+        "headerClass"
      )
    );
  },
--- a/app/assets/javascripts/discourse/app/components/d-modal.js
+++ b/app/assets/javascripts/discourse/app/components/d-modal.js
@ -23,6 +23,7 @@ export default Component.extend({
  title: null,
  subtitle: null,
  role: "dialog",
+  headerClass: null,

  init() {
    this._super(...arguments);
@ -129,6 +130,10 @@ export default Component.extend({
      this.set("dismissable", true);
    }

+    if (data.headerClass) {
+      this.set("headerClass", data.headerClass);
+    }
+
    if (this.element) {
      const autofocusInputs = this.element.querySelectorAll(
        ".modal-body input[autofocus]"
--- a/app/assets/javascripts/discourse/app/controllers/composer.js
+++ b/app/assets/javascripts/discourse/app/controllers/composer.js
@ -18,6 +18,7 @@ import discourseComputed, {
 import DiscourseURL from "discourse/lib/url";
 import Draft from "discourse/models/draft";
 import I18n from "I18n";
+import { iconHTML } from "discourse-common/lib/icon-library";
 import { Promise } from "rsvp";
 import bootbox from "bootbox";
 import { buildQuote } from "discourse/lib/quote";
@ -545,9 +546,7 @@ export default Controller.extend({
    },

    cancel() {
-      const differentDraftContext =
-        this.get("topic.id") !== this.get("model.topic.id");
-      this.cancelComposer(differentDraftContext);
+      this.cancelComposer();
    },

    save(ignore, event) {
@ -903,13 +902,7 @@ export default Controller.extend({
          }
        }

-        // If it's a different draft, cancel it and try opening again.
-        const differentDraftContext =
-          opts.post && composerModel.topic
-            ? composerModel.topic.id !== opts.post.topic_id
-            : true;
-
-        return this.cancelComposer(differentDraftContext)
+        return this.cancelComposer()
          .then(() => this.open(opts))
          .then(resolve, reject);
      }
@ -1037,18 +1030,19 @@ export default Controller.extend({
    return false;
  },

-  destroyDraft() {
+  destroyDraft(draftSequence = null) {
    const key = this.get("model.draftKey");
    if (key) {
-      if (key === "new_topic") {
-        this.send("clearTopicDraft");
+      if (key === Composer.NEW_TOPIC_KEY) {
+        this.currentUser.set("has_topic_draft", false);
      }

      if (this._saveDraftPromise) {
        return this._saveDraftPromise.then(() => this.destroyDraft());
      }

-      return Draft.clear(key, this.get("model.draftSequence")).then(() =>
+      const sequence = draftSequence || this.get("model.draftSequence");
+      return Draft.clear(key, sequence).then(() =>
        this.appEvents.trigger("draft:destroyed", key)
      );
    } else {
@ -1078,9 +1072,12 @@ export default Controller.extend({
          {
            label: I18n.t("drafts.abandon.yes_value"),
            class: "btn-danger",
+            icon: iconHTML("far-trash-alt"),
            callback: () => {
-              data.draft = null;
-              resolve(data);
+              this.destroyDraft(data.draft_sequence).finally(() => {
+                data.draft = null;
+                resolve(data);
+              });
            },
          },
        ]);
@ -1091,22 +1088,20 @@ export default Controller.extend({
    }
  },

-  cancelComposer(differentDraft = false) {
+  cancelComposer() {
    this.skipAutoSave = true;

    if (this._saveDraftDebounce) {
      cancel(this._saveDraftDebounce);
    }

-    let promise = new Promise((resolve, reject) => {
+    let promise = new Promise((resolve) => {
      if (this.get("model.hasMetaData") || this.get("model.replyDirty")) {
        const controller = showModal("discard-draft", {
          model: this.model,
          modalClass: "discard-draft-modal",
-          title: "post.abandon.title",
        });
        controller.setProperties({
-          differentDraft,
          onDestroyDraft: () => {
            this.destroyDraft()
              .then(() => {
@ -1118,15 +1113,16 @@ export default Controller.extend({
              });
          },
          onSaveDraft: () => {
-            // cancel composer without destroying draft on new draft context
-            if (differentDraft) {
-              this.model.clearState();
-              this.close();
-              resolve();
+            this._saveDraft();
+            if (this.model.draftKey === Composer.NEW_TOPIC_KEY) {
+              this.currentUser.set("has_topic_draft", true);
            }
-
-            reject();
+            this.model.clearState();
+            this.close();
+            resolve();
          },
+          // needed to resume saving drafts if composer stays open
+          onDismissModal: () => resolve(),
        });
      } else {
        // it is possible there is some sort of crazy draft with no body ... just give up on it
--- a/app/assets/javascripts/discourse/app/controllers/discard-draft.js
+++ b/app/assets/javascripts/discourse/app/controllers/discard-draft.js
@ -1,40 +1,19 @@
 import Controller from "@ember/controller";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
-import discourseComputed from "discourse-common/utils/decorators";

 export default Controller.extend(ModalFunctionality, {
-  differentDraft: null,
-
-  @discourseComputed()
-  keyPrefix() {
-    return this.model.action === "edit" ? "post.abandon_edit" : "post.abandon";
-  },
-
-  @discourseComputed("keyPrefix")
-  descriptionKey(keyPrefix) {
-    return `${keyPrefix}.confirm`;
-  },
-
-  @discourseComputed("keyPrefix")
-  discardKey(keyPrefix) {
-    return `${keyPrefix}.yes_value`;
-  },
-
-  @discourseComputed("keyPrefix", "differentDraft")
-  saveKey(keyPrefix, differentDraft) {
-    return differentDraft
-      ? `${keyPrefix}.no_save_draft`
-      : `${keyPrefix}.no_value`;
-  },
-
  actions: {
-    _destroyDraft() {
+    destroyDraft() {
      this.onDestroyDraft();
      this.send("closeModal");
    },
-    _saveDraft() {
+    saveDraftAndClose() {
      this.onSaveDraft();
      this.send("closeModal");
    },
+    dismissModal() {
+      this.onDismissModal();
+      this.send("closeModal");
+    },
  },
 });
--- a/app/assets/javascripts/discourse/app/controllers/navigation/categories.js
+++ b/app/assets/javascripts/discourse/app/controllers/navigation/categories.js
@ -1,15 +1,6 @@
 import NavigationDefaultController from "discourse/controllers/navigation/default";
-import discourseComputed from "discourse-common/utils/decorators";
 import { inject } from "@ember/controller";

 export default NavigationDefaultController.extend({
  discoveryCategories: inject("discovery/categories"),
-
-  @discourseComputed(
-    "discoveryCategories.model",
-    "discoveryCategories.model.draft"
-  )
-  draft() {
-    return this.get("discoveryCategories.model.draft");
-  },
 });
--- a/app/assets/javascripts/discourse/app/controllers/navigation/default.js
+++ b/app/assets/javascripts/discourse/app/controllers/navigation/default.js
@ -1,13 +1,6 @@
 import Controller, { inject as controller } from "@ember/controller";
 import FilterModeMixin from "discourse/mixins/filter-mode";
-import discourseComputed from "discourse-common/utils/decorators";

 export default Controller.extend(FilterModeMixin, {
  discovery: controller(),
-  discoveryTopics: controller("discovery/topics"),
-
-  @discourseComputed("discoveryTopics.model", "discoveryTopics.model.draft")
-  draft: function () {
-    return this.get("discoveryTopics.model.draft");
-  },
 });
--- a/app/assets/javascripts/discourse/app/controllers/tag-show.js
+++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js
@ -29,11 +29,6 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, {
  q: null,
  showInfo: false,

-  @discourseComputed("list", "list.draft")
-  createTopicLabel(list, listDraft) {
-    return listDraft ? "topic.open_draft" : "topic.create";
-  },
-
  @discourseComputed(
    "canCreateTopic",
    "category",
--- a/app/assets/javascripts/discourse/app/routes/discourse.js
+++ b/app/assets/javascripts/discourse/app/routes/discourse.js
@ -1,4 +1,5 @@
 import Composer from "discourse/models/composer";
+import Draft from "discourse/models/draft";
 import Route from "@ember/routing/route";
 import { once } from "@ember/runloop";
 import { seenUser } from "discourse/lib/user-presence";
@ -42,23 +43,6 @@ const DiscourseRoute = Route.extend({
    refreshTitle() {
      once(this, this._refreshTitleOnce);
    },
-
-    clearTopicDraft() {
-      // perhaps re-delegate this to root controller in all cases?
-      // TODO also poison the store so it does not come back from the
-      // dead
-      if (this.get("controller.list.draft")) {
-        this.set("controller.list.draft", null);
-      }
-
-      if (this.controllerFor("discovery/categories").get("model.draft")) {
-        this.controllerFor("discovery/categories").set("model.draft", null);
-      }
-
-      if (this.controllerFor("discovery/topics").get("model.draft")) {
-        this.controllerFor("discovery/topics").set("model.draft", null);
-      }
-    },
  },

  redirectIfLoginRequired() {
@ -68,20 +52,24 @@ const DiscourseRoute = Route.extend({
    }
  },

-  openTopicDraft(model) {
+  openTopicDraft() {
    const composer = this.controllerFor("composer");

    if (
      composer.get("model.action") === Composer.CREATE_TOPIC &&
-      composer.get("model.draftKey") === model.draft_key
+      composer.get("model.draftKey") === Composer.NEW_TOPIC_KEY
    ) {
      composer.set("model.composeState", Composer.OPEN);
    } else {
-      composer.open({
-        action: Composer.CREATE_TOPIC,
-        draft: model.draft,
-        draftKey: model.draft_key,
-        draftSequence: model.draft_sequence,
+      Draft.get(Composer.NEW_TOPIC_KEY).then((data) => {
+        if (data.draft) {
+          composer.open({
+            action: Composer.CREATE_TOPIC,
+            draft: data.draft,
+            draftKey: Composer.NEW_TOPIC_KEY,
+            draftSequence: data.draft_sequence,
+          });
+        }
      });
    }
  },
--- a/app/assets/javascripts/discourse/app/routes/discovery-categories.js
+++ b/app/assets/javascripts/discourse/app/routes/discovery-categories.js
@ -118,9 +118,8 @@ const DiscoveryCategoriesRoute = DiscourseRoute.extend(OpenComposer, {
    },

    createTopic() {
-      const model = this.controllerFor("discovery/categories").get("model");
-      if (model.draft) {
-        this.openTopicDraft(model);
+      if (this.get("currentUser.has_topic_draft")) {
+        this.openTopicDraft();
      } else {
        this.openComposer(this.controllerFor("discovery/categories"));
      }
--- a/app/assets/javascripts/discourse/app/routes/discovery.js
+++ b/app/assets/javascripts/discourse/app/routes/discovery.js
@ -59,9 +59,8 @@ export default DiscourseRoute.extend(OpenComposer, {
    },

    createTopic() {
-      const model = this.controllerFor("discovery/topics").get("model");
-      if (model.draft) {
-        this.openTopicDraft(model);
+      if (this.get("currentUser.has_topic_draft")) {
+        this.openTopicDraft();
      } else {
        this.openComposer(this.controllerFor("discovery/topics"));
      }
--- a/app/assets/javascripts/discourse/app/routes/tag-show.js
+++ b/app/assets/javascripts/discourse/app/routes/tag-show.js
@ -180,11 +180,10 @@ export default DiscourseRoute.extend(FilterModeMixin, {
    },

    createTopic() {
-      const controller = this.controllerFor("tag.show");
-
-      if (controller.get("list.draft")) {
-        this.openTopicDraft(controller.get("list"));
+      if (this.get("currentUser.has_topic_draft")) {
+        this.openTopicDraft();
      } else {
+        const controller = this.controllerFor("tag.show");
        const composerController = this.controllerFor("composer");
        composerController
          .open({
--- a/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs
+++ b/app/assets/javascripts/discourse/app/templates/components/d-modal.hbs
@ -1,7 +1,7 @@
 <div class="modal-outer-container">
  <div class="modal-middle-container">
    <div class="modal-inner-container">
-      <div class="modal-header">
+      <div class="modal-header {{headerClass}}">
        {{#if dismissable}}
          {{d-button icon="times" action=(route-action "closeModal" "initiatedByCloseButton") class="btn-flat modal-close close" title="modal.close"}}
        {{/if}}
@ -23,7 +23,8 @@
                panel=panel
                panelsLength=panels.length
                selectedPanel=selectedPanel
-                onSelectPanel=onSelectPanel}}
+                onSelectPanel=onSelectPanel
+              }}
            {{/each}}
          </ul>
        {{/if}}
--- a/app/assets/javascripts/discourse/app/templates/modal/discard-draft.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/discard-draft.hbs
@ -1,10 +1,11 @@
-{{#d-modal-body}}
+{{#d-modal-body dismissable=false headerClass="empty"}}
  <div class="instructions">
-    {{i18n descriptionKey}}
+    {{i18n "post.cancel_composer.confirm"}}
  </div>
 {{/d-modal-body}}

 <div class="modal-footer">
-  {{d-button label=discardKey class="btn-danger" action=(action "_destroyDraft")}}
-  {{d-button label=saveKey action=(action "_saveDraft")}}
+  {{d-button icon="far-trash-alt" label="post.cancel_composer.discard" class="btn-danger" action=(action "destroyDraft")}}
+  {{d-button label="post.cancel_composer.save_draft" class="save-draft" action=(action "saveDraftAndClose")}}
+  {{d-button label="post.cancel_composer.keep_editing" class="keep-editing" action=(action "dismissModal")}}
 </div>
--- a/app/assets/javascripts/discourse/app/templates/navigation/categories.hbs
+++ b/app/assets/javascripts/discourse/app/templates/navigation/categories.hbs
@ -5,6 +5,6 @@
    createCategory=(route-action "createCategory")
    reorderCategories=(route-action "reorderCategories")
    canCreateTopic=canCreateTopic
-    hasDraft=draft
+    hasDraft=currentUser.has_topic_draft
    createTopic=(route-action "createTopic")}}
 {{/d-section}}
--- a/app/assets/javascripts/discourse/app/templates/navigation/category.hbs
+++ b/app/assets/javascripts/discourse/app/templates/navigation/category.hbs
@ -24,7 +24,7 @@
    canCreateTopic=canCreateTopic
    createTopic=(route-action "createTopic")
    createTopicDisabled=cannotCreateTopicOnCategory
-    hasDraft=draft
+    hasDraft=currentUser.has_topic_draft
    editCategory=(route-action "editCategory" category)}}

  {{plugin-outlet name="category-navigation" args=(hash category=category)}}
--- a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs
+++ b/app/assets/javascripts/discourse/app/templates/navigation/default.hbs
@ -2,6 +2,6 @@
  {{d-navigation
    filterMode=filterMode
    canCreateTopic=canCreateTopic
-    hasDraft=draft
+    hasDraft=currentUser.has_topic_draft
    createTopic=(route-action "createTopic")}}
 {{/d-section}}
--- a/app/assets/javascripts/discourse/app/templates/tags/show.hbs
+++ b/app/assets/javascripts/discourse/app/templates/tags/show.hbs
@ -10,7 +10,7 @@
        {{d-navigation
          filterMode=filterMode
          canCreateTopic=canCreateTopic
-          hasDraft=draft
+          hasDraft=currentUser.has_topic_draft
          createTopic=(route-action "createTopic")
          category=category
          tag=tag
--- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js
@ -291,12 +291,22 @@ acceptance("Composer", function (needs) {
    await click(".topic-post:nth-of-type(1) button.show-more-actions");
    await click(".topic-post:nth-of-type(1) button.edit");

-    await click(".modal-footer button:nth-of-type(2)");
-
-    assert.ok(!visible(".discard-draft-modal.modal"));
+    await click(".modal-footer button.keep-editing");
+    assert.ok(invisible(".discard-draft-modal.modal"));
    assert.equal(
      queryAll(".d-editor-input").val(),
-      "this is the content of my reply"
+      "this is the content of my reply",
+      "composer does not switch when using Keep Editing button"
+    );
+
+    await click(".topic-post:nth-of-type(1) button.edit");
+    await click(".modal-footer button.save-draft");
+    assert.ok(invisible(".discard-draft-modal.modal"));
+
+    assert.equal(
+      queryAll(".d-editor-input").val(),
+      queryAll(".topic-post:nth-of-type(1) .cooked > p").text(),
+      "composer has contents of post to be edited"
    );
  });

@ -590,10 +600,16 @@ acceptance("Composer", function (needs) {
      "it pops up a confirmation dialog"
    );
    assert.equal(
-      queryAll(".modal-footer button:nth-of-type(2)").text().trim(),
-      I18n.t("post.abandon.no_value")
+      queryAll(".modal-footer button.save-draft").text().trim(),
+      I18n.t("post.cancel_composer.save_draft"),
+      "has save draft button"
    );
-    await click(".modal-footer button:nth-of-type(1)");
+    assert.equal(
+      queryAll(".modal-footer button.keep-editing").text().trim(),
+      I18n.t("post.cancel_composer.keep_editing"),
+      "has keep editing button"
+    );
+    await click(".modal-footer button.save-draft");
    assert.equal(
      queryAll(".d-editor-input").val().indexOf("This is the second post."),
      0,
@ -615,14 +631,20 @@ acceptance("Composer", function (needs) {
      "it pops up a confirmation dialog"
    );
    assert.equal(
-      queryAll(".modal-footer button:nth-of-type(2)").text().trim(),
-      I18n.t("post.abandon.no_save_draft")
+      queryAll(".modal-footer button.save-draft").text().trim(),
+      I18n.t("post.cancel_composer.save_draft"),
+      "has save draft button"
    );
-    await click(".modal-footer button:nth-of-type(2)");
+    assert.equal(
+      queryAll(".modal-footer button.keep-editing").text().trim(),
+      I18n.t("post.cancel_composer.keep_editing"),
+      "has keep editing button"
+    );
+    await click(".modal-footer button.save-draft");
    assert.equal(
      queryAll(".d-editor-input").val(),
      "",
-      "it populates the input with the post text"
+      "it clears the composer input"
    );
  });

--- a/app/assets/stylesheets/common/base/modal.scss
+++ b/app/assets/stylesheets/common/base/modal.scss
@ -54,8 +54,10 @@

 .modal-header {
  display: flex;
-  padding: 10px 15px;
-  border-bottom: 1px solid var(--primary-low);
+  &:not(.empty) {
+    padding: 10px 15px;
+    border-bottom: 1px solid var(--primary-low);
+  }
  align-items: center;

  .title {
--- a/app/models/draft.rb
+++ b/app/models/draft.rb
@ -132,6 +132,12 @@ class Draft < ActiveRecord::Base
    data if current_sequence == draft_sequence
  end

+  def self.has_topic_draft(user)
+    return if !user || !user.id || !User.human_user_id?(user.id)
+
+    Draft.where(user_id: user.id, draft_key: NEW_TOPIC).present?
+  end
+
  def self.clear(user, key, sequence)
    return if !user || !user.id || !User.human_user_id?(user.id)

--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@ -51,6 +51,7 @@ class CurrentUserSerializer < BasicUserSerializer
             :featured_topic,
             :skip_new_user_tips,
             :do_not_disturb_until,
+             :has_topic_draft,

  def groups
    object.visible_groups.pluck(:id, :name).map { |id, name| { id: id, name: name } }
@ -238,4 +239,12 @@ class CurrentUserSerializer < BasicUserSerializer
  def featured_topic
    object.user_profile.featured_topic
  end
+
+  def has_topic_draft
+    true
+  end
+
+  def include_has_topic_draft?
+    Draft.has_topic_draft(object)
+  end
 end
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@ -346,9 +346,9 @@ en:
      new_private_message: "New private message draft"
      topic_reply: "Draft reply"
      abandon:
-        confirm: "You already opened another draft in this topic. Are you sure you want to abandon it?"
-        yes_value: "Yes, abandon"
-        no_value: "No, keep"
+        confirm: "You have a draft in progress for this topic. What would you like to do with it?"
+        yes_value: "Discard"
+        no_value: "Resume editing"

    topic_count_latest:
      one: "See %{count} new or updated topic"
@ -2917,18 +2917,11 @@ en:
        attachment_upload_not_allowed_for_new_user: "Sorry, new users can not upload attachments."
        attachment_download_requires_login: "Sorry, you need to be logged in to download attachments."

-      abandon_edit:
-        confirm: "Are you sure you want to discard your changes?"
-        no_value: "No, keep"
-        no_save_draft: "No, save draft"
-        yes_value: "Yes, discard edit"
-
-      abandon:
-        title: "Abandon Draft"
-        confirm: "Are you sure you want to abandon your post?"
-        no_value: "No, keep"
-        no_save_draft: "No, save draft"
-        yes_value: "Yes, abandon"
+      cancel_composer:
+        confirm: "What would you like to do with your post?"
+        discard: "Discard"
+        save_draft: "Save draft for later"
+        keep_editing: "Keep editing"

      via_email: "this post arrived via email"
      via_auto_generated_email: "this post arrived via an auto generated email"
--- a/spec/serializers/current_user_serializer_spec.rb
+++ b/spec/serializers/current_user_serializer_spec.rb
@ -138,4 +138,32 @@ RSpec.describe CurrentUserSerializer do
      expect(payload[:groups]).to eq([{ id: public_group.id, name: public_group.name }])
    end
  end
+
+  context "#has_topic_draft" do
+    fab!(:user) { Fabricate(:user) }
+    let :serializer do
+      CurrentUserSerializer.new(user, scope: Guardian.new, root: false)
+    end
+
+    it "is not included by default" do
+      payload = serializer.as_json
+      expect(payload).not_to have_key(:has_topic_draft)
+    end
+
+    it "returns true when user has a draft" do
+      Draft.set(user, Draft::NEW_TOPIC, 0, "test1")
+
+      payload = serializer.as_json
+      expect(payload[:has_topic_draft]).to eq(true)
+    end
+
+    it "clearing a draft removes has_topic_draft from payload" do
+      sequence = Draft.set(user, Draft::NEW_TOPIC, 0, "test1")
+      Draft.clear(user, Draft::NEW_TOPIC, sequence)
+
+      payload = serializer.as_json
+      expect(payload).not_to have_key(:has_topic_draft)
+    end
+
+  end
 end