Six digit ID support #1
Conversation
- show Add Card to Patient header only when pending association - cleanup old search results in six digit id flow
| (defn six-digit-ids [patient] | ||
| (or (:six-digit-ids patient) | ||
| (set (repeatedly (rand-int 3) spec/random-sdid)))) | ||
|
|
There was a problem hiding this comment.
Hrm. Shouldn't this be related to the UUIDs? Ideally, I'd have the structure: :patient {:cards [{:uuid "", :six-digit-id ""}]}}
There was a problem hiding this comment.
agreed, will need not so trivial refactoring. Let's do this once we are done with the features?
| (take 100) | ||
| shuffle | ||
| (take 6) | ||
| (apply str))) |
There was a problem hiding this comment.
There was a problem hiding this comment.
this doen't change the code much, and uses a lesser known fn instead of what we use everyday in clojure land. wouldn't like to do this.
There was a problem hiding this comment.
Generation is different from the functionality that clojure.core is meant to do. I would embrace test.check as a core library in this case. See https://clojure.org/guides/spec#_custom_generators.
| (let [six-digit-ids (set (concat (:six-digit-ids patient) | ||
| (map ->six-digit-id | ||
| (:card-uuids patient))))] | ||
| (contains? six-digit-ids six-digit-id))) |
There was a problem hiding this comment.
We wouldn't need to check in both uuids and six-digit-ids. Ideally, the six-digit-ids is just a cached conversion of UUIDs, and potentially some unmapped ones. But we would need to look up only in six-digit-ids. Not for performance reasons, but for simplicity.
There was a problem hiding this comment.
Intertwining six digits and uuids would make it more complex, not simpler. you are already talking about caching and what not.
There was a problem hiding this comment.
I'm not implying that we do caching, just that that's the perspective we should take.
UUIDs and six-digit IDs are both representations of the same thing: The card. And we should model that.
| vals | ||
| (filter #(has-six-digit-id? % six-digit-id)))) | ||
|
|
||
| (defn handle-six-digit-keyboard [{:keys [db]} [_ six-digit-id]] |
There was a problem hiding this comment.
I read this and assumed the function had something to do with the android keyboard :). Which, doesn't appear to be the case. Have you seen assoc-into-db in events?
There was a problem hiding this comment.
- We are already following this convention:
(defn handle-bp-keyboard [{:keys [db]} [_ kind value]]
(cond
(and (= kind :systolic)
(or (and (re-find #"^[12]" value)
(= 3 (count value)))
(and (not (re-find #"^[12]" value))
(= 2 (count value)))))
(.focus (get-in db [:ui :bp :diastolic :ref])))
{:db (assoc-in db [:ui :bp :value kind] value)})assoc-into-dbreturns afn, is not super composable, and is used once in in the codebase. not sure if it should exist.
There was a problem hiding this comment.
We do this when we're writing a full handler anyway. assoc-into-db is not meant to be composed. It's meant to be a simple replacement for the full event handler.
| (let [uuid (:uuid card) | ||
| sdid (:six-digit-id card) | ||
| field-to-update (if uuid :card-uuids :six-digit-ids) | ||
| update-with-value (or uuid sdid)] |
There was a problem hiding this comment.
update-with-value shouldn't be independent from the previous decision of the field to update. We should use something in the lines of:
(case field-to-update
:card-uuids uuid
:six-digit-ids sdid)There was a problem hiding this comment.
This is done in a couple of places, so we could extract this into a function too.
There was a problem hiding this comment.
agreed, will make this change
There was a problem hiding this comment.
actually, i did try abstracting this out, and couldn't settle on the 'right' way to do this. let's pair on this.
|
|
||
| :else | ||
| :pending)] | ||
| [:set-active-card uuid six-digit-id status])) |
There was a problem hiding this comment.
Please extract this cond into a function :) Also, this doesn't feel exhaustive to me. Let's go over this together.
No description provided.