FIX: editing post while replying

BEFORE: if you click the "reply" button on a post and then decided that you want to "edit" the same post, clicking the "edit" button would do nothing. Clicking "edit" on another post works, but editing the same post would appear broken.

AFTER: if you click the "edit" button, it will properly load the content of the post you're trying to edit. No matter which one it is.

This was somewhat tricky to track down as the system specs seemed to contradict the qunit tests until I realized that the qunit tests were only testing the edit on the 1st post and the system specs were testing on replies.

I improved the qunit tests to test both editing OP and a reply and (hopefully) made the system specs a little bit clearer.

This is a follow up to bbe62d88d2554189e15f407736b1f7a01968c631.

app/assets/javascripts/discourse/app/controllers/topic.js

@@ -848,12 +848,7 @@ export default class TopicController extends Controller.extend(
       return false;
     }
 
-    const composer = this.composer;
-    let topic = this.model;
-    const composerModel = composer.get("model");
-    let editingFirst =
-      composerModel &&
-      (post.get("firstPost") || composerModel.get("editingFirstPost"));
+    const topic = this.model;
 
     let editingSharedDraft = false;
     let draftsCategoryId = this.get("site.shared_drafts_category_id");
@@ -872,22 +867,14 @@ export default class TopicController extends Controller.extend(
       opts.destinationCategoryId = topic.get("destination_category_id");
     }
 
-    // Reopen the composer if we're editing the same post
-    const editingExisting =
-      post.id === composerModel?.post?.id &&
-      opts?.action === Composer.EDIT &&
-      composerModel?.draftKey === opts.draftKey;
-    if (editingExisting) {
-      composer.unshrink();
-      return;
-    }
+    const { composer } = this;
+    const composerModel = composer.get("model");
+    const editingSamePost =
+      opts.post.id === composerModel?.post?.id &&
+      opts.action === composerModel?.action &&
+      opts.draftKey === composerModel?.draftKey;
 
-    // Cancel and reopen the composer for the first post
-    if (editingFirst) {
-      composer.cancelComposer(opts).then(() => composer.open(opts));
-    } else {
-      composer.open(opts);
-    }
+    return editingSamePost ? composer.unshrink() : composer.open(opts);
   }
 
   @action

app/assets/javascripts/discourse/tests/acceptance/composer-test.js

@@ -571,7 +571,7 @@ acceptance("Composer", function (needs) {
     );
   });
 
