From fbb1f9e634dcaa6f66d676f94b5050d2cf308dcc Mon Sep 17 00:00:00 2001 From: BitCompass <1698200+bitcompass@users.noreply.github.com> Date: Thu, 14 May 2026 06:43:57 -0400 Subject: [PATCH] :bug: Fix plugin API error message for nested malli validation paths (#9486) When a plugin call fails malli validation, the frontend renders one "plugins.validation.message" line per error via `app.plugins.utils/error-messages`, which reduces the explain via `csm/interpret-schema-problem` and then destructures each entry as `[field {:keys [message]}]` for translation. That works only when the underlying malli error path has a single element. `interpret-schema-problem` calls `(assoc-in acc field ...)` where `field` can be a multi-element vector (e.g. `[:sets 0 :name]`). For single-element paths the resulting map is flat (`{:group {:message "..."}}`); for multi-element paths it is nested (`{:sets {0 {:name {:message "..."}}}}`). The destructure assumes the flat shape, so for a nested error the consumer reads: field -> :sets message -> nil (the nested entry has no :message at the top level) and the produced i18n line resolves to `Field sets is invalid: ` -- or, when several errors are merged together at the same outer key, to the user-facing `Field message is invalid` that the bug report calls out, because `:message` then becomes the field name of the deepest nested entry. The original consumer carried a `#_(mapcat (comp seq val))` FIXME that hinted at the missing flattening but did not implement one, because the data shape produced by `interpret-schema-problem` is not uniform. Fix --- Add a private `flatten-error-map` helper inside `app.plugins.utils` that walks the error map produced by `interpret-schema-problem` and yields `[path message]` pairs where `path` is the dot-joined field path. Keywords use `(name k)`, strings pass through, anything else (such as numeric indices from vector positions in the malli path) is coerced via `str`. The recursion descends until it hits a leaf that carries `:message`, which matches what `interpret-schema-problem` produces in every branch. The producer side (`csm/interpret-schema-problem` in `common/src/app/common/schema/messages.cljc`) is left alone: it already has another consumer (`collect-schema-errors` + the form-validators pipeline) that depends on the keyed-by-field-path shape, so normalising it at the source would require auditing every validator. Flattening at the plugin consumer is the narrowest fix. The FIXME comment is removed because the new helper supersedes it. Tests ----- `frontend-tests.plugins.utils-test` (new file, registered in `runner.cljs`) covers: - flat single-segment paths (`{:group {:message "..."}}`) - nested multi-segment paths (`{:sets {0 {:name {:message "..."}}}}`) -- the case from #9417 - mixed single- and multi-segment paths at the same explain - mixed key types (keyword / string / numeric index) - empty explain (no validation errors) Closes #9417 Signed-off-by: bitcompass Co-authored-by: Andrey Antukh --- frontend/src/app/plugins/utils.cljs | 32 ++++++++++- .../frontend_tests/plugins/utils_test.cljs | 56 +++++++++++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 frontend/test/frontend_tests/plugins/utils_test.cljs diff --git a/frontend/src/app/plugins/utils.cljs b/frontend/src/app/plugins/utils.cljs index 5765b6cc6b..863ad37e2f 100644 --- a/frontend/src/app/plugins/utils.cljs +++ b/frontend/src/app/plugins/utils.cljs @@ -276,13 +276,39 @@ (let [s (set values)] (if (= (count s) 1) (first s) "mixed"))) +(defn- flatten-error-map + "Walk an error map produced by `csm/interpret-schema-problem` and yield + `[path message]` pairs, where `path` is the dot-joined field path + (e.g. `:group` -> \"group\", `[:sets 0 :name]` -> \"sets.0.name\"). + + `interpret-schema-problem` calls `(assoc-in acc field {:message …})`, so + when the malli error path has more than one element the resulting map is + nested (e.g. `{:sets {0 {:name {:message \"…\"}}}}`); when the path has + a single element it is flat (`{:group {:message \"…\"}}`). The plugin + error-message renderer needs both cases reduced to per-leaf + `[path message]` pairs so it can produce one `plugins.validation.message` + string per actual validation problem." + ([m] (flatten-error-map [] m)) + ([prefix m] + (mapcat + (fn [[k v]] + (let [segment (cond + (keyword? k) (name k) + (string? k) k + :else (str k)) + path (conj prefix segment)] + (if (and (map? v) (not (contains? v :message))) + (flatten-error-map path v) + [[(str/join "." path) (:message v)]]))) + m))) + (defn error-messages [explain] (->> (:errors explain) (reduce csm/interpret-schema-problem {}) - #_(mapcat (comp seq val)) ;; FIXME: why is this for? it breaks the message - (map (fn [[field {:keys [message]}]] - (tr "plugins.validation.message" (name field) message))) + (flatten-error-map) + (map (fn [[field message]] + (tr "plugins.validation.message" field message))) (str/join ". "))) (defn handle-error diff --git a/frontend/test/frontend_tests/plugins/utils_test.cljs b/frontend/test/frontend_tests/plugins/utils_test.cljs new file mode 100644 index 0000000000..8fbf4e4f40 --- /dev/null +++ b/frontend/test/frontend_tests/plugins/utils_test.cljs @@ -0,0 +1,56 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.plugins.utils-test + (:require + [app.plugins.utils :as plugins.utils] + [cljs.test :as t :include-macros true])) + +;; Access the private flattener for direct testing. +(def ^:private flatten-error-map @#'plugins.utils/flatten-error-map) + +(t/deftest test-flatten-error-map-flat-input + ;; Regression test for issue #9417. + ;; + ;; When a malli error path has a single element, `interpret-schema-problem` + ;; produces a flat map. The flattener must pass that through unchanged. + (let [result (flatten-error-map {:group {:message "must be string"}})] + (t/is (= [["group" "must be string"]] result)))) + +(t/deftest test-flatten-error-map-nested-input + ;; Regression test for issue #9417. + ;; + ;; When a malli error path has multiple elements, `interpret-schema-problem` + ;; produces a nested map via `(assoc-in acc path …)`. The old plugin + ;; consumer destructured assuming a flat shape, so the nested case rendered + ;; the message text as the field name (`Field message is invalid`) instead + ;; of the real validation reason. The flattener must descend until it finds + ;; a leaf carrying a `:message`. + (let [explain {:sets {0 {:name {:message "must not be empty"}}}} + result (set (flatten-error-map explain))] + (t/is (= #{["sets.0.name" "must not be empty"]} result)))) + +(t/deftest test-flatten-error-map-multiple-fields + ;; Multiple validation problems on the same explain produce multiple + ;; entries, none of which clobber each other. + (let [explain {:group {:message "must be string"} + :sets {0 {:message "set not found"}}} + result (set (flatten-error-map explain))] + (t/is (= #{["group" "must be string"] + ["sets.0" "set not found"]} result)))) + +(t/deftest test-flatten-error-map-mixed-key-types + ;; Numeric indices (from vector positions in the malli path) must render + ;; cleanly; string keys must also be accepted alongside keywords. + (let [explain {:items {2 {"label" {:message "invalid label"}}}} + [[path message]] (flatten-error-map explain)] + (t/is (= "items.2.label" path)) + (t/is (= "invalid label" message)))) + +(t/deftest test-flatten-error-map-empty + ;; No validation errors -> no output (callers join with ". " and would + ;; otherwise emit an empty string, which is fine). + (t/is (empty? (flatten-error-map {})))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 21816ba1a3..8935cd286a 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -24,6 +24,7 @@ [frontend-tests.plugins.context-shapes-test] [frontend-tests.plugins.parser-test] [frontend-tests.plugins.tokens-test] + [frontend-tests.plugins.utils-test] [frontend-tests.svg-fills-test] [frontend-tests.tokens.import-export-test] [frontend-tests.tokens.logic.token-actions-test] @@ -72,6 +73,7 @@ 'frontend-tests.plugins.context-shapes-test 'frontend-tests.plugins.parser-test 'frontend-tests.plugins.tokens-test + 'frontend-tests.plugins.utils-test 'frontend-tests.svg-fills-test 'frontend-tests.tokens.import-export-test 'frontend-tests.tokens.logic.token-actions-test