Inbound OML Support #13766
Inbound OML Support #13766
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
Integration Test Results 52 files 52 suites 13m 43s ⏱️ Results for commit 93df8dd. ♻️ This comment has been updated with latest results. |
| @@ -68,7 +68,7 @@ OBX|1|CWE|94558-4^SARS-CoV-2 (COVID-19) Ag [Presence] in Respiratory specimen by | |||
|
|
|||
| // Source: https://confluence.hl7.org/display/OO/v2+Sample+Messages | |||
| val unsupportedHL7 = """ | |||
| MSH|^~\&#|NIST EHR|NIST EHR Facility|NIST Test Lab APP|NIST Lab Facility|20130211184101-0500||OML^O21^OML_O21|NIST-LOI_5.0_1.1-NG|T|2.5.1|||AL|AL||||| | |||
| MSH|^~\&#|NIST EHR|NIST EHR Facility|NIST Test Lab APP|NIST Lab Facility|20130211184101-0500||OML^O33^OML_O33|NIST-LOI_5.0_1.1-NG|T|2.5.1|||AL|AL||||| | |||
There was a problem hiding this comment.
Note: This is suppose to fail. Adding support for OML_O21 means I had to change this to a different, still unsupported message type.
There was a problem hiding this comment.
@oslynn -- I believe so, yes. There's a test for a valid ORU message before this -- then this message is used as part of an assertFailure.
There was a problem hiding this comment.
I'm not actually using this test mapping at present... should I? Or should this change be removed?
| @@ -126,7 +127,9 @@ class HL7Reader(private val actionLogger: ActionLogger) : Logging { | |||
| v27_ORU_R01::class.java, | |||
| v251_ORU_R01::class.java | |||
| ) | |||
|
|
|||
| "OML" -> listOf( | |||
There was a problem hiding this comment.
I'm confused why OML needed to be added here to avoid a warning, yet ORM is nowhere to be found.
There was a problem hiding this comment.
@victor-chaparro @arnejduranovic are you able to provide some insight on this?
There was a problem hiding this comment.
ORM should also be added. I just tried processing an ORM message and got the warning.
There was a problem hiding this comment.
@victor-chaparro Any objections to me making that part of a separate PR? I was going to add it, but attempting to do so is inexplicably causes unit tests to fail:
Failed to map supported failure 'org.opentest4j.AssertionFailedError: expected to be true' with mapper 'org.gradle.api.internal.tasks.testing.failure.mappers.OpenTestAssertionFailedMapper@17bc1586': Cannot invoke "Object.getClass()" because "obj" is null
> Task :test
gov.cdc.prime.router.fhirengine.utils.FHIRBundleHelpersTests Test enhance bundle metadata 2-7() FAILED
org.opentest4j.AssertionFailedError: expected to be true
at app//gov.cdc.prime.router.fhirengine.utils.FHIRBundleHelpersTests.Test enhance bundle metadata 2-7(FHIRBundleHelpersTests.kt:672)
FAILURE: Executed 1097 tests in 1m 36s (1 failed, 4 skipped)
|
Deploying branch Storybook to Chromatic... |
There was a problem hiding this comment.
This all looks fine to me, but I would love if @oslynn @victor-chaparro or @arnejduranovic could also review.
|
Looking good to me. |
|
This PR adds support for inbound OML (laboratory order) messages. These are similar to the ORM (generic order) messages that are already supported.
Test Steps:
Changes
Checklist
Testing
./prime testor./gradlew testSmokeagainst local Docker ReportStream container?npm run lint:write? n/aProcess
Linked Issues
To Be Done
Specific Security-related subjects a reviewer should pay specific attention to
If you answered 'yes' to any of the questions above, conduct a detailed Review that addresses at least: