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