update `defendpoint` to specifically catch ApiFieldValidationException and...

update `defendpoint` to specifically catch ApiFieldValidationException and format the response to include specific field error notes that a client can more easily use.

also updating all of our defannotation calls to use `checkp` and ApiFieldValidationException so that we get our expected output.  updated unit tests accordingly.

src/metabase/api/common.clj

@@ -322,7 +322,7 @@
   "Param may not be `nil`."
   [symb value]
   (when-not value
-    (throw (ApiException. (int 400) (format "'%s' is a required param." symb))))
+    (throw (ApiFieldValidationException. (format "%s" symb) "field is a required param.")))
   value)
 
 (defannotation Date
@@ -331,7 +331,7 @@
   [symb value :nillable]
   (try (u/parse-iso8601 value)
           (catch Throwable _
-            (throw (ApiException. (int 400) (format "'%s' is not a valid date." symb))))))
+            (throw (ApiFieldValidationException. (format "%s" symb) (format "'%s' is not a valid date." value))))))
 
 (defannotation String->Integer
   "Param is converted from a string to an integer."
@@ -387,8 +387,7 @@
 (defannotation ComplexPassword
   "Param must be a complex password (*what does this mean?*)"
   [symb value]
-  (check (password/is-complex? value)
-    [400 (format "Invalid value for '%s': Insufficient password strength" symb)])
+  (checkp (password/is-complex? value) symb "Insufficient password strength")
   value)
 
 (defannotation FilterOptionAllOrMine

src/metabase/api/common/internal.clj

@@ -4,7 +4,8 @@
             [medley.core :as m]
             [swiss.arrows :refer :all]
             [metabase.util :as u])
-  (:import com.metabase.corvus.api.ApiException))
+  (:import com.metabase.corvus.api.ApiException
+           com.metabase.corvus.api.ApiFieldValidationException))
 
 ;;; # DEFENDPOINT HELPER FUNCTIONS + MACROS
 
