From 6adc67a7a89e8ff80d94808c74973e8185dd2d17 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 27 Sep 2023 17:11:44 +0200 Subject: [PATCH] FIX: Broken error reporting in modals (and other places) (#23680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. actually call `popupAjaxError`, thanks :P 2. don't close a modal on error 3. use `extractError()` instead of manually joining error messages 4. …or passing just the error object to `this.flash` --- .../addon/components/modal/install-theme.js | 4 +-- .../admin/addon/components/modal/reseed.js | 5 +-- .../addon/components/theme-upload-add.js | 3 +- .../app/components/modal/activation-edit.js | 27 +++++++++------- .../app/components/modal/do-not-disturb.js | 3 +- .../app/components/modal/edit-slow-mode.js | 3 +- .../modal/edit-user-directory-columns.js | 4 +-- .../app/components/modal/group-add-members.js | 32 +++++++++++-------- .../discourse/app/routes/associate-account.js | 4 +-- .../components/chat/modal/create-channel.js | 22 ++++++------- .../chat/modal/edit-channel-description.js | 21 ++++++------ 11 files changed, 70 insertions(+), 58 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/modal/install-theme.js b/app/assets/javascripts/admin/addon/components/modal/install-theme.js index e4f15d08ab1..14cfb4c9d7b 100644 --- a/app/assets/javascripts/admin/addon/components/modal/install-theme.js +++ b/app/assets/javascripts/admin/addon/components/modal/install-theme.js @@ -163,8 +163,8 @@ export default class InstallTheme extends Component { await theme.save({ name: this.name, component: this.component }); this.args.model.addTheme(theme); this.args.closeModal(); - } catch { - popupAjaxError; + } catch (e) { + popupAjaxError(e); } finally { this.loading = false; } diff --git a/app/assets/javascripts/admin/addon/components/modal/reseed.js b/app/assets/javascripts/admin/addon/components/modal/reseed.js index be3f270deee..e2581ba20ca 100644 --- a/app/assets/javascripts/admin/addon/components/modal/reseed.js +++ b/app/assets/javascripts/admin/addon/components/modal/reseed.js @@ -44,12 +44,13 @@ export default class Reseed extends Component { }, type: "POST", }); + + this.flash = null; + this.args.closeModal(); } catch { this.flash = I18n.t("generic_error"); } finally { this.reseeding = false; - this.flash = null; - this.args.closeModal(); } } } diff --git a/app/assets/javascripts/admin/addon/components/theme-upload-add.js b/app/assets/javascripts/admin/addon/components/theme-upload-add.js index f2d9a895a9a..5ef4604b161 100644 --- a/app/assets/javascripts/admin/addon/components/theme-upload-add.js +++ b/app/assets/javascripts/admin/addon/components/theme-upload-add.js @@ -4,6 +4,7 @@ import { tracked } from "@glimmer/tracking"; import I18n from "I18n"; import { ajax } from "discourse/lib/ajax"; import { isEmpty } from "@ember/utils"; +import { extractError } from "discourse/lib/ajax-error"; const THEME_FIELD_VARIABLE_TYPE_IDS = [2, 3, 4]; const SCSS_VARIABLE_NAMES = [ @@ -108,7 +109,7 @@ export default class ThemeUploadAdd extends Component { this.args.model.addUpload(upload); this.args.closeModal(); } catch (e) { - this.flash = e.jqXHR.responseJSON.errors.join(". "); + this.flash = extractError(e); } } } diff --git a/app/assets/javascripts/discourse/app/components/modal/activation-edit.js b/app/assets/javascripts/discourse/app/components/modal/activation-edit.js index 7716f0c843d..eea72894655 100644 --- a/app/assets/javascripts/discourse/app/components/modal/activation-edit.js +++ b/app/assets/javascripts/discourse/app/components/modal/activation-edit.js @@ -4,6 +4,7 @@ import { inject as service } from "@ember/service"; import { tracked } from "@glimmer/tracking"; import { changeEmail } from "discourse/lib/user-activation"; import ActivationResent from "./activation-resent"; +import { extractError } from "discourse/lib/ajax-error"; export default class ActivationEdit extends Component { @service login; @@ -17,18 +18,20 @@ export default class ActivationEdit extends Component { } @action - changeEmail() { - changeEmail({ - username: this.login?.loginName, - password: this.login?.loginPassword, - email: this.args.model.newEmail, - }) - .then(() => { - this.modal.show(ActivationResent, { - model: { currentEmail: this.args.model.newEmail }, - }); - }) - .catch((e) => (this.flash = e)); + async changeEmail() { + try { + await changeEmail({ + username: this.login?.loginName, + password: this.login?.loginPassword, + email: this.args.model.newEmail, + }); + + this.modal.show(ActivationResent, { + model: { currentEmail: this.args.model.newEmail }, + }); + } catch (e) { + this.flash = extractError(e); + } } @action diff --git a/app/assets/javascripts/discourse/app/components/modal/do-not-disturb.js b/app/assets/javascripts/discourse/app/components/modal/do-not-disturb.js index 61df1bc86bd..8252ab7b25e 100644 --- a/app/assets/javascripts/discourse/app/components/modal/do-not-disturb.js +++ b/app/assets/javascripts/discourse/app/components/modal/do-not-disturb.js @@ -2,6 +2,7 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; import { tracked } from "@glimmer/tracking"; import { inject as service } from "@ember/service"; +import { extractError } from "discourse/lib/ajax-error"; export default class DoNotDisturb extends Component { @service currentUser; @@ -15,7 +16,7 @@ export default class DoNotDisturb extends Component { await this.currentUser.enterDoNotDisturbFor(duration); this.args.closeModal(); } catch (e) { - this.flash = e; + this.flash = extractError(e); } } diff --git a/app/assets/javascripts/discourse/app/components/modal/edit-slow-mode.js b/app/assets/javascripts/discourse/app/components/modal/edit-slow-mode.js index edc5ae8c4c8..018f770f488 100644 --- a/app/assets/javascripts/discourse/app/components/modal/edit-slow-mode.js +++ b/app/assets/javascripts/discourse/app/components/modal/edit-slow-mode.js @@ -6,6 +6,7 @@ import I18n from "I18n"; import { timeShortcuts } from "discourse/lib/time-shortcut"; import Topic from "discourse/models/topic"; import { inject as service } from "@ember/service"; +import { extractError } from "discourse/lib/ajax-error"; const SLOW_MODE_OPTIONS = [ { @@ -154,7 +155,7 @@ export default class EditSlowMode extends Component { this.args.model.topic.set("slow_mode_seconds", 0); this.args.closeModal(); } catch (e) { - this.flash = e; + this.flash = extractError(e); } finally { this.saveDisabled = false; } diff --git a/app/assets/javascripts/discourse/app/components/modal/edit-user-directory-columns.js b/app/assets/javascripts/discourse/app/components/modal/edit-user-directory-columns.js index 61f7f68f3b2..d21b57049a2 100644 --- a/app/assets/javascripts/discourse/app/components/modal/edit-user-directory-columns.js +++ b/app/assets/javascripts/discourse/app/components/modal/edit-user-directory-columns.js @@ -27,8 +27,8 @@ export default class EditUserDirectoryColumns extends Component { this.columns = response.directory_columns .sort((a, b) => (a.position > b.position ? 1 : -1)) .map((c) => ({ ...c, enabled: Boolean(c.enabled) })); - } catch { - popupAjaxError; + } catch (e) { + popupAjaxError(e); } } diff --git a/app/assets/javascripts/discourse/app/components/modal/group-add-members.js b/app/assets/javascripts/discourse/app/components/modal/group-add-members.js index 0bcd8d2bb94..6adb4bea1ae 100644 --- a/app/assets/javascripts/discourse/app/components/modal/group-add-members.js +++ b/app/assets/javascripts/discourse/app/components/modal/group-add-members.js @@ -5,6 +5,7 @@ import { tracked } from "@glimmer/tracking"; import { emailValid } from "discourse/lib/utilities"; import I18n from "I18n"; import { inject as service } from "@ember/service"; +import { extractError } from "discourse/lib/ajax-error"; export default class GroupAddMembers extends Component { @service currentUser; @@ -42,28 +43,33 @@ export default class GroupAddMembers extends Component { } @action - addMembers() { + async addMembers() { if (isEmpty(this.usernamesAndEmails)) { return; } + this.loading = true; - const promise = this.setOwner - ? this.args.model.addOwners(this.usernames, true, this.notifyUsers) - : this.args.model.addMembers( + + try { + if (this.setOwner) { + await this.args.model.addOwners(this.usernames, true, this.notifyUsers); + } else { + await this.args.model.addMembers( this.usernames, true, this.notifyUsers, this.emails ); + } - promise - .then(() => { - this.router.transitionTo("group.members", this.args.model.name, { - queryParams: { ...(this.usernames && { filter: this.usernames }) }, - }); - this.args.closeModal(); - }) - .catch((e) => (this.flash = e.jqXHR.responseJSON.errors.join(". "))) - .finally(() => (this.loading = false)); + this.router.transitionTo("group.members", this.args.model.name, { + queryParams: { ...(this.usernames && { filter: this.usernames }) }, + }); + this.args.closeModal(); + } catch (e) { + this.flash = extractError(e); + } finally { + this.loading = false; + } } } diff --git a/app/assets/javascripts/discourse/app/routes/associate-account.js b/app/assets/javascripts/discourse/app/routes/associate-account.js index 8cd543fe5ea..5e899cbae2d 100644 --- a/app/assets/javascripts/discourse/app/routes/associate-account.js +++ b/app/assets/javascripts/discourse/app/routes/associate-account.js @@ -35,8 +35,8 @@ export default DiscourseRoute.extend({ `/associate/${encodeURIComponent(params.token)}.json` ); showModal("associate-account-confirm", { model }); - } catch { - popupAjaxError; + } catch (e) { + popupAjaxError(e); } }, }); diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.js b/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.js index f615a4eb5ba..b5beb6815bd 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.js @@ -9,6 +9,7 @@ import { inject as service } from "@ember/service"; import { isBlank, isPresent } from "@ember/utils"; import { htmlSafe } from "@ember/template"; import { tracked } from "@glimmer/tracking"; +import { extractError } from "discourse/lib/ajax-error"; const DEFAULT_HINT = htmlSafe( I18n.t("chat.create_channel.choose_category.default_hint", { @@ -108,17 +109,16 @@ export default class ChatModalCreateChannel extends Component { } } - #createChannel(data) { - return this.chatApi - .createChannel(data) - .then((channel) => { - this.args.closeModal(); - this.chatChannelsManager.follow(channel); - this.router.transitionTo("chat.channel", ...channel.routeModels); - }) - .catch((e) => { - this.flash = e.jqXHR.responseJSON.errors[0]; - }); + async #createChannel(data) { + try { + const channel = await this.chatApi.createChannel(data); + + this.args.closeModal(); + this.chatChannelsManager.follow(channel); + this.router.transitionTo("chat.channel", ...channel.routeModels); + } catch (e) { + this.flash = extractError(e); + } } #buildCategorySlug(category) { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/modal/edit-channel-description.js b/plugins/chat/assets/javascripts/discourse/components/chat/modal/edit-channel-description.js index 2336c6cc040..cae3369a75c 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/modal/edit-channel-description.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/modal/edit-channel-description.js @@ -2,6 +2,7 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; import { tracked } from "@glimmer/tracking"; import { inject as service } from "@ember/service"; +import { extractError } from "discourse/lib/ajax-error"; const DESCRIPTION_MAX_LENGTH = 280; @@ -27,18 +28,16 @@ export default class ChatModalEditChannelDescription extends Component { } @action - onSaveChatChannelDescription() { - return this.chatApi - .updateChannel(this.channel.id, { description: this.editedDescription }) - .then((result) => { - this.channel.description = result.channel.description; - this.args.closeModal(); - }) - .catch((event) => { - if (event.jqXHR?.responseJSON?.errors) { - this.flash = event.jqXHR.responseJSON.errors.join("\n"); - } + async onSaveChatChannelDescription() { + try { + const result = await this.chatApi.updateChannel(this.channel.id, { + description: this.editedDescription, }); + this.channel.description = result.channel.description; + this.args.closeModal(); + } catch (error) { + this.flash = extractError(error); + } } @action