[16143] Implement Convert Step Updates #16710

wcutshall · 2024-12-04T14:26:20Z

This PR contains coding changes for user story 16144.

Test Steps:

Debug unit tests or integration tests locally with breakpoints set in affected code to verify functionality.
POST a message through Postman (or equivalent) locally and observe the log files.
Connect dev env to testing app insights and verify events show up properly.

Changes

Modified FHIRConverter and associated tests to:
- Only add extensions if message is an ELR.
- Send an ITEM_TRANSFORMED Azure event.

Checklist

Testing

Tested locally?
Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?
Added tests?

Process

No changes.

github-actions · 2024-12-04T14:26:35Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

github-actions · 2024-12-04T14:42:33Z

Test Results

1 264 tests +9 1 260 ✅ +9 7m 41s ⏱️ -7s
164 suites ±0 4 💤 ±0
164 files ±0 0 ❌ ±0

Results for commit 270faef. ± Comparison against base commit 164926e.

♻️ This comment has been updated with latest results.

github-actions · 2024-12-04T14:42:45Z

Integration Test Results

59 files 59 suites 38m 59s ⏱️
423 tests 413 ✅ 10 💤 0 ❌
426 runs 416 ✅ 10 💤 0 ❌

Results for commit 270faef.

♻️ This comment has been updated with latest results.

arnejduranovic · 2024-12-05T19:02:21Z

prime-router/src/main/kotlin/fhirengine/utils/FHIRBundleHelpers.kt

