From df04a99db932c7afb2ea65ee0dd535f8635e7c61 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Tue, 27 Jun 2023 08:03:51 +0800
Subject: [PATCH] FIX: Abort transition when `transition.from` present on
 new-topic route (#22253)

Why is this change required?

The `/new-topic` route is a special route which we use to open the
composer by loading a URL. By default, the `new-topic` route is replaced with the
`discovery.latest` route. On a fresh page load, this makes sense since
there is no template for the `new-topic` route to render. However, this
behavior does not make sense if we're transition from another route.
There is no need to replace the current route with the `discovery.latest` when all we want
is to open the composer.

What does this commit do?

This commit fixes the undesirable behaviour described above by aborting
the existing transition to the `new-topic` route if `transition.from` is
present. This indicates that we're navigating from an existing route and
we can just open the composer.
---
 .../discourse/app/components/composer-body.js |  7 +--
 .../discourse/app/routes/new-topic.js         | 15 ++++--
 .../tests/acceptance/new-topic-test.js        | 52 +++++++++++++------
 3 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/composer-body.js b/app/assets/javascripts/discourse/app/components/composer-body.js
index 2ec0b47aafc..8b6f8f8da1e 100644
--- a/app/assets/javascripts/discourse/app/components/composer-body.js
+++ b/app/assets/javascripts/discourse/app/components/composer-body.js
@@ -117,14 +117,15 @@ export default Component.extend(KeyEnterEscape, {
   @observes("composeState", "composer.{action,canEditTopicFeaturedLink}")
   _triggerComposerResized() {
     schedule("afterRender", () => {
-      if (!this.element || this.isDestroying || this.isDestroyed) {
-        return;
-      }
       discourseDebounce(this, this.composerResized, 300);
     });
   },
 
   composerResized() {
+    if (!this.element || this.isDestroying || this.isDestroyed) {
+      return;
+    }
+
     this.appEvents.trigger("composer:resized");
   },
 
diff --git a/app/assets/javascripts/discourse/app/routes/new-topic.js b/app/assets/javascripts/discourse/app/routes/new-topic.js
index abf30681a7c..f66848f90b9 100644
--- a/app/assets/javascripts/discourse/app/routes/new-topic.js
+++ b/app/assets/javascripts/discourse/app/routes/new-topic.js
@@ -43,11 +43,16 @@ export default DiscourseRoute.extend({
           }
         });
       } else {
-        this.replaceWith("discovery.latest").then((e) => {
-          if (this.controllerFor("navigation/default").canCreateTopic) {
-            this._sendTransition(e, transition);
-          }
-        });
+        if (transition.from) {
+          transition.abort();
+          this.send("createNewTopicViaParams");
+        } else {
+          this.replaceWith("discovery.latest").then((e) => {
+            if (this.controllerFor("navigation/default").canCreateTopic) {
+              this._sendTransition(e, transition);
+            }
+          });
+        }
       }
     } else {
       // User is not logged in
diff --git a/app/assets/javascripts/discourse/tests/acceptance/new-topic-test.js b/app/assets/javascripts/discourse/tests/acceptance/new-topic-test.js
index 18daaab0a5a..671e33dacf1 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/new-topic-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/new-topic-test.js
@@ -1,11 +1,7 @@
-import {
-  acceptance,
-  exists,
-  query,
-} from "discourse/tests/helpers/qunit-helpers";
+import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
 import { test } from "qunit";
-import { visit } from "@ember/test-helpers";
+import { currentURL, visit, waitFor } from "@ember/test-helpers";
 
 acceptance("New Topic - Anonymous", function () {
   test("accessing new-topic route when logged out", async function (assert) {
@@ -17,26 +13,48 @@ acceptance("New Topic - Anonymous", function () {
 
 acceptance("New Topic - Authenticated", function (needs) {
   needs.user();
-  test("accessing new-topic route when logged in", async function (assert) {
+
+  test("accessing new-topic route", async function (assert) {
+    await visit("/c/1");
+
+    try {
+      await visit("/new-topic");
+    } catch (error) {
+      assert.strictEqual(
+        error.message,
+        "TransitionAborted",
+        "it aborts the transition"
+      );
+    }
+
+    assert.strictEqual(currentURL(), "/c/1");
+
+    await waitFor(".composer-fields", { timeout: 5000 });
+
+    assert.dom(".composer-fields").exists("it opens the composer");
+  });
+
+  test("accessing new-topic route with title, body and category param", async function (assert) {
     await visit(
       "/new-topic?title=topic%20title&body=topic%20body&category=bug"
     );
 
     assert.ok(exists(".composer-fields"), "it opens composer");
-    assert.strictEqual(
-      query("#reply-title").value.trim(),
-      "topic title",
-      "it pre-fills topic title"
-    );
-    assert.strictEqual(
-      query(".d-editor-input").value.trim(),
-      "topic body",
-      "it pre-fills topic body"
-    );
+
+    assert
+      .dom("#reply-title")
+      .hasValue("topic title", "it pre-fills the topic title");
+
+    assert
+      .dom(".d-editor-input")
+      .hasValue("topic body", "it pre-fills topic body");
+
     assert.strictEqual(
       selectKit(".category-chooser").header().value(),
       "1",
       "it selects desired category"
     );
+
+    assert.strictEqual(currentURL(), "/c/1");
   });
 });