Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Humanize only includes one error #499

Closed
hansbugge opened this issue Aug 3, 2021 · 3 comments
Closed

Humanize only includes one error #499

hansbugge opened this issue Aug 3, 2021 · 3 comments
Labels
bug Something isn't working help wanted Help most welcome

Comments

@hansbugge
Copy link
Contributor

hansbugge commented Aug 3, 2021

In some cases calling malli.error/humanize on multiple errors returns only one error message, even though it could return multiple. The behaviour changed after PR #333 was merged, but not to what I would expect.

;;; For base values there are no changes before and after #333
(-> (m/explain [:and
                [:fn {:error/message "error 1"} (constantly false)]
                [:fn {:error/message "error 2"} (constantly false)]]
               :some-value)
    me/humanize)
;;; Before #333 merged (0eac0b70d):
;; => ["error 1"]

;;; After #333 merged (cfc998543):
;; => ["error 1"]

;;; Expected result: ["error 1" "error 2"]


;;; For maps there used to be multiple error messages as expected (provided they were strings),
(-> (m/explain [:and
                [:fn {:error/message "error 1"} (constantly false)]
                [:fn {:error/message "error 2"} (constantly false)]]
               {:a :map})
    me/humanize)
;;; Before #333 merged (0eac0b70d):
;; => #:malli{:error ["error 1" "error 2"]}

;;; After #333 merged (cfc998543):
;; => #:malli{:error ["error 1"]}

;;; Expected result: #:malli{:error ["error 1" "error 2"]}


;;; Note that the behaviour used to be different when the humanize wrap function
;;; returned something else than a string, due to a call to string? in malli.error/-just-error?
(-> (m/explain [:and
                [:fn {:error/message "error 1"} (constantly false)]
                [:fn {:error/message "error 2"} (constantly false)]]
               {:a :map})
    (me/humanize {:wrap #(select-keys % [:message])}))
;;; Before #333 merged (0eac0b70d):
;; => #:malli{:error [{:message "error 2"}]}

;;; After #333 merged (cfc998543):
;; => #:malli{:error [{:message "error 1"}]}

;;; Expected result: #malli{:error [{:message "error 1"} {:message "error 2"}]}

I wanted to use humanize to provide (structured) form validation output, but I need to show more than one error at a time when possible, so I think I have to make a workaround.

@ikitommi ikitommi added the bug Something isn't working label Aug 3, 2021
@ikitommi
Copy link
Member

ikitommi commented Aug 3, 2021

Doesn't look right.

@ikitommi ikitommi added the help wanted Help most welcome label Aug 3, 2021
@ikitommi
Copy link
Member

ikitommi commented Aug 6, 2021

Does this look ok?

(deftest robust-humanize-form
  (let [f (fn [s] [:fn {:error/message s} (constantly false)])
        => ::irrelevant]
    (are [schema value _ expected]
      (is (= expected (-> (m/explain schema value) (me/humanize))))

      ;; simple cases
      :any :any => nil
      [:and :any :any] :any => nil
      [:and (f "1") :any] :any => ["1"]
      [:and (f "1") (f "1") :any] :any => ["1" "1"]
      [:and (f "1") (f "2")] {:a :map} => ["1" "2"]

      ;; accumulate into maps if error shape is already a map
      [:map [:x [:and [:map [:y :any]] seq?]]] 123 => ["invalid type"]
      [:map [:x [:and [:map [:y :any]] seq?]]] {} => {:x ["missing required key"]}
      [:map [:x [:and [:map [:y :any]] seq?]]] {:x 123} => {:x ["invalid type" "should be a seq"]}
      [:map [:x [:and [:map [:y :any]] seq? (f "kosh")]]] {:x {}} => {:x {:y ["missing required key"]
                                                                          :malli/error ["should be a seq" "kosh"]}}
      [:map [:x [:and [:map [:y :any]] seq?]]] {:x {:y 123}} => {:x ["should be a seq"]}

      ;; don't derive error form from value in case of top-level error
      [:map [:x [:and seq? [:map [:y :any]]]]] 123 => ["invalid type"]
      [:map [:x [:and seq? [:map [:y :any]]]]] {} => {:x ["missing required key"]}
      [:map [:x [:and seq? [:map [:y :any]]]]] {:x 123} => {:x ["should be a seq" "invalid type"]}
      [:map [:x [:and seq? [:map [:y :any]]]]] {:x {}} => {:x ["should be a seq"]}

      ;; tuple sizes
      [:map [:x [:tuple :int :int :int]]] {} => {:x ["missing required key"]}
      [:map [:x [:tuple :int :int :int]]] {:x []} => {:x ["invalid tuple size 0, expected 3"]}
      [:map [:x [:tuple :int :int :int]]] {:x [1 "2" 3]} => {:x [nil ["should be an integer"]]}
      [:map [:x [:tuple :int :int :int]]] {:x [1 "2" "3"]} => {:x [nil ["should be an integer"] ["should be an integer"]]}
      [:map [:x [:tuple :int [:and :int (f "fails")] :int]]] {:x [1 "2" "3"]} => {:x [nil ["should be an integer" "fails"] ["should be an integer"]]}
      [:map [:x [:tuple :int :int :int]]] {:x [1 2 3]} => nil

      ;; sequences
      [:and [:sequential :int] (f "1") (f "2")] [1 "2"] => [nil ["should be an integer"]]
      [:and [:sequential :int] (f "1") (f "2")] [1 2] => ["1" "2"])))

@ikitommi
Copy link
Member

ikitommi commented Aug 6, 2021

Fixed in master

@ikitommi ikitommi closed this as completed Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Help most welcome
Projects
None yet
Development

No branches or pull requests

2 participants