feature-friend-request #20
Conversation
| @@ -0,0 +1,9 @@ | |||
| CREATE TABLE friend_requests ( | |||
| id SERIAL, | |||
| sender_username text, | |||
There was a problem hiding this comment.
Use foreign keys to ensure consistency.
| id SERIAL, | ||
| sender_username text, | ||
| recipient_username text, | ||
| request_state text, |
There was a problem hiding this comment.
Consider using postgres Enums for this.
| [chaat.db.user :as user] | ||
| [java-time.api :as jt])) | ||
|
|
||
| ;; Will be more logical to have the user-exists? and exists? check in the model layer. |
There was a problem hiding this comment.
There's a tradeoff here.
- Putting it in the model layer means you're able to visualize the flow better at the correct layer. Putting it in the db layer lets you use db features to do this better.
- If you're doing the check in the model layer, you'll have to use a transaction. Otherwise, you'll have to check again in the db layer anyway (or implicitly, it will fail the uniqueness check which you should handle)
- Just relying on the uniqueness check, will allow you to skip any explicit checks entirely. The downside to this is that it isn't clear from just the code that there is a uniqueness guarantee. However, this may be okay since any complex enough system is relying on the db for a variety of guarantees. It depends on how much you want to tie your app to DB features. The more you rely on them, the more valuable the dependency (but also the more dependent you are and moving away will be harder).
|
|
||
| (defn insert | ||
| "Insert a friend request into the db" | ||
| [db {:keys [sender_username recipient_username] :as content}] |
There was a problem hiding this comment.
content could be renamed to friend-request
There was a problem hiding this comment.
It's usually worth doing the hypen <> underscore translation at the db boundary to make the rest of your clojure code consistent.
| (jdbc/with-transaction [tx (db)] | ||
| (if (and (user/user-exists? tx sender_username) | ||
| (user/user-exists? tx recipient_username)) | ||
| (let [query-result (sql/insert! tx :friend_requests content)] |
There was a problem hiding this comment.
Need to handle what happens when the insert fails.
There was a problem hiding this comment.
The let block might be superfluous if we're not handling those errors. We could just inline (sql/insert! tx :friend_requests content)
| [db id] | ||
| (jdbc/with-transaction [tx (db)] | ||
| (if (exists? tx id) | ||
| (let [query-result (sql/update! (db) :friend_requests {:request_state "accepted"} {:id id}) |
| {:result nil :error (:db-state-error error-table)})) | ||
| {:result nil :error (:friend-request-not-exists error-table)}))) | ||
|
|
||
| (defn reject |
There was a problem hiding this comment.
Extract the common code out into an update-request-state
| (friend-request/create db content))] | ||
| (send-response result))) | ||
|
|
||
| ;; need authentication |
|
|
||
| (defn create | ||
| [db content] | ||
| (let [;; put model layer validation |
There was a problem hiding this comment.
You may want to keep validations at your boundaries.
|
Thank you Neena. We just discussed most of your comments. Also, since chaat development is now parked, I will not continue the discussion here. |
Story: As a user I should be able to send friend requests to other users#3