From b8434f3b6ac80ab78502ec441415c8b2ee4e5cf7 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 30 Jul 2024 10:24:28 +0200 Subject: [PATCH] Fix displaying element validation errors. With the change in 30ef25f5b we return a 422 from the element update action. This needs to be handled via the `ajax:complete` event, because a 422 is considered an error response and not a succes. --- .../components/element_editor.js | 15 +++---- .../admin/edit_elements_feature_spec.rb | 31 +++++++++++++++ .../components/element_editor.spec.js | 39 ++++++++++++++----- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/app/javascript/alchemy_admin/components/element_editor.js b/app/javascript/alchemy_admin/components/element_editor.js index 6bd2240dc5..2181527471 100644 --- a/app/javascript/alchemy_admin/components/element_editor.js +++ b/app/javascript/alchemy_admin/components/element_editor.js @@ -19,7 +19,7 @@ export class ElementEditor extends HTMLElement { // Triggered by child elements this.addEventListener("alchemy:element-update-title", this) // We use of @rails/ujs for Rails remote forms - this.addEventListener("ajax:success", this) + this.addEventListener("ajax:complete", this) // Dirty observer this.addEventListener("change", this) @@ -57,11 +57,11 @@ export class ElementEditor extends HTMLElement { this.onClickElement() } break - case "ajax:success": + case "ajax:complete": if (event.target === this.body) { - const responseJSON = event.detail[0] + const xhr = event.detail[0] event.stopPropagation() - this.onSaveElement(responseJSON) + this.onSaveElement(xhr) } break case "alchemy:element-update-title": @@ -116,9 +116,10 @@ export class ElementEditor extends HTMLElement { * Sets the element to saved state * Updates title * Shows error messages if ingredient validations fail - * @argument {JSON} data + * @argument {XMLHttpRequest} xhr */ - onSaveElement(data) { + onSaveElement(xhr) { + const data = JSON.parse(xhr.responseText) // JS event bubbling will also update the parents element quote. this.setClean() // Reset errors that might be visible from last save attempt @@ -128,7 +129,7 @@ export class ElementEditor extends HTMLElement { .querySelectorAll(".ingredient-editor") .forEach((el) => el.classList.remove("validation_failed")) // If validation failed - if (data.errors) { + if (xhr.status === 422) { const warning = data.warning // Create error messages data.errors.forEach((message) => { diff --git a/spec/features/admin/edit_elements_feature_spec.rb b/spec/features/admin/edit_elements_feature_spec.rb index 5505115a15..7a7d00b755 100644 --- a/spec/features/admin/edit_elements_feature_spec.rb +++ b/spec/features/admin/edit_elements_feature_spec.rb @@ -149,6 +149,37 @@ end end + describe "Updating an element", :js do + context "with valid data" do + let!(:element) { create(:alchemy_element, page_version: a_page.draft_version) } + + scenario "shows success notice" do + visit alchemy.admin_elements_path(page_version_id: element.page_version_id) + expect(page).to have_button("Save") + click_button("Save") + within "#flash_notices" do + expect(page).to have_content(/Saved element/) + end + end + end + + context "with invalid data" do + let!(:element) { create(:alchemy_element, name: "all_you_can_eat", page_version: a_page.draft_version) } + + scenario "shows error notice" do + visit alchemy.admin_elements_path(page_version_id: element.page_version_id) + expect(page).to have_button("Save") + click_button("Save") + within "#flash_notices" do + expect(page).to have_content(/Validation failed/) + end + within ".error-messages" do + expect(page).to have_content(/Please enter a headline/) + end + end + end + end + describe "With an element that has ingredient groups" do let(:element) do create( diff --git a/spec/javascript/alchemy_admin/components/element_editor.spec.js b/spec/javascript/alchemy_admin/components/element_editor.spec.js index b413678d99..1171df40d2 100644 --- a/spec/javascript/alchemy_admin/components/element_editor.spec.js +++ b/spec/javascript/alchemy_admin/components/element_editor.spec.js @@ -251,12 +251,17 @@ describe("alchemy-element-editor", () => { }) }) - describe("on ajax:success", () => { + describe("on ajax:complete", () => { describe("if event was triggered on this element", () => { it("sets element to saved state", () => { - const event = new CustomEvent("ajax:success", { + const event = new CustomEvent("ajax:complete", { bubbles: true, - detail: [{ ingredientAnchors: [] }] + detail: [ + { + status: 200, + responseText: JSON.stringify({ ingredientAnchors: [] }) + } + ] }) editor.dirty = true editor.body.dispatchEvent(event) @@ -287,9 +292,17 @@ describe("alchemy-element-editor", () => { `) - const event = new CustomEvent("ajax:success", { + const event = new CustomEvent("ajax:complete", { bubbles: true, - detail: [{ previewText: "Child Element", ingredientAnchors: [] }] + detail: [ + { + status: 200, + responseText: JSON.stringify({ + previewText: "Child Element", + ingredientAnchors: [] + }) + } + ] }) const childElement = editor.querySelector("#element_789") childElement.dirty = true @@ -410,8 +423,11 @@ describe("alchemy-element-editor", () => { `) const data = { - notice: "Element saved", - ingredientAnchors: [{ ingredientId: 55, active: true }] + status: 200, + responseText: JSON.stringify({ + notice: "Element saved", + ingredientAnchors: [{ ingredientId: 55, active: true }] + }) } editor.dirty = true editor.onSaveElement(data) @@ -463,9 +479,12 @@ describe("alchemy-element-editor", () => { `) const data = { - warning: "Something is not right", - errors: ["Please enter a value"], - ingredientsWithErrors: [666] + status: 422, + responseText: JSON.stringify({ + warning: "Something is not right", + errors: ["Please enter a value"], + ingredientsWithErrors: [666] + }) } editor.onSaveElement(data) })