FIX: Username validation in create-account modal (#24114)

* Move the create account test
* Clean up username-validation
* Fix the username validation bug

app/assets/javascripts/discourse/app/mixins/username-validation.js

@@ -4,55 +4,65 @@ import { isEmpty } from "@ember/utils";
 import { setting } from "discourse/lib/computed";
 import User from "discourse/models/user";
 import discourseDebounce from "discourse-common/lib/debounce";
-import { observes } from "discourse-common/utils/decorators";
+import discourseComputed from "discourse-common/utils/decorators";
 import I18n from "discourse-i18n";
 
 function failedResult(attrs) {
-  let result = EmberObject.create({
+  return EmberObject.create({
     shouldCheck: false,
     failed: true,
     ok: false,
     element: document.querySelector("#new-account-username"),
+    ...attrs,
   });
-  result.setProperties(attrs);
-  return result;
 }
 
 function validResult(attrs) {
-  let result = EmberObject.create({ ok: true });
+  return EmberObject.create({ ok: true, ...attrs });
-  result.setProperties(attrs);
-  return result;
 }
 
 export default Mixin.create({
+  checkedUsername: null,
+  usernameValidationResult: null,
   uniqueUsernameValidation: null,
   maxUsernameLength: setting("max_username_length"),
   minUsernameLength: setting("min_username_length"),
 
-  fetchExistingUsername() {
+  async fetchExistingUsername() {
-    User.checkUsername(null, this.accountEmail).then((result) => {
+    const result = await User.checkUsername(null, this.accountEmail);
-      if (
+
-        result.suggestion &&
+    if (
-        (isEmpty(this.accountUsername) ||
+      result.suggestion &&
-          this.accountUsername === this.get("authOptions.username"))
+      (isEmpty(this.accountUsername) ||
-      ) {
+        this.accountUsername === this.get("authOptions.username"))
-        this.setProperties({
+    ) {
-          accountUsername: result.suggestion,
+      this.setProperties({
-          prefilledUsername: result.suggestion,
+        accountUsername: result.suggestion,
-        });
+        prefilledUsername: result.suggestion,
-      }
+      });
-    });
+    }
   },
 
-  @observes("accountUsername")
+  @discourseComputed(
-  triggerValidation() {
+    "usernameValidationResult",
-    let { accountUsername } = this;
+    "accountUsername",
+    "forceValidationReason"
+  )
+  usernameValidation() {
+    if (
+      this.usernameValidationResult &&
+      this.checkedUsername === this.accountUsername
+    ) {
+      return this.usernameValidationResult;
+    }
+
+    const result = this.basicUsernameValidation(this.accountUsername);
 
-    let result = this.basicUsernameValidation(accountUsername);
     if (result.shouldCheck) {
       discourseDebounce(this, this.checkUsernameAvailability, 500);
     }
-    this.set("usernameValidation", result);
+
+    return result;
   },
 
   basicUsernameValidation(username) {
@@ -83,42 +93,40 @@ export default Mixin.create({
     });
   },
 
-  checkUsernameAvailability() {
+  async checkUsernameAvailability() {
-    return User.checkUsername(this.accountUsername, this.accountEmail).then(
+    const result = await User.checkUsername(
-      (result) => {
+      this.accountUsername,
-        if (this.isDestroying || this.isDestroyed) {
+      this.accountEmail
-          return;
-        }
-
-        this.set("isDeveloper", false);
-        if (result.available) {
-          if (result.is_developer) {
-            this.set("isDeveloper", true);
-          }
-          return this.set(
-            "usernameValidation",
-            validResult({ reason: I18n.t("user.username.available") })
-          );
-        } else {
-          if (result.suggestion) {
-            return this.set(
-              "usernameValidation",
-              failedResult({
-                reason: I18n.t("user.username.not_available", result),
-              })
-            );
-          } else {
-            return this.set(
-              "usernameValidation",
-              failedResult({
-                reason: result.errors
-                  ? result.errors.join(" ")
-                  : I18n.t("user.username.not_available_no_suggestion"),
-              })
-            );
-          }
-        }
-      }
     );
+
+    if (this.isDestroying || this.isDestroyed) {
+      return;
+    }
+
+    this.set("checkedUsername", this.accountUsername);
+    this.set("isDeveloper", !!result.is_developer);
+
+    if (result.available) {
+      this.set(
+        "usernameValidationResult",
+        validResult({ reason: I18n.t("user.username.available") })
+      );
+    } else if (result.suggestion) {
+      this.set(
+        "usernameValidationResult",
+        failedResult({
+          reason: I18n.t("user.username.not_available", result),
+        })
+      );
+    } else {
+      this.set(
+        "usernameValidationResult",
+        failedResult({
+          reason: result.errors
+            ? result.errors.join(" ")
+            : I18n.t("user.username.not_available_no_suggestion"),
+        })
+      );
+    }
   },
 });

app/assets/javascripts/discourse/tests/acceptance/create-account-test.js

@@ -0,0 +1,50 @@
+import { click, fillIn, visit } from "@ember/test-helpers";
+import { test } from "qunit";
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
+import I18n from "discourse-i18n";
+
+acceptance("Create Account", function () {
+  test("create an account", async function (assert) {
+    await visit("/");
+    await click("header .sign-up-button");
+
+    assert.dom(".create-account").exists("it shows the create account modal");
+
+    await fillIn("#new-account-name", "Dr. Good Tuna");
+    await fillIn("#new-account-password", "cool password bro");
+
+    // without this double fill, field will sometimes being empty
+    // got consistent repro by having browser search bar focused when starting test
+    await fillIn("#new-account-email", "good.tuna@test.com");
+    await fillIn("#new-account-email", "good.tuna@test.com");
+
+    // Check username
+    await fillIn("#new-account-username", "taken");
+    assert
+      .dom("#username-validation.bad")
+      .exists("the username validation is bad");
+    await click(".modal-footer .btn-primary");
+
+    await fillIn("#new-account-username", "good-tuna");
+    assert
+      .dom("#username-validation.good")
+      .exists("the username validation is good");
+
+    await click(".modal-footer .btn-primary");
+    assert
+      .dom(".modal-footer .btn-primary:disabled")
+      .exists("create account is disabled");
+  });
+
+  test("validate username", async function (assert) {
+    await visit("/");
+    await click("header .sign-up-button");
+
+    await fillIn("#new-account-email", "z@z.co");
+    await click(".modal-footer .btn-primary");
+
+    assert
+      .dom("#username-validation")
+      .hasText(I18n.t("user.username.required"), "shows signup error");
+  });
+});

app/assets/javascripts/discourse/tests/acceptance/sign-in-test.js

@@ -149,41 +149,6 @@ acceptance("Signing In", function () {
     assert.notOk(exists("#login-button:visible"), "hides the login button");
   });
 
-  test("create account", async function (assert) {
-    await visit("/");
-    await click("header .sign-up-button");
-
-    assert.ok(exists(".create-account"), "it shows the create account modal");
-
-    await fillIn("#new-account-name", "Dr. Good Tuna");
-    await fillIn("#new-account-password", "cool password bro");
-
-    // without this double fill, field will sometimes being empty
-    // got consistent repro by having browser search bar focused when starting test
-    await fillIn("#new-account-email", "good.tuna@test.com");
-    await fillIn("#new-account-email", "good.tuna@test.com");
-
-    // Check username
-    await fillIn("#new-account-username", "taken");
-    assert.ok(
-      exists("#username-validation.bad"),
-      "the username validation is bad"
-    );
-    await click(".modal-footer .btn-primary");
-
-    await fillIn("#new-account-username", "good-tuna");
-    assert.ok(
-      exists("#username-validation.good"),
-      "the username validation is good"
-    );
-
-    await click(".modal-footer .btn-primary");
-    assert.ok(
-      exists(".modal-footer .btn-primary:disabled"),
-      "create account is disabled"
-    );
-  });
-
   test("second factor backup - valid token", async function (assert) {
     await visit("/");
     await click("header .login-button");