From e901403621f88741120487825a08ae763dd91368 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Mon, 21 Nov 2022 13:11:29 -0300 Subject: [PATCH] FEATURE: API to customize server side composer errors handling in the client side (#19107) This will be used by plugins to handle the client side of their custom post validations without having to overwrite the whole composer save action as it was done in other plugins. Co-authored-by: Penar Musaraj --- .../discourse/app/controllers/composer.js | 26 +++++++-- .../discourse/app/lib/plugin-api.js | 28 +++++++++- .../tests/acceptance/composer-test.js | 53 +++++++++++++++++++ docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md | 7 +++ lib/post_creator.rb | 4 +- lib/post_revisor.rb | 2 +- spec/lib/post_creator_spec.rb | 9 ++++ spec/lib/post_revisor_spec.rb | 11 ++++ 8 files changed, 132 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js index 9c0951ebd0e..f56dab12044 100644 --- a/app/assets/javascripts/discourse/app/controllers/composer.js +++ b/app/assets/javascripts/discourse/app/controllers/composer.js @@ -67,6 +67,7 @@ async function loadDraft(store, opts = {}) { } const _popupMenuOptionsCallbacks = []; +const _composerSaveErrorCallbacks = []; let _checkDraftPopup = !isTesting(); @@ -82,6 +83,14 @@ export function addPopupMenuOptionsCallback(callback) { _popupMenuOptionsCallbacks.push(callback); } +export function clearComposerSaveErrorCallback() { + _composerSaveErrorCallbacks.length = 0; +} + +export function addComposerSaveErrorCallback(callback) { + _composerSaveErrorCallbacks.push(callback); +} + export default Controller.extend({ topicController: controller("topic"), router: service(), @@ -1039,9 +1048,20 @@ export default Controller.extend({ .catch((error) => { composer.set("disableDrafts", false); if (error) { - this.appEvents.one("composer:will-open", () => - this.dialog.alert(error) - ); + this.appEvents.one("composer:will-open", () => { + if ( + _composerSaveErrorCallbacks.length === 0 || + !_composerSaveErrorCallbacks + .map((c) => { + return c.call(this, error); + }) + .some((i) => { + return i; + }) + ) { + this.dialog.alert(error); + } + }); } }); diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 13faa140b0a..b55155b5b87 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -55,7 +55,10 @@ import { addNavItem } from "discourse/models/nav-item"; import { addPluginDocumentTitleCounter } from "discourse/components/d-document"; import { addPluginOutletDecorator } from "discourse/components/plugin-connector"; import { addPluginReviewableParam } from "discourse/components/reviewable-item"; -import { addPopupMenuOptionsCallback } from "discourse/controllers/composer"; +import { + addComposerSaveErrorCallback, + addPopupMenuOptionsCallback, +} from "discourse/controllers/composer"; import { addPostClassesCallback } from "discourse/widgets/post"; import { addGroupPostSmallActionCode, @@ -109,7 +112,7 @@ import { registerModelTransformer } from "discourse/lib/model-transformers"; // based on Semantic Versioning 2.0.0. Please update the changelog at // docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md whenever you change the version // using the format described at https://keepachangelog.com/en/1.0.0/. -const PLUGIN_API_VERSION = "1.4.0"; +const PLUGIN_API_VERSION = "1.5.0"; // This helper prevents us from applying the same `modifyClass` over and over in test mode. function canModify(klass, type, resolverName, changes) { @@ -1246,6 +1249,27 @@ class PluginApi { Composer.reopen({ beforeSave: method }); } + /** + * Registers a callback function to handle the composer save errors. + * This allows you to implement custom logic that will happen before + * the raw error is presented to the user. + * The passed function is expected to return true if the error was handled, + * false otherwise. + * + * Example: + * + * api.addComposerSaveErrorCallback((error) => { + * if (error == "my_error") { + * //handle error + * return true; + * } + * return false; + * }) + */ + addComposerSaveErrorCallback(callback) { + addComposerSaveErrorCallback(callback); + } + /** * Adds a field to topic edit serializer * diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js index e2e0593cc39..b250106bd91 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js @@ -1106,6 +1106,59 @@ acceptance("Composer - Customizations", function (needs) { }); }); +acceptance("Composer - Error Extensibility", function (needs) { + needs.user(); + needs.settings({ + general_category_id: 1, + default_composer_category: 1, + }); + + needs.hooks.beforeEach(() => { + withPluginApi("1.5.0", (api) => { + api.addComposerSaveErrorCallback((error) => { + if (error.match(/PLUGIN_XYZ ERROR/)) { + // handle error + return true; + } + return false; + }); + }); + }); + + test("Create a topic with server side errors handled by a plugin", async function (assert) { + pretender.post("/posts", function () { + return response(422, { errors: ["PLUGIN_XYZ ERROR"] }); + }); + + await visit("/"); + await click("#create-topic"); + await fillIn("#reply-title", "this title triggers an error"); + await fillIn(".d-editor-input", "this is the *content* of a post"); + await click("#reply-control button.create"); + assert.notOk(exists(".dialog-body"), "it does not pop up an error message"); + }); + + test("Create a topic with server side errors not handled by a plugin", async function (assert) { + pretender.post("/posts", function () { + return response(422, { errors: ["PLUGIN_ABC ERROR"] }); + }); + + await visit("/"); + await click("#create-topic"); + await fillIn("#reply-title", "this title triggers an error"); + await fillIn(".d-editor-input", "this is the *content* of a post"); + await click("#reply-control button.create"); + assert.ok(exists(".dialog-body"), "it pops up an error message"); + assert.ok( + query(".dialog-body").innerText.match(/PLUGIN_ABC ERROR/), + "it contains the server side error text" + ); + await click(".dialog-footer .btn-primary"); + assert.ok(!exists(".dialog-body"), "it dismisses the error"); + assert.ok(exists(".d-editor-input"), "the composer input is visible"); + }); +}); + acceptance("Composer - Focus Open and Closed", function (needs) { needs.user(); needs.settings({ allow_uncategorized_topics: true }); diff --git a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md index e4f56275683..bc4a6ede374 100644 --- a/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md +++ b/docs/CHANGELOG-JAVASCRIPT-PLUGIN-API.md @@ -7,6 +7,13 @@ in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.5.0] - 2022-11-21 + +### Added + +- Adds `addComposerSaveErrorCallback`, which allows users to register custom error handling + for server-side errors when submitting on the composer. + ## [1.4.0] - 2022-09-27 ### Added diff --git a/lib/post_creator.rb b/lib/post_creator.rb index f58af75a00b..6aa6e478515 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -6,7 +6,7 @@ class PostCreator include HasErrors - attr_reader :opts + attr_reader :opts, :post # Acceptable options: # @@ -161,7 +161,7 @@ class PostCreator return false end - DiscourseEvent.trigger :before_create_post, @post + DiscourseEvent.trigger :before_create_post, @post, @opts DiscourseEvent.trigger :validate_post, @post post_validator = PostValidator.new(skip_topic: true) diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 7a79689566d..7fc1ff8144b 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -621,7 +621,7 @@ class PostRevisor end def plugin_callbacks - DiscourseEvent.trigger(:before_edit_post, @post) + DiscourseEvent.trigger(:before_edit_post, @post, @fields) DiscourseEvent.trigger(:validate_post, @post) end diff --git a/spec/lib/post_creator_spec.rb b/spec/lib/post_creator_spec.rb index d21db96998f..b125caac8a0 100644 --- a/spec/lib/post_creator_spec.rb +++ b/spec/lib/post_creator_spec.rb @@ -119,6 +119,15 @@ RSpec.describe PostCreator do ) end + it "before_create_post event signature contains both post and opts" do + events = DiscourseEvent.track_events { creator.create } + + expect(events).to include( + event_name: :before_create_post, + params: [creator.post, creator.opts] + ) + end + it "does not notify on system messages" do messages = MessageBus.track_publish do p = PostCreator.create(admin, basic_topic_params.merge(post_type: Post.types[:moderator_action])) diff --git a/spec/lib/post_revisor_spec.rb b/spec/lib/post_revisor_spec.rb index 4d32af66fc1..2810a21e89b 100644 --- a/spec/lib/post_revisor_spec.rb +++ b/spec/lib/post_revisor_spec.rb @@ -691,6 +691,17 @@ RSpec.describe PostRevisor do expect(post.revisions.size).to eq(1) end end + + context 'when editing the before_edit_post event signature' do + it 'contains post and params' do + params = { raw: 'body (edited)' } + events = DiscourseEvent.track_events { subject.revise!(user, params) } + expect(events).to include( + event_name: :before_edit_post, + params: [post, params] + ) + end + end end describe "topic excerpt" do