+ *
+ * @return true if has a MesssageHeader that contains an R01 or ORU_R01, otherwise false.
+ */
+fun Bundle.isElr(): Boolean {


RS currently receives test orders and results. Instead of isElr, perhaps a method like getMessageType that returns an enum value like LAB_TEST_RESULT or LAB_TEST_ORDER or UNKOWN would be more reusable? Then, I would say let's stamp the message if we get back either LAB_TEST_RESULT or UNKOWN just to be safe?

Implementing detection for LAB_TEST_ORDER is optional at this point as there is no need to distinguish this as of yet.

Added some code for this with only LAB_RESULT or UNKNOWN as options. Future features will need to add additional values to the enum.

arnejduranovic · 2024-12-05T19:11:00Z

prime-router/src/main/kotlin/fhirengine/utils/FHIRBundleHelpers.kt

+fun Bundle.isElr(): Boolean {
+    var isElr = false
+    if (this.type == Bundle.BundleType.MESSAGE && this.entry.isNotEmpty()) {
+        // By rule, the first entry must be a MessageHeader


Can you link to documentation that says this? This makes me uneasy. See fun Bundle.getDiagnosticReportNoObservations(): List<Base> { below to see how we can utilize FHIRPath here for a better solution imo.

Look at the documentation around "entry" for the Bundle resource

This repeating element order: For bundles of type 'document' and 'message', the first resource is special (must be Composition or MessageHeader respectively). For all bundles, the meaning of the order of entries depends on the bundle type.

arnejduranovic · 2024-12-10T16:52:50Z

prime-router/src/main/kotlin/fhirengine/engine/FHIRConverter.kt

@@ -323,6 +326,10 @@ class FHIRConverter(
                            // We know from the null check above that this cannot be null
                            val bundle = processedItem.bundle!!
                            transformer?.process(bundle)
+                            logger.info(


Some suggestions to make this log message more detailed:

Do this log after you make the child report (line 335) so that you can set childReportId to an actual value

(optional) You can access the processedItem var here, so add the tracking id of the item to this log message: processedItem.getTrackingId().

Good catch. Changes made.

arnejduranovic · 2024-12-10T19:50:59Z

prime-router/src/main/kotlin/fhirengine/engine/FHIRConverter.kt

@@ -508,24 +514,32 @@ class FHIRConverter(
                }
                // 'stamp' observations with their condition code
                if (item.bundle != null) {
+                    val isElr = if (item.bundle!!.getRSMessageType() == RSMessageType.LAB_RESULT) true else false


This can be simplified to: item.bundle!!.getRSMessageType() == RSMessageType.LAB_RESULT

arnejduranovic · 2024-12-10T19:51:43Z

prime-router/src/main/kotlin/fhirengine/engine/FHIRConverter.kt

-                                    UnmappableConditionMessage(
-                                        it.failures.map { it.code },
-                                        it.source
+                        // Only do this if it is an ELR item.


comment not needed, the code is clear enough. Try to use comments to explain the "why" not the "what". In this case, no comment is needed imo.

Cleaned it up.

arnejduranovic · 2024-12-10T19:54:47Z

prime-router/src/main/kotlin/fhirengine/engine/FHIRConverter.kt

                            }
                        }
                    }
+                    logger.info(


Why do we need this exact log message again?

This came out of the conversation we had when we were reviewing the code previously. The gist of the conversation was that you thought we should capture every time the message was modified in some way.

arnejduranovic · 2024-12-10T19:57:47Z

prime-router/src/main/kotlin/fhirengine/utils/FHIRBundleHelpers.kt

+ *
+ * @return true if has a MesssageHeader that contains an R01 or ORU_R01, otherwise false.
+ */
+fun Bundle.isElr(): Boolean {


Have we tried moving the logic here into getRSMessageType? I think that would be cleaner, mostly because the code in this method that reads the message header value is going to be reused by test orders as well as other message types probably, right? I don't see the value of an isELR method.

My thought on this is that, if we intend to add several potential groupings (i.e., enums in RSMessageType), then getRSMessageType would become quite large. Keeping just meaningful function names related to the message grouping type makes the switch statement clearer in my opinion. Additionally, there is an old programming rule of thumb that goes something like "no subroutine should be longer than one screen in length". This, obviously, is rather vague but speaks to keeping code segments small to increase "understandability". You'll often see this argument backed by references like The Magic Number Seven, Plus or Minus Two.

arnejduranovic · 2024-12-10T19:58:49Z

prime-router/src/main/kotlin/fhirengine/utils/FHIRBundleHelpers.kt

+ */
+fun Bundle.getRSMessageType(): RSMessageType {
+    when {
+        isElr() -> return RSMessageType.LAB_RESULT


If you update this method per my suggestion above, you can just do this instead:

event is Coding && ((event.code == "R01") || (event.code == "ORU_R01")) -> return RSMessageType.LAB_RESULT

See my comment about your suggestion above.

arnejduranovic · 2024-12-10T19:59:56Z

prime-router/src/test/kotlin/common/UniversalPipelineTestUtils.kt

@@ -43,14 +43,27 @@ const val validFHIRRecord1 =
    """{"resourceType":"Bundle","id":"1667861767830636000.7db38d22-b713-49fc-abfa-2edba9c12347","meta":{"lastUpdated":"2022-11-07T22:56:07.832+00:00"},"identifier":{"value":"1234d1d1-95fe-462c-8ac6-46728dba581c"},"type":"message","timestamp":"2021-08-03T13:15:11.015+00:00","entry":[{"fullUrl":"Observation/d683b42a-bf50-45e8-9fce-6c0531994f09","resource":{"resourceType":"Observation","id":"d683b42a-bf50-45e8-9fce-6c0531994f09","status":"final","code":{"coding":[{"system":"http://loinc.org","code":"80382-5"}],"text":"Flu A"},"subject":{"reference":"Patient/9473889b-b2b9-45ac-a8d8-191f27132912"},"performer":[{"reference":"Organization/1a0139b9-fc23-450b-9b6c-cd081e5cea9d"}],"valueCodeableConcept":{"coding":[{"system":"http://snomed.info/sct","code":"260373001","display":"Detected"}]},"interpretation":[{"coding":[{"system":"http://terminology.hl7.org/CodeSystem/v2-0078","code":"A","display":"Abnormal"}]}],"method":{"extension":[{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/testkit-name-id","valueCoding":{"code":"BD Veritor System for Rapid Detection of SARS-CoV-2 & Flu A+B_Becton, Dickinson and Company (BD)"}},{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/equipment-uid","valueCoding":{"code":"BD Veritor System for Rapid Detection of SARS-CoV-2 & Flu A+B_Becton, Dickinson and Company (BD)"}}],"coding":[{"display":"BD Veritor System for Rapid Detection of SARS-CoV-2 & Flu A+B*"}]},"specimen":{"reference":"Specimen/52a582e4-d389-42d0-b738-bee51cf5244d"},"device":{"reference":"Device/78dc4d98-2958-43a3-a445-76ceef8c0698"}}}]}"""
 const val validFHIRRecord1Identifier = "1234d1d1-95fe-462c-8ac6-46728dba581c"

+@Suppress("ktlint:standard:max-line-length")
+const val validFHIRRecord1a =
+    """{"resourceType":"Bundle","id":"1667861767830636000.7db38d22-b713-49fc-abfa-2edba9c12347","meta":{"lastUpdated":"2022-11-07T22:56:07.832+00:00"},"identifier":{"value":"1234d1d1-95fe-462c-8ac6-46728dba581c"},"type":"message","timestamp":"2021-08-03T13:15:11.015+00:00","entry":[{"fullUrl":"MessageHeader/0993dd0b-6ce5-3caf-a177-0b81cc780c18","resource":{"resourceType":"MessageHeader","id":"0993dd0b-6ce5-3caf-a177-0b81cc780c18","extension":[{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/encoding-characters","valueString":"^~\\&#"},{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/character-set","valueString":"UNICODE UTF-8"},{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/msh-message-header","extension":[{"url":"MSH.7","valueString":"20230501102531-0400"}]}],"eventCoding":{"system":"http://terminology.hl7.org/CodeSystem/v2-0003","code":"R01","display":"ORU^R01^ORU_R01"},"sender":{"reference":"Organization/1710886092467181000.213628f7-9569-4400-a95d-621c3bfbf121"}}},{"fullUrl":"Observation/d683b42a-bf50-45e8-9fce-6c0531994f09","resource":{"resourceType":"Observation","id":"d683b42a-bf50-45e8-9fce-6c0531994f09","status":"final","code":{"coding":[{"system":"http://loinc.org","code":"80382-5"}],"text":"Flu A"},"subject":{"reference":"Patient/9473889b-b2b9-45ac-a8d8-191f27132912"},"performer":[{"reference":"Organization/1a0139b9-fc23-450b-9b6c-cd081e5cea9d"}],"valueCodeableConcept":{"coding":[{"system":"http://snomed.info/sct","code":"260373001","display":"Detected"}]},"interpretation":[{"coding":[{"system":"http://terminology.hl7.org/CodeSystem/v2-0078","code":"A","display":"Abnormal"}]}],"method":{"extension":[{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/testkit-name-id","valueCoding":{"code":"BD Veritor System for Rapid Detection of SARS-CoV-2 & Flu A+B_Becton, Dickinson and Company (BD)"}},{"url":"https://reportstream.cdc.gov/fhir/StructureDefinition/equipment-uid","valueCoding":{"code":"BD Veritor System for Rapid Detection of SARS-CoV-2 & Flu A+B_Becton, Dickinson and Company (BD)"}}],"coding":[{"display":"BD Veritor System for Rapid Detection of SARS-CoV-2 & Flu A+B*"}]},"specimen":{"reference":"Specimen/52a582e4-d389-42d0-b738-bee51cf5244d"},"device":{"reference":"Device/78dc4d98-2958-43a3-a445-76ceef8c0698"}}}]}"""


why did these 1a versions need to be created?

This was something that I meant to ask before I committed the code. We never reached a definitive conclusion about whether or not we could expect the event.Coding in the MessageHeader. If I just add it to the existing variables then it will break other tests. The 1a versions were added to support my tests without breaking the other existing tests.

arnejduranovic · 2024-12-10T21:22:40Z

prime-router/src/test/kotlin/fhirengine/engine/FhirConverterTests.kt

@@ -544,6 +646,12 @@ class FhirConverterTests {
        }
    }

+//    @Test


Please delete

sonarcloud · 2024-12-11T14:05:48Z

Quality Gate passed

Issues
16 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

wcutshall added 2 commits December 3, 2024 14:49

[16143] Checking in WIP.

e5d7a5a

[16143] Checking in WIP.

2891d0c

wcutshall had a problem deploying to staging December 4, 2024 14:26 — with GitHub Actions Failure

wcutshall added the platform Platform Team label Dec 4, 2024

Merge branch 'main' into platform/bill/16143

f3e19c0

wcutshall had a problem deploying to staging December 4, 2024 14:30 — with GitHub Actions Failure

wcutshall added 2 commits December 4, 2024 13:45

[16143] Added new code to check for ELR..

12aaeb2

[16143] Removed tmp file.

48cf73b

wcutshall temporarily deployed to staging December 4, 2024 18:47 — with GitHub Actions Inactive

[16143] Added additional testing to integration test.

63d3649

wcutshall temporarily deployed to staging December 5, 2024 14:49 — with GitHub Actions Inactive

arnejduranovic reviewed Dec 5, 2024

View reviewed changes

[16143] Cleanup based on PR review.

87e3c7a

wcutshall temporarily deployed to staging December 6, 2024 21:06 — with GitHub Actions Inactive

[16143] Added code based on PR review.

466e4d7

wcutshall temporarily deployed to staging December 9, 2024 19:58 — with GitHub Actions Inactive

Merge branch 'main' into platform/bill/16143

7224ee4

wcutshall temporarily deployed to staging December 10, 2024 16:26 — with GitHub Actions Inactive

wcutshall marked this pull request as ready for review December 10, 2024 17:08

wcutshall requested a review from a team as a code owner December 10, 2024 17:08

arnejduranovic requested changes Dec 10, 2024

View reviewed changes

[16143] Cleanup from PR review.

270faef

wcutshall temporarily deployed to staging December 11, 2024 13:43 — with GitHub Actions Inactive

arnejduranovic self-assigned this Dec 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16143] Implement Convert Step Updates #16710

[16143] Implement Convert Step Updates #16710

wcutshall commented Dec 4, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

arnejduranovic Dec 5, 2024

wcutshall Dec 9, 2024

arnejduranovic Dec 5, 2024

wcutshall Dec 6, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024 •

edited

Loading

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

arnejduranovic Dec 10, 2024

wcutshall Dec 11, 2024

sonarcloud bot commented Dec 11, 2024

[16143] Implement Convert Step Updates #16710

Are you sure you want to change the base?

[16143] Implement Convert Step Updates #16710

Conversation

wcutshall commented Dec 4, 2024 • edited Loading

Changes

Checklist

Testing

Process

github-actions bot commented Dec 4, 2024 • edited Loading

Dependency Review

OpenSSF Scorecard

Scanned Manifest Files

github-actions bot commented Dec 4, 2024 • edited Loading

Test Results

github-actions bot commented Dec 4, 2024 • edited Loading

Integration Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcutshall Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 11, 2024

Quality Gate passed

wcutshall commented Dec 4, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

wcutshall Dec 11, 2024 •

edited

Loading