From d3649873a208833a5db23c5b1f6c1442681a485b Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 7 Dec 2022 11:16:01 +0100 Subject: [PATCH] DEV: Replace preferences/username route with a component (#19318) That was a weird UX (why hide the preferences navigation?) and a deprecated implementation (manually rendering a template into a named outlet) This PR replaces it with an inline component. --- .../app/components/username-preference.hbs | 55 ++++++++++ .../app/components/username-preference.js | 100 ++++++++++++++++++ .../app/controllers/preferences/username.js | 91 ---------------- .../discourse/app/routes/app-route-map.js | 1 - .../app/routes/preferences-username.js | 26 ----- .../app/templates/preferences-username.hbs | 38 ------- .../app/templates/preferences/account.hbs | 14 +-- .../tests/acceptance/preferences-test.js | 5 - .../user-preferences-account-test.js | 42 +++++++- 9 files changed, 196 insertions(+), 176 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/username-preference.hbs create mode 100644 app/assets/javascripts/discourse/app/components/username-preference.js delete mode 100644 app/assets/javascripts/discourse/app/controllers/preferences/username.js delete mode 100644 app/assets/javascripts/discourse/app/routes/preferences-username.js delete mode 100644 app/assets/javascripts/discourse/app/templates/preferences-username.hbs diff --git a/app/assets/javascripts/discourse/app/components/username-preference.hbs b/app/assets/javascripts/discourse/app/components/username-preference.hbs new file mode 100644 index 0000000000..42cb6f854d --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/username-preference.hbs @@ -0,0 +1,55 @@ +{{#if this.editing}} +
+
+ + +
+

+ {{#if this.taken}} + {{i18n "user.change_username.taken"}} + {{/if}} + {{this.errorMessage}} +

+
+
+ +
+ + + + + {{#if this.saved}}{{i18n "saved"}}{{/if}} +
+
+{{else}} +
+ {{@user.username}} + + {{#if @user.can_edit_username}} + + {{/if}} +
+ + {{#if this.siteSettings.enable_mentions}} +
+ {{html-safe (i18n "user.username.short_instructions" username=@user.username)}} +
+ {{/if}} +{{/if}} diff --git a/app/assets/javascripts/discourse/app/components/username-preference.js b/app/assets/javascripts/discourse/app/components/username-preference.js new file mode 100644 index 0000000000..b2ac7012de --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/username-preference.js @@ -0,0 +1,100 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import DiscourseURL, { userPath } from "discourse/lib/url"; +import { empty, or } from "@ember/object/computed"; +import { setting } from "discourse/lib/computed"; +import I18n from "I18n"; +import User from "discourse/models/user"; +import { isEmpty } from "@ember/utils"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { inject as service } from "@ember/service"; +import { action } from "@ember/object"; + +export default class UsernamePreference extends Component { + @service siteSettings; + @service dialog; + + @tracked editing = false; + @tracked newUsername = this.args.user.username; + @tracked errorMessage = null; + @tracked saving = false; + @tracked taken = false; + + @setting("max_username_length") maxLength; + @setting("min_username_length") minLength; + @empty("newUsername") newUsernameEmpty; + + @or("saving", "newUsernameEmpty", "taken", "unchanged", "errorMessage") + saveDisabled; + + get unchanged() { + return this.newUsername === this.args.user.username; + } + + get saveButtonText() { + return this.saving ? I18n.t("saving") : I18n.t("user.change"); + } + + @action + toggleEditing() { + this.editing = !this.editing; + + this.newUsername = this.args.user.username; + this.errorMessage = null; + this.saving = false; + this.taken = false; + } + + @action + async onInput(event) { + this.newUsername = event.target.value; + this.taken = false; + this.errorMessage = null; + + if (isEmpty(this.newUsername)) { + return; + } + + if (this.newUsername === this.args.user.username) { + return; + } + + if (this.newUsername.length < this.minLength) { + this.errorMessage = I18n.t("user.name.too_short"); + return; + } + + const result = await User.checkUsername( + this.newUsername, + undefined, + this.args.user.id + ); + + if (result.errors) { + this.errorMessage = result.errors.join(" "); + } else if (result.available === false) { + this.taken = true; + } + } + + @action + changeUsername() { + return this.dialog.yesNoConfirm({ + title: I18n.t("user.change_username.confirm"), + didConfirm: async () => { + this.saving = true; + + try { + await this.args.user.changeUsername(this.newUsername); + DiscourseURL.redirectTo( + userPath(this.newUsername.toLowerCase() + "/preferences") + ); + } catch (e) { + popupAjaxError(e); + } finally { + this.saving = false; + } + }, + }); + } +} diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/username.js b/app/assets/javascripts/discourse/app/controllers/preferences/username.js deleted file mode 100644 index cb756dbb52..0000000000 --- a/app/assets/javascripts/discourse/app/controllers/preferences/username.js +++ /dev/null @@ -1,91 +0,0 @@ -import DiscourseURL, { userPath } from "discourse/lib/url"; -import discourseComputed, { observes } from "discourse-common/utils/decorators"; -import { empty, or } from "@ember/object/computed"; -import { propertyEqual, setting } from "discourse/lib/computed"; -import Controller from "@ember/controller"; -import I18n from "I18n"; -import User from "discourse/models/user"; -import { isEmpty } from "@ember/utils"; -import { popupAjaxError } from "discourse/lib/ajax-error"; -import { inject as service } from "@ember/service"; - -export default Controller.extend({ - dialog: service(), - taken: false, - saving: false, - errorMessage: null, - newUsername: null, - - maxLength: setting("max_username_length"), - minLength: setting("min_username_length"), - newUsernameEmpty: empty("newUsername"), - saveDisabled: or( - "saving", - "newUsernameEmpty", - "taken", - "unchanged", - "errorMessage" - ), - unchanged: propertyEqual("newUsername", "username"), - - @observes("newUsername") - checkTaken() { - let newUsername = this.newUsername; - - if (newUsername && newUsername.length < this.minLength) { - this.set("errorMessage", I18n.t("user.name.too_short")); - } else { - this.set("taken", false); - this.set("errorMessage", null); - - if (isEmpty(this.newUsername)) { - return; - } - if (this.unchanged) { - return; - } - - User.checkUsername(newUsername, undefined, this.get("model.id")).then( - (result) => { - if (result.errors) { - this.set("errorMessage", result.errors.join(" ")); - } else if (result.available === false) { - this.set("taken", true); - } - } - ); - } - }, - - @discourseComputed("saving") - saveButtonText(saving) { - if (saving) { - return I18n.t("saving"); - } - return I18n.t("user.change"); - }, - - actions: { - changeUsername() { - if (this.saveDisabled) { - return; - } - - return this.dialog.yesNoConfirm({ - title: I18n.t("user.change_username.confirm"), - didConfirm: () => { - this.set("saving", true); - this.model - .changeUsername(this.newUsername) - .then(() => { - DiscourseURL.redirectTo( - userPath(this.newUsername.toLowerCase() + "/preferences") - ); - }) - .catch(popupAjaxError) - .finally(() => this.set("saving", false)); - }, - }); - }, - }, -}); diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index 24baedb7a5..b30a365ca6 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -175,7 +175,6 @@ export default function () { this.route("apps"); this.route("sidebar"); - this.route("username"); this.route("email"); this.route("second-factor"); this.route("second-factor-backup"); diff --git a/app/assets/javascripts/discourse/app/routes/preferences-username.js b/app/assets/javascripts/discourse/app/routes/preferences-username.js deleted file mode 100644 index 1bf9b425a7..0000000000 --- a/app/assets/javascripts/discourse/app/routes/preferences-username.js +++ /dev/null @@ -1,26 +0,0 @@ -import RestrictedUserRoute from "discourse/routes/restricted-user"; - -export default RestrictedUserRoute.extend({ - showFooter: true, - - model() { - return this.modelFor("user"); - }, - - renderTemplate() { - return this.render({ into: "user" }); - }, - - // A bit odd, but if we leave to /preferences we need to re-render that outlet - deactivate() { - this._super(...arguments); - this.render("preferences", { into: "user", controller: "preferences" }); - }, - - setupController(controller, user) { - controller.setProperties({ - model: user, - newUsername: user.get("username"), - }); - }, -}); diff --git a/app/assets/javascripts/discourse/app/templates/preferences-username.hbs b/app/assets/javascripts/discourse/app/templates/preferences-username.hbs deleted file mode 100644 index 9d4e1ec997..0000000000 --- a/app/assets/javascripts/discourse/app/templates/preferences-username.hbs +++ /dev/null @@ -1,38 +0,0 @@ - -
-
- -
-
-

{{i18n "user.change_username.title"}}

-
-
- -
- -
- -
-
-

- {{#if this.taken}} - {{i18n "user.change_username.taken"}} - {{/if}} - {{this.errorMessage}} -

-
-
- -
-
- - - {{i18n "cancel"}} - - {{#if this.saved}}{{i18n "saved"}}{{/if}} -
-
- -
-
- diff --git a/app/assets/javascripts/discourse/app/templates/preferences/account.hbs b/app/assets/javascripts/discourse/app/templates/preferences/account.hbs index c8a33c91fd..90f67422aa 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/account.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/account.hbs @@ -1,18 +1,6 @@
-
- {{this.model.username}} - {{#if this.model.can_edit_username}} - - {{d-icon "pencil-alt"}} - - {{/if}} -
- {{#if this.siteSettings.enable_mentions}} -
- {{html-safe (i18n "user.username.short_instructions" username=this.model.username)}} -
- {{/if}} +
{{#unless this.siteSettings.discourse_connect_overrides_avatar}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js b/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js index 6f60d307d7..9b6e3089cb 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js @@ -111,11 +111,6 @@ acceptance("User Preferences", function (needs) { "apps tab isn't there when you have no authorized apps" ); }); - - test("username", async function (assert) { - await visit("/u/eviltrout/preferences/username"); - assert.ok(exists("#change_username"), "it has the input element"); - }); }); acceptance("Custom User Fields", function (needs) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-account-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-account-test.js index e33b9ccb45..83d8162c6c 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-account-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-account-test.js @@ -1,14 +1,17 @@ import { test } from "qunit"; import I18n from "I18n"; import sinon from "sinon"; -import { click, visit } from "@ember/test-helpers"; +import { click, fillIn, visit } from "@ember/test-helpers"; import { acceptance, exists, query, } from "discourse/tests/helpers/qunit-helpers"; import DiscourseURL from "discourse/lib/url"; -import { fixturesByUrl } from "discourse/tests/helpers/create-pretender"; +import pretender, { + fixturesByUrl, + response, +} from "discourse/tests/helpers/create-pretender"; import { cloneJSON } from "discourse-common/lib/object"; acceptance("User Preferences - Account", function (needs) { @@ -58,6 +61,41 @@ acceptance("User Preferences - Account", function (needs) { pickAvatarRequestData = null; }); + test("changing username", async function (assert) { + const stub = sinon + .stub(DiscourseURL, "redirectTo") + .withArgs("/u/good_trout/preferences"); + + pretender.put("/u/eviltrout/preferences/username", (data) => { + assert.strictEqual(data.requestBody, "new_username=good_trout"); + + return response({ + id: fixturesByUrl["/u/eviltrout.json"].user.id, + username: "good_trout", + }); + }); + + await visit("/u/eviltrout/preferences/account"); + + assert.strictEqual( + query(".username-preference__current-username").innerText, + "eviltrout" + ); + + await click(".username-preference__edit-username"); + + assert.strictEqual(query(".username-preference__input").value, "eviltrout"); + assert.true(query(".username-preference__submit").disabled); + + await fillIn(query(".username-preference__input"), "good_trout"); + assert.false(query(".username-preference__submit").disabled); + + await click(".username-preference__submit"); + await click(".dialog-container .btn-primary"); + + sinon.assert.calledOnce(stub); + }); + test("Delete dialog", async function (assert) { sinon.stub(DiscourseURL, "redirectAbsolute");