Conversation
| onChange={(_, selectedLab: any) => { | ||
| const alreadySelected = selectedLabs.find((tempLab) => { | ||
| const tempLabCode = `${tempLab.item.itemCode}${tempLab.lab.labGuid}`; | ||
| const selectedLabCode = `${selectedLab.item.itemCode}${selectedLab.lab.labGuid}`; |
There was a problem hiding this comment.
Would it be sufficient to use the lab.item.uniqueName as the comparison here rather than itemCode + labGuid? I see that uniqueName is used as the getOptionKey so it would at least be consistent. And if the answer is that uniqueName wouldn't be unique enough for this comparison, then I wonder if uniqueName should be replaced as the getOptionKey
There was a problem hiding this comment.
Yeah that sounds good to me.
| setSelectedLabs((prev) => | ||
| prev.filter((tempLab) => { | ||
| const tempLabCode = `${tempLab.item.itemCode}${tempLab.lab.labGuid}`; | ||
| const labCode = `${lab.item.itemCode}${lab.lab.labGuid}`; |
There was a problem hiding this comment.
Same thought here about this vs uniqueName
There was a problem hiding this comment.
Yeah lets use uniqueName
| const errorMessage = [sdkError.message]; | ||
| setError(errorMessage); |
There was a problem hiding this comment.
Could consolidate these lines
| </DialogTitle> | ||
|
|
||
| <DialogContent dividers> | ||
| {labSets.map((set, idx) => ( |
There was a problem hiding this comment.
What's the state of the UI when labSets.length === 0?
There was a problem hiding this comment.
i send undefined from backend when labsets len is 0 so that should never happen
| labs: { | ||
| display: string; // list of names of the tests contained in this list formatted list {lab name / filler lab name} |
There was a problem hiding this comment.
love the comment but I wonder if
// list of names of the tests contained in this list
should live above line 25 and the rest stay where it is, e.g.
| labs: { | |
| display: string; // list of names of the tests contained in this list formatted list {lab name / filler lab name} | |
| // list of names of the tests contained in this list | |
| labs: { | |
| display: string; // formatted 'test name / filler lab name' |
There was a problem hiding this comment.
But the object isn't just a list of names, its a list of display, itemCode and labGuid
There was a problem hiding this comment.
Ok, what about this? My primary reason for changing this is rn it makes it seem like "display" is a list of something when it isn't
| labs: { | |
| display: string; // list of names of the tests contained in this list formatted list {lab name / filler lab name} | |
| // tests contained in this list | |
| labs: { | |
| display: string; // formatted list {test name / filler lab name} |
There was a problem hiding this comment.
Ah okay, that makes sense, will update
| dx, | ||
| encounter, | ||
| orderableItem, | ||
| encounter, // why do we send the whole encounter? can probably just do the id and grab the resource later |
There was a problem hiding this comment.
At the very least to check encounter.account no?
There was a problem hiding this comment.
I dunno it feels weird to send the whole object and then use that data from the front end rather than fetching the resource from the back end. I have no intention of changing this now, just more of a note to someone in the future if they are doing work here thats less disruptive, i think it would be good to change.
|
|
||
| validateLabOrgAndOrderingLocationAndGetAccountNumber(labOrganization, orderingLocation); | ||
| // create lab order requests will come in for the same patient, encounter, psc flag, location | ||
| // the only thing that is potentially different is the lab the test will be run by |
There was a problem hiding this comment.
Lol "is the lab the test will be run by" - i can see how thats not super clear, will update to "the only potential difference is the lab that will perform the test"
| clinicalInfoNoteByUser, | ||
| } = resources; | ||
| console.log( | ||
| `Formatting fhir batch input requests. Requisition: ${requisitionNumber} Test: ${orderableItem.item.itemName} ${orderableItem.item.itemCode}` |
There was a problem hiding this comment.
To help disambiguate, would be nice to include the labGuid in this log too please
| const formattedListDTOs: LabListsDTO[] = []; | ||
| labLists.forEach((list) => { | ||
| const formatted: LabListsDTO = { | ||
| listId: list.id ?? 'missing', |
There was a problem hiding this comment.
Since listId gets used as a FE component key, might as well stick an idx in here as well to keep listId unique so the FE isn't mad. But also I know this is a type thing not something that actually happens
| if (selectedLabSet) { | ||
| console.log('searching orderable items for the lab set', selectedLabSet.listName); | ||
| const labRequests = selectedLabSet.labs.map((lab) => { | ||
| return getLabs([lab.labGuid], { itemCodes: [lab.itemCode] }, m2mToken); | ||
| }); | ||
| const allLabsResults = await Promise.all(labRequests); | ||
| labs = allLabsResults.flat(); |
There was a problem hiding this comment.
Depending on your answer here maybe this could go away?
There was a problem hiding this comment.
I don't see how we can get away without doing this.
There was a problem hiding this comment.
Yeah I forgot we stick basically all of orderable item info on the ServiceRequest so when I saw it earlier I thought we were just using the test name and lab guid. But you're right that this would be tough to get around
2 participants
I'm going to create another PR pointing at this branch for inhouse, but wanted to break it up a little to make it easier to review, i'll include the tf config file there.
https://linear.app/zapehr/issue/OTR-1534/add-ability-to-select-from-lab-sets