-  test("Composer can toggle between edit and reply", async function (assert) {
+  test("Composer can toggle between edit and reply on the OP", async function (assert) {
     await visit("/t/this-is-a-test-topic/9");
 
     await click(".topic-post:nth-of-type(1) button.edit");
@@ -579,12 +579,10 @@ acceptance("Composer", function (needs) {
       query(".d-editor-input").value.startsWith("This is the first post."),
       "it populates the input with the post text"
     );
+
     await click(".topic-post:nth-of-type(1) button.reply");
-    assert.strictEqual(
-      query(".d-editor-input").value,
-      "",
-      "it clears the input"
-    );
+    assert.dom(".d-editor-input").hasNoValue("it clears the input");
+
     await click(".topic-post:nth-of-type(1) button.edit");
     assert.ok(
       query(".d-editor-input").value.startsWith("This is the first post."),
@@ -592,6 +590,25 @@ acceptance("Composer", function (needs) {
     );
   });
 
+  test("Composer can toggle between edit and reply on a reply", async function (assert) {
+    await visit("/t/this-is-a-test-topic/9");
+
+    await click(".topic-post:nth-of-type(2) button.edit");
+    assert.ok(
+      query(".d-editor-input").value.startsWith("This is the second post."),
+      "it populates the input with the post text"
+    );
+
+    await click(".topic-post:nth-of-type(2) button.reply");
+    assert.dom(".d-editor-input").hasNoValue("it clears the input");
+
+    await click(".topic-post:nth-of-type(2) button.edit");
+    assert.ok(
+      query(".d-editor-input").value.startsWith("This is the second post."),
+      "it populates the input with the post text"
+    );
+  });
+
   test("Composer can toggle whispers when whisperer user", async function (assert) {
     const menu = selectKit(".toolbar-popup-menu-options");
 
@@ -798,12 +815,9 @@ acceptance("Composer", function (needs) {
         i18n("post.cancel_composer.keep_editing"),
         "has keep editing button"
       );
+
     await click(".d-modal__footer button.save-draft");
-    assert.strictEqual(
-      query(".d-editor-input").value,
-      "",
-      "it clears the composer input"
-    );
+    assert.dom(".d-editor-input").hasNoValue("it clears the input");
   });
 
   test("Checks for existing draft", async function (assert) {

spec/system/composer/discard_draft_spec.rb

@@ -1,68 +1,72 @@
 # frozen_string_literal: true
 
 describe "Composer - discard draft modal", type: :system do
-  fab!(:current_user) { Fabricate(:admin) }
-  fab!(:topic_1) { Fabricate(:topic) }
+  fab!(:topic)
+  fab!(:user) { Fabricate(:admin) }
 
   let(:topic_page) { PageObjects::Pages::Topic.new }
-  let(:discard_draft_modal) { PageObjects::Modals::DiscardDraft.new }
   let(:composer) { PageObjects::Components::Composer.new }
+  let(:discard_draft_modal) { PageObjects::Modals::DiscardDraft.new }
 
-  before { sign_in(current_user) }
+  before { sign_in(user) }
 
   context "when editing different post" do
-    fab!(:post_1) { Fabricate(:post, topic: topic_1, user: current_user) }
-    fab!(:post_2) { Fabricate(:post, topic: topic_1, user: current_user) }
+    fab!(:post_1) { Fabricate(:post, topic:, user:) }
+    fab!(:post_2) { Fabricate(:post, topic:, user:) }
 
-    it "shows the discard modal" do
+    it "shows the discard modal when there are changes" do
       topic_page.visit_topic(post_1.topic)
       topic_page.click_post_action_button(post_1, :edit)
-
       composer.fill_content("a b c d e f g")
       composer.minimize
+
       topic_page.click_post_action_button(post_2, :edit)
 
       expect(discard_draft_modal).to be_open
     end
-  end
 
-  context "when re-editing the first post" do
-    fab!(:post_1) { Fabricate(:post, topic: topic_1, user: current_user) }
-
-    it "doesn’t show the discard modal" do
+    it "doesn't show the discard modal when there's no changes" do
       topic_page.visit_topic(post_1.topic)
       topic_page.click_post_action_button(post_1, :edit)
-
-      composer.fill_content("a b c d e f g")
       composer.minimize
-      topic_page.click_post_action_button(post_1, :edit)
 
-      expect(discard_draft_modal).to be_closed
-      expect(composer).to be_opened
-
-      topic_page.click_post_action_button(post_1, :edit)
+      topic_page.click_post_action_button(post_2, :edit)
 
       expect(discard_draft_modal).to be_closed
       expect(composer).to be_opened
     end
   end
 
-  context "when re-editing the second post" do
-    fab!(:post_1) { Fabricate(:post, topic: topic_1, user: current_user) }
-    fab!(:post_2) { Fabricate(:post, topic: topic_1, user: current_user) }
+  context "when editing the same post" do
+    fab!(:post_1) { Fabricate(:post, topic:, user:) }
 
-    it "doesn’t show the discard modal" do
+    it "doesn’t show the discard modal even if there are changes" do
       topic_page.visit_topic(post_1.topic)
-      topic_page.click_post_action_button(post_2, :edit)
-
+      topic_page.click_post_action_button(post_1, :edit)
       composer.fill_content("a b c d e f g")
       composer.minimize
-      topic_page.click_post_action_button(post_2, :edit)
+
+      topic_page.click_post_action_button(post_1, :edit)
 
       expect(discard_draft_modal).to be_closed
       expect(composer).to be_opened
+      expect(composer).to have_content("a b c d e f g")
 
-      topic_page.click_post_action_button(post_2, :edit)
+      composer.minimize
+      expect(composer).to be_minimized
+
+      topic_page.click_post_action_button(post_1, :edit)
+
+      expect(discard_draft_modal).to be_closed
+      expect(composer).to be_opened
+    end
+
+    it "doesn’t show the discard modal when there's no changes" do
+      topic_page.visit_topic(post_1.topic)
+      topic_page.click_post_action_button(post_1, :edit)
+      composer.minimize
+
+      topic_page.click_post_action_button(post_1, :edit)
 
       expect(discard_draft_modal).to be_closed
       expect(composer).to be_opened

spec/system/page_objects/components/composer.rb

@@ -14,6 +14,10 @@ module PageObjects
         page.has_css?("#{COMPOSER_ID}.closed", visible: :all)
       end
 
+      def minimized?
+        page.has_css?("#{COMPOSER_ID}.draft")
+      end
+
       def open_composer_actions
         find(".composer-action-title .btn").click
         self