-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug fix: Add units to lab component values #2920
Conversation
@ashton-skylight Hi, this is ready for review! Just a quick question, do you have any thoughts on whether or not you want a space between the value and the unit? Right now, there's a space, so that something like |
@@ -126,7 +126,7 @@ specimenReceivedTime: "Observation.extension[0].extension.where(url = 'specimen | |||
specimenSource: "Observation.extension[0].extension.where(url = 'specimen source').valueString" | |||
observationReferenceValue: "Observation.extension[0].extension.where(url = 'observation entry reference value').valueString" | |||
observationComponent: "code.coding.display.first()" | |||
observationValue: (valueQuantity.value.toString() | valueString | valueCodeableConcept.coding.display | iif(interpretation.coding.display.exists(), ' (' | interpretation.coding.display | ')', '')).join('') | |||
observationValue: (valueQuantity.value.toString() | valueString | valueCodeableConcept.coding.display | iif(valueQuantity.unit.exists(), ' ' | valueQuantity.unit, '') | iif(interpretation.coding.display.exists(), ' (' | interpretation.coding.display | ')', '')).join('') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the value quantity unit and interpretation coding display both exist, would we get double units now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! the fhirpath logic is a lot to parse (as a human) there - do you think it would be worth adding a comment, something like
# <value> <units> (<flag>)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added! TY Mary!
@angelathe This looks great! Is there a way that we could remove the space for %? |
Hi @ashton-skylight, yes we can! Should that also be the case for units under the |
@angelathe Amazing yes please! Preferably, it would be best to only remove the space for percentages in the value and ref range. If removing the space in the % affects everything (ie. M/ml or days), we'll keep it with the space as is. |
Making a note that fixing the ref range unit spacing will be be made into its own ticket |
Co-authored-by: Mary McGrath <m.c.mcgrath13@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
PULL REQUEST
Summary
Related Issue
Fixes #2671
Acceptance Criteria
Screenshots
Before (
10e5b07b-5d3f-426f-a7e1-13ea909ff5a5
):After (
10e5b07b-5d3f-426f-a7e1-13ea909ff5a5
):Checklist
Examples from QA process: