Skip to content

Commit

Permalink
♻️ Extract territory-bro.ui.forms/validation-error-{page,htmx}-response
Browse files Browse the repository at this point in the history
Why:
- Remove duplication of form validation error responses.
- Support throwing ValidationException directly, instead of wrapping it
  in a ring.util.http-response/bad-request!
  • Loading branch information
luontola committed May 12, 2024
1 parent c653763 commit 9069d4d
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 56 deletions.
3 changes: 1 addition & 2 deletions src/territory_bro/api.clj
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@
(require-logged-in!)
(let [cong-id (UUID/fromString (get-in request [:params :congregation]))
user-id (or (parse-uuid (get-in request [:params :userId]))
;; TODO: throw ValidationException directly
(bad-request! {:errors (.getErrors (ValidationException. [[:invalid-user-id]]))}))
(throw (ValidationException. [[:invalid-user-id]])))
state (state-for-request request)]
(db/with-db [conn {}]
(dispatch! conn state {:command/type :congregation.command/add-user
Expand Down
36 changes: 36 additions & 0 deletions src/territory_bro/ui/forms.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
;; Copyright © 2015-2024 Esko Luontola
;; This software is released under the Apache License 2.0.
;; The license text is at http://www.apache.org/licenses/LICENSE-2.0

(ns territory-bro.ui.forms
(:require [territory-bro.ui.html :as html]
[territory-bro.ui.layout :as layout])
(:import (clojure.lang ExceptionInfo)
(territory_bro ValidationException)))

(def validation-error-http-status 422)

(defn validation-error-htmx-response [^Exception e request model! view]
(let [errors (cond
(instance? ValidationException e)
(.getErrors ^ValidationException e)

;; TODO: stop wrapping ValidationException into ring.util.http-response/bad-request!
(instance? ExceptionInfo e)
(when (and (= :ring.util.http-response/response (:type (ex-data e)))
(= 400 (:status (:response (ex-data e)))))
(:errors (:body (:response (ex-data e))))))]
(if (some? errors)
(-> (model! request)
(assoc :errors errors)
(view)
(html/response)
(assoc :status validation-error-http-status))
(throw e))))

(defn validation-error-page-response [^Exception e request model! view]
(let [view (fn [model]
(-> model
(view)
(layout/page! request)))]
(validation-error-htmx-response e request model! view)))
7 changes: 0 additions & 7 deletions src/territory_bro/ui/http_status.clj

This file was deleted.

19 changes: 4 additions & 15 deletions src/territory_bro/ui/registration_page.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@
[ring.util.http-response :as http-response]
[territory-bro.api :as api]
[territory-bro.infra.authentication :as auth]
[territory-bro.ui.forms :as forms]
[territory-bro.ui.html :as html]
[territory-bro.ui.http-status :as http-status]
[territory-bro.ui.i18n :as i18n]
[territory-bro.ui.layout :as layout])
(:import (clojure.lang ExceptionInfo)))
[territory-bro.ui.layout :as layout]))

(defn model! [request]
(auth/with-user-from-session request
Expand Down Expand Up @@ -59,18 +58,8 @@
api-request (assoc request :params {:name cong-name})
cong-id (:id (:body (api/create-congregation api-request)))]
(http-response/see-other (str "/congregation/" cong-id)))
(catch ExceptionInfo e ; TODO: catch ValidationException directly
(let [errors (when (and (= :ring.util.http-response/response (:type (ex-data e)))
(= 400 (:status (:response (ex-data e)))))
(:errors (:body (:response (ex-data e)))))]
(if (some? errors)
(-> (model! request)
(assoc :errors errors)
(view)
(layout/page! request)
(html/response)
(assoc :status http-status/validation-error))
(throw e))))))
(catch Exception e
(forms/validation-error-page-response e request model! view))))

(def routes
["/register"
Expand Down
32 changes: 6 additions & 26 deletions src/territory_bro/ui/settings_page.clj
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@
[territory-bro.api :as api]
[territory-bro.infra.authentication :as auth]
[territory-bro.ui.css :as css]
[territory-bro.ui.forms :as forms]
[territory-bro.ui.html :as html]
[territory-bro.ui.http-status :as http-status]
[territory-bro.ui.i18n :as i18n]
[territory-bro.ui.info-box :as info-box]
[territory-bro.ui.layout :as layout])
(:import (clojure.lang ExceptionInfo)
(java.util UUID)))
(:import (java.util UUID)))

