6306 add next of kin to patient edit #6339
Conversation
β¦ext-of-kin-to-patient-edit
β¦ext-of-kin-to-patient-edit
github-actions
bot
added
feature: vaccine card
Team: KororΔ π§
Bundle size differenceComparing this PR to
|
|
Hi @Chris-Petty FYI I just rebased this off #6348 so patient input should be working for u to test β€οΈ |
| if (!props.visible) { | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Wouldn't it be better put this at the top of the func and avoid fetching the patient data for an invisible component?
There was a problem hiding this comment.
Nit: could have destructured visible out like the rest of the props at the top
There was a problem hiding this comment.
Hahaha I was curious what ControlProps were, it's a rabbit hole of jsonforms package types all the way down. Dunno how you're supposed to know what props a component even has, or rather implements? Like how do you know a component actually does anything with the visible prop? Let alone the other dozen on ControlProps that we really don't use in this instance.
Just kinda feels like typescript going full circle, typescripting so hard that the intellisense no longer is reliable haha
There was a problem hiding this comment.
I actually should have requested changes on the first 2 but I forgot about these comments lol
There was a problem hiding this comment.
Hahaha I was curious what ControlProps were...
Haha gonna be so honest I just copied from the other JsonForm components, assumed they were handled one way or another π
Wouldn't it be better put this at the top of the func and avoid fetching the patient data for an invisible component?
Can't conditionally render a hook, but can pull this out into another component π 10e4d3c
There was a problem hiding this comment.
Hahaha I was curious what ControlProps were...
Haha gonna be so honest I just copied from the other JsonForm components, assumed they were handled one way or another π
Wouldn't it be better put this at the top of the func and avoid fetching the patient data for an invisible component?
Can't conditionally render a hook, but can pull this out into another component π 10e4d3c
Oh duh yea, and whoa life hack!
| is_deceased, | ||
| date_of_death, | ||
| next_of_kin_id, | ||
| }; |
There was a problem hiding this comment.
I've never really questioned it and assumed it must have something to do with DB read/writes as it's often in those areas and tests. But seeing as it's presented itself here... why do we have inline_init it seems like it's just instantiating a struct but with more steps?
To take another random example:
fn row_4() -> SyncBufferRow {
inline_init(|r: &mut SyncBufferRow| {
r.record_id = "4".to_string();
r.table_name = "name".to_string();
r.received_datetime = Defaults::naive_date_time();
})
}Or
fn row_4() -> SyncBufferRow {
SyncBufferRow {
record_id: "4".to_string(),
table_name: "name".to_string(),
received_datetime: Defaults::naive_date_time(),
..Default::default()
}
}Only tangible difference is having to call ..Default::default(), which the compiler will remind you of anyway. I figure a rust NERD would say the latter is more idiomatic, just using the lang feature for instantiating. Also just more readable/less chars imo. Just a guess, because rust has such a surface area or even changed, perhaps ..Default::default() just flew under the radar heh. Pretty easy to happen when std structs can have like 100 std implemented functions.
One heck of a refactor issue π€£
There was a problem hiding this comment.
Yeah inline_init is old - recommending ..Default::default() now. Yep like you say pretty sure default() was missed, but we should lean on language features. I try to change when I run into them, but yet another thing we haven't taken the time to clean up yet π
Especially in this case it's not good, because we actually don't want to default the rest of the input fields, we want to handle them all π
| @@ -5,6 +5,7 @@ mod add_index_to_sync_buffer; | |||
| mod add_name_next_of_kin_id; | |||
| mod add_program_deleted_datetime; | |||
| mod backend_plugins; | |||
|
|
|||
Should be workin? |
Fixes #6306
π©π»βπ» What does this PR do?
Adds next of kin to patient edit:
Note that this will need to be mapped separately for custom patient implementations in program schemas repo π
π Any notes for the reviewer?
NOTE THAT PATIENT INPUT CURRENTLY NOT WORKING PER #6338- working now :)π§ͺ Testing
π Documentation
1.
2.