FIX: use full screen login for new-topic route (#7467)

FIX: use full screen login for new-topic route (#7467)

DEV: add javascript tests for new-topic and new-message routes

DEV: fix an existing test that was being skipped

diff --git a/app/assets/javascripts/discourse.js.es6 b/app/assets/javascripts/discourse.js.es6
index b1784a9..9d4b5d4 100644
--- a/app/assets/javascripts/discourse.js.es6
+++ b/app/assets/javascripts/discourse.js.es6
@@ -11,7 +11,7 @@ const Discourse = Ember.Application.extend({
   _docTitle: document.title,
   RAW_TEMPLATES: {},
   __widget_helpers: {},
-  showingSignup: false,
+  useFullScreenLogin: false,
   customEvents: {
     paste: "paste"
   },
diff --git a/app/assets/javascripts/discourse/models/login-method.js.es6 b/app/assets/javascripts/discourse/models/login-method.js.es6
index 29bd3b1..d60b492 100644
--- a/app/assets/javascripts/discourse/models/login-method.js.es6
+++ b/app/assets/javascripts/discourse/models/login-method.js.es6
@@ -93,7 +93,8 @@ export function findAll(siteSettings, capabilities, isMobileDevice) {
   // On Mobile, Android or iOS always go with full screen
   if (
     isMobileDevice ||
-    (capabilities && (capabilities.isIOS || capabilities.isAndroid))
+    (capabilities && (capabilities.isIOS || capabilities.isAndroid)) ||
+    Discourse.useFullScreenLogin
   ) {
     methods.forEach(m => m.set("full_screen_login", true));
   }
diff --git a/app/assets/javascripts/discourse/routes/new-message.js.es6 b/app/assets/javascripts/discourse/routes/new-message.js.es6
index f926ba3..3dae659 100644
--- a/app/assets/javascripts/discourse/routes/new-message.js.es6
+++ b/app/assets/javascripts/discourse/routes/new-message.js.es6
@@ -55,11 +55,8 @@ export default Discourse.Route.extend({
       });
     } else {
       $.cookie("destination_url", window.location.href);
-      if (Discourse.showingSignup) {
-        Discourse.showingSignup = false;
-      } else {
-        this.replaceWith("login");
-      }
+      Discourse.useFullScreenLogin = true;
+      this.replaceWith("login");
     }
   }
 });
diff --git a/app/assets/javascripts/discourse/routes/new-topic.js.es6 b/app/assets/javascripts/discourse/routes/new-topic.js.es6
index 947193b..69e705a 100644
--- a/app/assets/javascripts/discourse/routes/new-topic.js.es6
+++ b/app/assets/javascripts/discourse/routes/new-topic.js.es6
@@ -72,12 +72,8 @@ export default Discourse.Route.extend({
     } else {
       // User is not logged in
       $.cookie("destination_url", window.location.href);
-      if (Discourse.showingSignup) {
-        // We're showing the sign up modal
-        Discourse.showingSignup = false;
-      } else {
-        self.replaceWith("login");
-      }
+      Discourse.useFullScreenLogin = true;
+      self.replaceWith("login");
     }
   },
 
diff --git a/test/javascripts/acceptance/category-chooser-test.js.es6 b/test/javascripts/acceptance/category-chooser-test.js.es6
index 4c09552..83b1af2 100644
--- a/test/javascripts/acceptance/category-chooser-test.js.es6
+++ b/test/javascripts/acceptance/category-chooser-test.js.es6
@@ -18,9 +18,7 @@ QUnit.test("does not display uncategorized if not allowed", async assert => {
   assert.ok(categoryChooser.rowByIndex(0).name() !== "uncategorized");
 });
 
-// TODO: fix the test to work with new code to land on category page
-// (https://github.com/discourse/discourse/commit/7d9c97d66141d35d00258fe544211d9fd7f79a76)
-QUnit.skip("prefill category when category_id is set", async assert => {
+QUnit.test("prefill category when category_id is set", async assert => {
   await visit("/new-topic?category_id=1");
 
   assert.equal(
diff --git a/test/javascripts/acceptance/new-message-test.js.es6 b/test/javascripts/acceptance/new-message-test.js.es6
new file mode 100644
index 0000000..9c536ab
--- /dev/null
+++ b/test/javascripts/acceptance/new-message-test.js.es6
@@ -0,0 +1,43 @@
+import { acceptance, logIn } from "helpers/qunit-helpers";
+
+acceptance("New Message");
+
+QUnit.test("accessing new-message route when logged out", async assert => {
+  await visit(
+    "/new-message?username=eviltrout&title=message%20title&body=message%20body"
+  );
+
+  assert.ok(exists(".modal.login-modal"), "it shows the login modal");
+});
+
+QUnit.test("accessing new-message route when logged in", async assert => {
+  logIn();
+  Discourse.reset();
+
+  await visit(
+    "/new-message?username=eviltrout&title=message%20title&body=message%20body"
+  );
+
+  assert.ok(exists(".composer-fields"), "it opens composer");
+  assert.equal(
+    find("#reply-title")
+      .val()
+      .trim(),
+    "message title",
+    "it pre-fills message title"
+  );
+  assert.equal(
+    find(".d-editor-input")
+      .val()
+      .trim(),
+    "message body",
+    "it pre-fills message body"
+  );
+  assert.equal(
+    find(".users-input .item:eq(0)")
+      .text()
+      .trim(),
+    "eviltrout",
+    "it selects correct username"
+  );
+});
diff --git a/test/javascripts/acceptance/new-topic-test.js.es6 b/test/javascripts/acceptance/new-topic-test.js.es6
new file mode 100644
index 0000000..1ad6fc0
--- /dev/null
+++ b/test/javascripts/acceptance/new-topic-test.js.es6
@@ -0,0 +1,39 @@
+import { acceptance, logIn } from "helpers/qunit-helpers";
+
+acceptance("New Topic");
+
+QUnit.test("accessing new-topic route when logged out", async assert => {
+  await visit("/new-topic?title=topic%20title&body=topic%20body");
+
+  assert.ok(exists(".modal.login-modal"), "it shows the login modal");
+});
+
+QUnit.test("accessing new-topic route when logged in", async assert => {
+  logIn();
+  Discourse.reset();
+
+  await visit("/new-topic?title=topic%20title&body=topic%20body&category=bug");
+
+  assert.ok(exists(".composer-fields"), "it opens composer");
+  assert.equal(
+    find("#reply-title")
+      .val()
+      .trim(),
+    "topic title",
+    "it pre-fills topic title"
+  );
+  assert.equal(
+    find(".d-editor-input")
+      .val()
+      .trim(),
+    "topic body",
+    "it pre-fills topic body"
+  );
+  assert.equal(
+    selectKit(".category-chooser")
+      .header()
+      .value(),
+    1,
+    "it selects desired category"
+  );
+});
diff --git a/test/javascripts/fixtures/user_fixtures.js.es6 b/test/javascripts/fixtures/user_fixtures.js.es6
index 954fb14..c9fb2f0 100644
--- a/test/javascripts/fixtures/user_fixtures.js.es6
+++ b/test/javascripts/fixtures/user_fixtures.js.es6
@@ -150,7 +150,7 @@ export default {
         { action_type: 11, count: 20, id: null }
       ],
       can_send_private_messages: true,
-      can_send_private_message_to_user: false,
+      can_send_private_message_to_user: true,
       bio_excerpt:
         '<p>Co-founder of Discourse. Previously, I created <a href="http://forumwarz.com">Forumwarz</a>. <a href="https://twitter.com/eviltrout">Follow me on Twitter</a>. I am <a class="mention" href="/u/eviltrout">@eviltrout</a>.</p>',
       trust_level: 4,
diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6
index a7313a8..2054a13 100644
--- a/test/javascripts/helpers/create-pretender.js.es6
+++ b/test/javascripts/helpers/create-pretender.js.es6
@@ -62,6 +62,18 @@ export default function() {
       return response(json);
     });
 
+    this.get("/c/bug/l/latest.json", () => {
+      const json = fixturesByUrl["/c/bug/l/latest.json"];
+
+      if (loggedIn()) {
+        // Stuff to let us post
+        json.topic_list.can_create_topic = true;
+        json.topic_list.draft_key = "new_topic";
+        json.topic_list.draft_sequence = 1;
+      }
+      return response(json);
+    });
+
     this.get("/tags", () => {
       return response({
         tags: [

GitHub sha: b5ea50a1

This commit has been mentioned on Discourse Meta. There might be relevant details there:

I think we need to do the opposite.

We ALWAYS do full screen login.

Unless you click on one of these:

1 Like

@SamSaffron I believe that’s what is happening after this change?

Here is /new-topic login flow when both GitHub and local logins are enabled:

… and here is /new-topic login flow when only GitHub logins are enabled:

1 Like

I am confused, why is the click doing full screen in the first example?

Discourse.useFullScreenLogin = true;

Super mega uneasy about adding a global here.

Simply always use full screen, unless given explicit instructions not to, something that happens when you click “github” logo etc in the click handler.

2 Likes

Aha, I understand what you mean now. Will do.

This will also result in nullifying my global useFullScreenLogin hack. :+1:

2 Likes

Followed up in:

2 Likes