(defn model! [request]
(let [congregation (:body (api/get-congregation request {}))
Expand Down Expand Up @@ -233,36 +232,17 @@
(try
(api/save-congregation-settings request)
(http-response/see-other html/*page-path*)
(catch ExceptionInfo e ; TODO: catch ValidationException directly
(let [errors (when (and (= :ring.util.http-response/response (:type (ex-data e)))
(= 400 (:status (:response (ex-data e)))))
(:errors (:body (:response (ex-data e)))))]
(if (some? errors)
(-> (model! request)
(assoc :errors errors)
(view)
(layout/page! request)
(html/response)
(assoc :status http-status/validation-error))
(throw e))))))
(catch Exception e
(forms/validation-error-page-response e request model! view))))

(defn add-user! [request]
(try
(let [request (update-in request [:params :userId] str/trim)
user-id (parse-uuid (get-in request [:params :userId]))]
(api/add-user request)
(http-response/see-other (str html/*page-path* "/users?new-user=" user-id)))
(catch ExceptionInfo e ; TODO: catch ValidationException directly
(let [errors (when (and (= :ring.util.http-response/response (:type (ex-data e)))
(= 400 (:status (:response (ex-data e)))))
(:errors (:body (:response (ex-data e)))))]
(if (some? errors)
(-> (model! request)
(assoc :errors errors)
(user-management-section)
(html/response)
(assoc :status http-status/validation-error))
(throw e))))))
(catch Exception e
(forms/validation-error-htmx-response e request model! user-management-section))))

(defn remove-user! [request]
(let [request (assoc-in request [:params :permissions] [])
Expand Down
4 changes: 2 additions & 2 deletions test/territory_bro/ui/registration_page_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
[territory-bro.dispatcher :as dispatcher]
[territory-bro.infra.authentication :as auth]
[territory-bro.test.fixtures :refer :all]
[territory-bro.ui.forms :as forms]
[territory-bro.ui.html :as html]
[territory-bro.ui.http-status :as http-status]
[territory-bro.ui.registration-page :as registration-page])
(:import (clojure.lang ExceptionInfo)
(territory_bro ValidationException)))
Expand Down Expand Up @@ -69,7 +69,7 @@
(binding [dispatcher/command! (fn [& _]
(throw (ValidationException. [[:missing-name]])))]
(let [response (registration-page/submit! request)]
(is (= http-status/validation-error (:status response)))
(is (= forms/validation-error-http-status (:status response)))
(is (str/includes?
(html/visible-text (:body response))
(html/normalize-whitespace
Expand Down
8 changes: 4 additions & 4 deletions test/territory_bro/ui/settings_page_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
[territory-bro.infra.authentication :as auth]
[territory-bro.test.fixtures :refer :all]
[territory-bro.test.testutil :refer [replace-in]]
[territory-bro.ui.forms :as forms]
[territory-bro.ui.html :as html]
[territory-bro.ui.http-status :as http-status]
[territory-bro.ui.settings-page :as settings-page])
(:import (clojure.lang ExceptionInfo)
(java.util UUID)
Expand Down Expand Up @@ -243,7 +243,7 @@
(throw (ValidationException. [[:missing-name]
[:disallowed-loans-csv-url]])))]
(let [response (settings-page/save-congregation-settings! request)]
(is (= http-status/validation-error (:status response)))
(is (= forms/validation-error-http-status (:status response)))
(is (str/includes?
(html/visible-text (:body response))
(html/normalize-whitespace
Expand Down Expand Up @@ -288,7 +288,7 @@
(binding [dispatcher/command! (fn [& _]
(throw (ValidationException. [[:no-such-user new-user-id]])))]
(let [response (settings-page/add-user! request)]
(is (= http-status/validation-error (:status response)))
(is (= forms/validation-error-http-status (:status response)))
(is (str/includes?
(html/visible-text (:body response))
(html/normalize-whitespace
Expand All @@ -299,7 +299,7 @@
(throw (ValidationException. [[:invalid-user-id]])))]
(let [request (replace-in request [:params :userId] (str new-user-id) "foo")
response (settings-page/add-user! request)]
(is (= http-status/validation-error (:status response)))
(is (= forms/validation-error-http-status (:status response)))
(is (str/includes?
(html/visible-text (:body response))
(html/normalize-whitespace
Expand Down

0 comments on commit 9069d4d

Please sign in to comment.