From 6fe91bbbbb9a9bd076db6371c1f814f71b0ee711 Mon Sep 17 00:00:00 2001 From: Jeff Wong Date: Thu, 5 Mar 2020 15:29:51 -1000 Subject: [PATCH] Revert "FEATURE: prevent accidental canceling when drafting penalties (#9105)" (#9122) This reverts commit 243284f998e213034f9f0c6b228f97f7c9834fdd. There are some issues in how the JS tests interact that I will need to figure out here before this can be merged. --- .../admin/mixins/penalty-controller.js.es6 | 24 +----------------- .../discourse/components/d-modal.js.es6 | 17 ++++++------- .../discourse/routes/application.js.es6 | 15 ++--------- config/locales/client.en.yml | 1 - .../acceptance/admin-suspend-user-test.js.es6 | 25 ------------------- test/javascripts/acceptance/modal-test.js.es6 | 10 ++++---- 6 files changed, 15 insertions(+), 77 deletions(-) diff --git a/app/assets/javascripts/admin/mixins/penalty-controller.js.es6 b/app/assets/javascripts/admin/mixins/penalty-controller.js.es6 index f9237106492..cb07e9d2ca3 100644 --- a/app/assets/javascripts/admin/mixins/penalty-controller.js.es6 +++ b/app/assets/javascripts/admin/mixins/penalty-controller.js.es6 @@ -1,7 +1,6 @@ import ModalFunctionality from "discourse/mixins/modal-functionality"; import { popupAjaxError } from "discourse/lib/ajax-error"; import Mixin from "@ember/object/mixin"; -import { next } from "@ember/runloop"; import { Promise } from "rsvp"; export default Mixin.create(ModalFunctionality, { @@ -12,7 +11,6 @@ export default Mixin.create(ModalFunctionality, { user: null, postId: null, successCallback: null, - confirmClose: false, resetModal() { this.setProperties({ @@ -23,30 +21,10 @@ export default Mixin.create(ModalFunctionality, { postEdit: null, postAction: "delete", before: null, - successCallback: null, - confirmClose: false + successCallback: null }); }, - beforeClose() { - // prompt a confirmation if we have unsaved content - if ( - (this.reason && this.reason.length > 1) || - (this.message && this.message.length > 1 && !this.confirmClose) - ) { - this.send("hideModal"); - bootbox.confirm(I18n.t("admin.user.confirm_cancel_penalty"), result => { - if (result) { - this.set("confirmClose", true); - this.send("closeModal"); - } else { - next(() => this.send("reopenModal")); - } - }); - return false; - } - }, - penalize(cb) { let before = this.before; let promise = before ? before() : Promise.resolve(); diff --git a/app/assets/javascripts/discourse/components/d-modal.js.es6 b/app/assets/javascripts/discourse/components/d-modal.js.es6 index eeae7edc3f3..300a3e585cb 100644 --- a/app/assets/javascripts/discourse/components/d-modal.js.es6 +++ b/app/assets/javascripts/discourse/components/d-modal.js.es6 @@ -31,16 +31,13 @@ export default Component.extend({ @on("didInsertElement") setUp() { - $("html").on("keyup.discourse-modal", e => { - //only respond to events when the modal is visible - if ($("#discourse-modal:visible").length > 0) { - if (e.which === 27 && this.dismissable) { - next(() => $(".modal-header button.modal-close").click()); - } + $("html").on("keydown.discourse-modal", e => { + if (e.which === 27 && this.dismissable) { + next(() => $(".modal-header button.modal-close").click()); + } - if (e.which === 13 && this.triggerClickOnEnter(e)) { - next(() => $(".modal-footer .btn-primary").click()); - } + if (e.which === 13 && this.triggerClickOnEnter(e)) { + next(() => $(".modal-footer .btn-primary").click()); } }); @@ -49,7 +46,7 @@ export default Component.extend({ @on("willDestroyElement") cleanUp() { - $("html").off("keyup.discourse-modal"); + $("html").off("keydown.discourse-modal"); this.appEvents.off("modal:body-shown", this, "_modalBodyShown"); }, diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6 index 8dca333360a..e877d8ceff2 100644 --- a/app/assets/javascripts/discourse/routes/application.js.es6 +++ b/app/assets/javascripts/discourse/routes/application.js.es6 @@ -157,23 +157,12 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { // Close the current modal, and destroy its state. closeModal() { + this.render("hide-modal", { into: "modal", outlet: "modalBody" }); + const route = getOwner(this).lookup("route:application"); let modalController = route.controllerFor("modal"); const controllerName = modalController.get("name"); - if (controllerName) { - const controller = getOwner(this).lookup( - `controller:${controllerName}` - ); - if (controller && controller.beforeClose) { - if (false === controller.beforeClose()) { - return; - } - } - } - - this.render("hide-modal", { into: "modal", outlet: "modalBody" }); - if (controllerName) { const controller = getOwner(this).lookup( `controller:${controllerName}` diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c7705ab9485..127c964d041 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4291,7 +4291,6 @@ en: threshold_reached: "Received too many bounces from that email." trust_level_change_failed: "There was a problem changing the user's trust level." suspend_modal_title: "Suspend User" - confirm_cancel_penalty: "Are you sure you want to discard the penalty?" trust_level_2_users: "Trust Level 2 Users" trust_level_3_requirements: "Trust Level 3 Requirements" trust_level_locked_tip: "trust level is locked, system will not promote or demote user" diff --git a/test/javascripts/acceptance/admin-suspend-user-test.js.es6 b/test/javascripts/acceptance/admin-suspend-user-test.js.es6 index 936f7080bb8..d9983848caf 100644 --- a/test/javascripts/acceptance/admin-suspend-user-test.js.es6 +++ b/test/javascripts/acceptance/admin-suspend-user-test.js.es6 @@ -34,31 +34,6 @@ QUnit.test("suspend a user - cancel", async assert => { assert.equal(find(".suspend-user-modal:visible").length, 0); }); -QUnit.test("suspend a user - cancel with input", async assert => { - await visit("/admin/users/1234/regular"); - await click(".suspend-user"); - - assert.equal(find(".suspend-user-modal:visible").length, 1); - - await fillIn(".suspend-reason", "for breaking the rules"); - await fillIn(".suspend-message", "this is an email reason why"); - - await click(".d-modal-cancel"); - - assert.equal(find(".bootbox.modal:visible").length, 1); - - await click(".modal-footer .btn-default"); - assert.equal(find(".suspend-user-modal:visible").length, 1); - assert.equal( - find(".suspend-message")[0].value, - "this is an email reason why" - ); - - await click(".d-modal-cancel"); - await click(".modal-footer .btn-primary"); - assert.equal(find(".suspend-user-modal:visible").length, 0); -}); - QUnit.test("suspend, then unsuspend a user", async assert => { const suspendUntilCombobox = selectKit(".suspend-until .combobox"); diff --git a/test/javascripts/acceptance/modal-test.js.es6 b/test/javascripts/acceptance/modal-test.js.es6 index 4e43be89bba..13725f4d6cc 100644 --- a/test/javascripts/acceptance/modal-test.js.es6 +++ b/test/javascripts/acceptance/modal-test.js.es6 @@ -28,7 +28,7 @@ QUnit.test("modal", async function(assert) { await click(".login-button"); assert.ok(find(".d-modal:visible").length === 1, "modal should reappear"); - await keyEvent("#main-outlet", "keyup", 27); + await keyEvent("#main-outlet", "keydown", 27); assert.ok( find(".d-modal:visible").length === 0, "ESC should close the modal" @@ -47,7 +47,7 @@ QUnit.test("modal", async function(assert) { find(".d-modal:visible").length === 1, "modal should not disappear when you click outside" ); - await keyEvent("#main-outlet", "keyup", 27); + await keyEvent("#main-outlet", "keydown", 27); assert.ok( find(".d-modal:visible").length === 1, "ESC should not close the modal" @@ -61,7 +61,7 @@ QUnit.test("modal-keyboard-events", async function(assert) { await click(".toggle-admin-menu"); await click(".topic-admin-status-update button"); - await keyEvent(".d-modal", "keyup", 13); + await keyEvent(".d-modal", "keydown", 13); assert.ok( find("#modal-alert:visible").length === 1, @@ -72,7 +72,7 @@ QUnit.test("modal-keyboard-events", async function(assert) { "hitting Enter does not dismiss modal due to alert error" ); - await keyEvent("#main-outlet", "keyup", 27); + await keyEvent("#main-outlet", "keydown", 27); assert.ok( find(".d-modal:visible").length === 0, "ESC should close the modal" @@ -82,7 +82,7 @@ QUnit.test("modal-keyboard-events", async function(assert) { await click(".d-editor-button-bar .btn.link"); - await keyEvent(".d-modal", "keyup", 13); + await keyEvent(".d-modal", "keydown", 13); assert.ok( find(".d-modal:visible").length === 0, "modal should disappear on hitting Enter"