@@ -215,6 +216,9 @@
   "Execute BODY, and if an exception is thrown, return the appropriate HTTP response."
   [& body]
   `(try ~@body
+        (catch ApiFieldValidationException e#
+          {:status (.getStatusCode e#)
+           :body {:errors {(keyword (.getFieldName e#)) (.getMessage e#)}}})
         (catch ApiException e#
           {:status (.getStatusCode e#)
            :body (.getMessage e#)})

test/metabase/api/org_test.clj

@@ -123,10 +123,10 @@
       new-org)))
 
 ;; Test input validations on org create
-(expect "'name' is a required param."
+(expect {:errors {:name "field is a required param."}}
   ((user->client :crowberto) :post 400 "org" {}))
 
-(expect "'slug' is a required param."
+(expect {:errors {:slug "field is a required param."}}
   ((user->client :crowberto) :post 400 "org" {:name "anything"}))
 
 
@@ -354,18 +354,18 @@
         (select-keys [:id :admin :user_id :organization_id])))))
 
 ;; Test input validations on org member create
-(expect "'first_name' is a required param."
+(expect {:errors {:first_name "field is a required param."}}
   ((user->client :crowberto) :post 400 (format "org/%d/members" @org-id) {}))
 
-(expect "'last_name' is a required param."
+(expect {:errors {:last_name "field is a required param."}}
   ((user->client :crowberto) :post 400 (format "org/%d/members" @org-id) {:first_name "anything"}))
 
-(expect "'email' is a required param."
+(expect {:errors {:email "field is a required param."}}
   ((user->client :crowberto) :post 400 (format "org/%d/members" @org-id) {:first_name "anything"
                                                                           :last_name "anything"}))
 
 ;; this should fail due to invalid formatted email address
-(expect "Invalid value 'anything' for 'email': Not a valid email address."
+(expect {:errors {:email "Invalid value 'anything' for 'email': Not a valid email address."}}
   ((user->client :crowberto) :post 400 (format "org/%d/members" @org-id) {:first_name "anything"
                                                                           :last_name "anything"
                                                                           :email "anything"}))

test/metabase/api/session_test.clj

@@ -19,10 +19,10 @@
       (client :post 200 "session" (user->credentials :rasta))))
 
 ;; Test for required params
-(expect "'email' is a required param."
+(expect {:errors {:email "field is a required param."}}
   (client :post 400 "session" {}))
 
-(expect "'password' is a required param."
+(expect {:errors {:password "field is a required param."}}
   (client :post 400 "session" {:email "anything@metabase.com"}))
 
 ;; Test for inactive user (user shouldn't be able to login if :is_active = false)
@@ -60,7 +60,7 @@
     (reset-fields-set?)))
 
 ;; Test that email is required
-(expect "'email' is a required param."
+(expect {:errors {:email "field is a required param."}}
   (client :post 400 "session/forgot_password" {}))
 
 ;; Test that email not found gives 404
@@ -96,10 +96,10 @@
     (sel :one :fields [User :reset_token :reset_triggered] :id id)))
 
 ;; Test that token and password are required
-(expect "'token' is a required param."
+(expect {:errors {:token "field is a required param."}}
   (client :post 400 "session/reset_password" {}))
 
-(expect "'password' is a required param."
+(expect {:errors {:password "field is a required param."}}
   (client :post 400 "session/reset_password" {:token "anything"}))
 
 ;; Test that invalid token returns 404

test/metabase/api/setup_test.clj

@@ -27,36 +27,36 @@
 
 
 ;; Test input validations
-(expect "'first_name' is a required param."
+(expect {:errors {:first_name "field is a required param."}}
   (http/client :post 400 "setup/user" {}))
 
-(expect "'last_name' is a required param."
+(expect {:errors {:last_name "field is a required param."}}
   (http/client :post 400 "setup/user" {:first_name "anything"}))
 
-(expect "'email' is a required param."
+(expect {:errors {:email "field is a required param."}}
   (http/client :post 400 "setup/user" {:first_name "anything"
                                        :last_name "anything"}))
 
-(expect "'password' is a required param."
+(expect {:errors {:password "field is a required param."}}
   (http/client :post 400 "setup/user" {:first_name "anything"
                                        :last_name "anything"
                                        :email "anything@metabase.com"}))
 
-(expect "'token' is a required param."
+(expect {:errors {:token "field is a required param."}}
   (http/client :post 400 "setup/user" {:first_name "anything"
                                        :last_name "anything"
                                        :email "anything@metabase.com"
                                        :password "anythingUP12!!"}))
 
 ;; valid email + complex password
-(expect "Invalid value 'anything' for 'email': Not a valid email address."
+(expect {:errors {:email "Invalid value 'anything' for 'email': Not a valid email address."}}
   (http/client :post 400 "setup/user" {:token "anything"
                                        :first_name "anything"
                                        :last_name "anything"
                                        :email "anything"
                                        :password "anything"}))
 
-(expect "Invalid value for 'password': Insufficient password strength"
+(expect {:errors {:password "Insufficient password strength"}}
   (http/client :post 400 "setup/user" {:token "anything"
                                        :first_name "anything"
                                        :last_name "anything"
@@ -64,7 +64,7 @@
                                        :password "anything"}))
 
 ;; token match
-(expect "Invalid value 'anything' for 'token': Token does not match the setup token."
+(expect {:errors {:token "Invalid value 'anything' for 'token': Token does not match the setup token."}}
   (http/client :post 400 "setup/user" {:token "anything"
                                        :first_name "anything"
                                        :last_name "anything"

test/metabase/api/user_test.clj

@@ -183,7 +183,7 @@
                                                                                      :old_password "whatever"}))
 
 ;; Test input validations on password change
-(expect "'password' is a required param."
+(expect {:errors {:password "field is a required param."}}
   ((user->client :rasta) :put 400 (format "user/%d/password" (user->id :rasta)) {}))
 
 ;; Make sure that if current password doesn't match we get a 400