-
Notifications
You must be signed in to change notification settings - Fork 55
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
Map single entry symptom responses to FHIR #8183
Conversation
@@ -802,7 +807,7 @@ private void addCorrectionNote( | |||
} | |||
} | |||
|
|||
public Set<Observation> convertToAOESymptomObservation( | |||
public Set<Observation> convertToAOESymptomaticObservation( |
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.
@@ -51,6 +51,8 @@ private FhirConstants() { | |||
|
|||
public static final String LOINC_AOE_IDENTIFIER = "81959-9"; | |||
public static final String LOINC_AOE_SYMPTOMATIC = "95419-8"; | |||
public static final String LOINC_SYMPTOM_TIMING_PANEL = "99582-9"; |
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.
@@ -51,6 +51,8 @@ private FhirConstants() { | |||
|
|||
public static final String LOINC_AOE_IDENTIFIER = "81959-9"; | |||
public static final String LOINC_AOE_SYMPTOMATIC = "95419-8"; | |||
public static final String LOINC_SYMPTOM_TIMING_PANEL = "99582-9"; | |||
public static final String LOINC_SYMPTOM = "75325-1"; |
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.
@@ -378,6 +378,51 @@ public static String parseState(String s) { | |||
throw IllegalGraphqlArgumentException.invalidInput(s, "state"); | |||
} | |||
|
|||
private static final Map<String, String> RESPIRATORY_SYMPTOMS = |
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.
separated them out like the frontend -- thought this would make things easier to read and consistent with the frontend
Let me know if you all prefer one map of these symptoms
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.
This logic makes sense to me, especially considering that it's how we are making the distinction between symptoms across the rest of the app.
bb0be5d
to
fa1b9b9
Compare
fa1b9b9
to
5f7623b
Compare
List<String> positiveSymptoms = getPositiveSymptoms(surveyData.getSymptoms()); | ||
observations.addAll(convertToSymptomsObservations(positiveSymptoms)); |
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.
This is very clean and makes sense to me from a logic perspective. However, does anyone know why we weren't making the distinction between postive and negative symptoms before in the Fhir conversion?
Or was that distinction already being made, just without the helper function?
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.
We were only adding the "Yes" symptomatic observation if there were true
symptom values (see line 973 above)
Since that only checks if any symptom value is true
, I needed to create an additional method to return all the symptoms that were true
so it's easier to create an Observation
for each positive symptom.
Let me know if that makes sense or you see any other optimization! 😸
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.
I think this is a tiny bit unclear - I would have initially assumed Positive
and Negative
meant that it was a positive or negative test result.
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.
I'll rename this and the variable... maybe something like trueSymptoms
and getTrueSymptoms
edit: ended up renaming it as getFilteredTrueSymptoms
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.
Ok I updated this! Let me know if that makes it more clear! 😸
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.
Small nit for the name, I think symptomsPresent
and getFilteredSymptomsPresent
would be clearer than "true" and be consistent with the clinical context of patients presenting with certain symptoms
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.
Thank you for the suggestion! I like those better! Naming is hard 😅
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.
Left some clarifying questions for myself but overall looks great! Thanks for your work on this
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.
LGTM testing it out locally! Great work on this!
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.
Overall this looks good. Left one comment on a piece I think could be more clear.
4703aa6
Quality Gate passedIssues Measures |
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.
🚢
BACKEND PULL REQUEST
Related Issue
Changes Proposed
Additional Information
Testing
elisa/log-6609-symptoms-fhir-single-entry
branch has been pushed so you can see a log in your backend console with the symptoms mapped to FHIR