Eliminate warnings for missing fields #1656

jbiskie · 2024-12-12T19:33:27Z

Description

This PR updates and/or removes unnecessary fields in our FHIR mappings that were causing warnings when sending messages.

Issue

#940

When an extension contains a URL but no value of any type, the message "Resource is missing required element: value" is printed in the log.

There were 3 "value-less" extensions remaining that were producing these warnings.

Patient identifier type code CX-5 (PID-3.5 in the v2 message) - transform: RemovePatientIdentifiers
Patient name type code XPN-7 (PID-5.7 in the v2 message) - transform: RemovePatientNameTypeCode
Observation request OBR-16 (occurs only when ORC-12 is blank - transform: CopyOrcOrderProviderToObrOrderProvider

This PR ensures that the "value-less" extensions do not get added during the TI transforms and does some additional cleanup of the transform output to ensure that the values being cleared by the transforms no longer exist in other places in the affected resource.

See the Areas of Concern comment in the issue for more details.

Testing instructions

Run a sample CA message with empty ORC-12 and OBR-16 through RS and TI end to end (such as CA examples 7, 19, or 20). Review the transformed fhir and final HL7.
The transformed fhir should have no extensions without a value (specifically CX.5, XPN.7, and OBR.16)
There should be no change in the final HL7

Checklist

I have added tests to cover my changes

…meter refactor, fix logic for existence of practitioner

Co-Authored-By: Joel Biskie <84460447+jbiskie@users.noreply.github.com> Co-Authored-By: Jeremy Rosenfeld <10262289+JeremyIR@users.noreply.github.com>

Co-authored-by: tjohnson@flexion.us Co-authored-by: jrosenfeld@flexion.us

github-actions · 2024-12-12T19:34:22Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Possible Bug The method `getObr16Extension` might return null, which is not handled in `getObr16ExtensionPractitioner`. This could lead to a NullPointerException when calling `obr16Extension.value`. Code Improvement The method `removePID3_5Value` could be optimized by checking if the extension exists before attempting to remove it, which would avoid unnecessary method calls and potential NullPointerExceptions.

...iary/etor/ruleengine/transformation/custom/CopyOrcOrderProviderToObrOrderProviderTest.groovy

shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiHelper.java

github-actions · 2024-12-12T19:36:19Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add null check for `obrExtension` to prevent potential NullPointerException Ensure that the `getObr16Extension` method handles the possibility of `obrExtension` being null to avoid potential NullPointerException when calling `getExtensionByUrl`. etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/transformation/custom/CopyOrcOrderProviderToObrOrderProviderTest.groovy [240-243] Extension getObr16Extension(serviceRequest) { def obrExtension = serviceRequest.getExtensionByUrl(HapiHelper.EXTENSION_OBR_URL) + if (obrExtension == null) return null def obr16Extension = obrExtension.getExtensionByUrl(HapiHelper.EXTENSION_OBR16_DATA_TYPE.toString()) return obr16Extension } Suggestion importance[1-10]: 8 Why: Adding a null check for `obrExtension` is crucial to prevent runtime exceptions when the extension is not present, which enhances the robustness of the method.	8
	Add null check for `organization` to prevent NullPointerException Check if `organization` is null after retrieving it in `getPID3_4Identifier` to avoid NullPointerException when calling `getResource`. shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiHelper.java [265-274] public static Identifier getPID3_4Identifier(Bundle bundle) { Identifier identifier = getPID3Identifier(bundle); if (identifier == null) { return null; } Organization organization = (Organization) identifier.getAssigner().getResource(); + if (organization == null) { + return null; + } return organization.getIdentifierFirstRep(); } Suggestion importance[1-10]: 8 Why: This suggestion correctly adds a null check for `organization` to avoid NullPointerException when the organization is not present, which is a critical improvement for error handling.	8
	Ensure `patientIdentifier` is properly checked for null and necessary conditions before usage Add a null check for `patientIdentifier` before accessing its methods in `removePID3_5Value` to prevent NullPointerException. shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiHelper.java [294-297] public static void removePID3_5Value(Identifier patientIdentifier) { - if (patientIdentifier == null) { + if (patientIdentifier == null \|\| !patientIdentifier.hasExtension(HapiHelper.EXTENSION_CX_IDENTIFIER_URL)) { return; } ... } Suggestion importance[1-10]: 7 Why: The suggestion correctly identifies the need for additional null checks to prevent potential NullPointerException, which improves the method's safety and reliability.	7
	Add checks to ensure the practitioner reference is valid before setting it Ensure that `setOBR16WithPractitioner` checks if `practitionerRole.getPractitioner()` has a valid reference before setting the value to avoid setting invalid data. shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiHelper.java [543-553] public static void setOBR16WithPractitioner( Extension obrExtension, PractitionerRole practitionerRole) { - if (practitionerRole == null \|\| !practitionerRole.getPractitioner().hasReference()) { + if (practitionerRole == null \|\| !practitionerRole.getPractitioner().hasReference() \|\| practitionerRole.getPractitioner().getReference().isEmpty()) { return; } ... } Suggestion importance[1-10]: 6 Why: The suggestion enhances the method by ensuring that the practitioner reference is valid before setting it, which prevents the setting of invalid data and improves data integrity.	6

… check in getObr16ExtensionPractitioner

Co-authored-by: tjohnson7021 Co-authored-by: jeremyir

...termediary/etor/ruleengine/transformation/custom/CopyOrcOrderProviderToObrOrderProvider.java

basiliskus · 2024-12-13T21:58:28Z

.../cdc/trustedintermediary/etor/ruleengine/transformation/custom/RemovePatientIdentifiers.java

-        HapiHelper.setPID3_4Value(bundle, ""); // remove PID.3-4
-        HapiHelper.setPID3_5Value(bundle, ""); // remove PID.3-5
+
+        Identifier patientIdentifier = HapiHelper.getPID3Identifier(bundle);


Can lines 20 to 24 be extracted to HapiHelper.setPID3_4Value?

Refactored.

basiliskus · 2024-12-13T22:00:21Z

...cdc/trustedintermediary/etor/ruleengine/transformation/custom/RemovePatientNameTypeCode.java

-        // Need to set the value for extension to empty instead of removing the extension,
-        // otherwise RS will set its own value in its place
-        HapiHelper.setPID5_7ExtensionValue(bundle, null);
+        HapiHelper.removePID5_7Value(bundle);


I just want to confirm that RS is not adding a value in PID-5.7 if we completely remove it instead of setting it to null

Correct. We noted the same behavior as the old comments indicated initially. We also need to null patient name use so ReportStream doesn't do a secondary mapping. I went ahead and added a link in the PR description to some comments in the task.

basiliskus · 2024-12-13T22:04:12Z

...iary/etor/ruleengine/transformation/custom/CopyOrcOrderProviderToObrOrderProviderTest.groovy

@@ -235,18 +237,18 @@ class CopyOrcOrderProviderToObrOrderProviderTest extends Specification{
        assert codingSystem == null || codingSystem[0]?.code == expectedIdentifierTypeCode
    }

+    Extension getObr16Extension(serviceRequest) {


Would it be useful to have getObr16Extension and some of the functions in this test in HapiHelper to be reused?

Refactored the OBR-16 and ORC-12 methods into HapiHelper

basiliskus

Great work!

…scriptes/README.md

- This is based on if you're running TI in docker vs gradle

… can default to leaving it as a config change per user environment Co-authored-by: Sylvie <sschuresko@flexion.us> Co-authored-by: GilmoreA6 <james.gilmore@agile6.com> Co-authored-by: pluckyswan <96704946+pluckyswan@users.noreply.github.com>

- template update to rename variables - README update to reference where local set up of RS and TI are - update local variables to be lower case

* comment to start branch * getFieldsAnnotatedWithInstance helper method * todo comment deleted * injectIntoField overloaded method * injectIntoNonSingleton * access modifier protected to public * revert access modifier change * changes: added logger to HapiFhirResource in order to test a non-singleton class created HapiFhirResourceTest in order to test the changes to HapiFhirResource * Added implementors unit test to ApplicationContextTest * Minor refactoring on some AppContext injection methods * Make the skip flag on TestApplicationContext switchable from the test classes * Update ApplicationContext.java Remove unreachable if statement * Update TestApplicationContext.groovy Add additional line in coverage * Update TestApplicationContext.groovy Remove parameter from injectRegisteredImplementations * added unit tests to cover new code * Update ApplicationContextTest.groovy Added test case for unsupported injection classes * Update ApplicationContext.java Print an error message if the class implementation is not found * use return instead of System.err * refactoring - dry * thown(NullPointerException) -> noExceptionThrown() * added comments to indicate changes that will be deleted * deleted commented out code * reinstated comments for test case * deleted test changes: in HapiFhirResource and deleted file HapifhirResourceTest.groovy --------- Co-authored-by: Luis Pabon <turkeyfried@gmail.com> Co-authored-by: Luis Pabon <luis.pabon@agile6.com>

…lue, refactor PID-3.4 testing into discrete unit tests

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

sonarqubecloud · 2024-12-18T18:17:47Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

jbiskie · 2024-12-19T15:52:52Z

Closing this PR in favor of #1664 due to unsigned commits in the branch.

jbiskie and others added 9 commits December 5, 2024 15:01

WIP - remove cx.5 patient identifier warnings

19227c0

Move PID-3.5 removal to HapiHelper

6d45952

Change parameter from bundle to patient indentifer

3caa95d

Add tests for PID-3.5 removal

f2847d8

Move creation of OBR-16 extension into HapiHelper method, method para…

cc3aa10

…meter refactor, fix logic for existence of practitioner

Unit Test for removal of PID-5.7

fda6397

Added method for full removal of PID5.7 and name use

c8f5801

Co-Authored-By: Joel Biskie <84460447+jbiskie@users.noreply.github.com> Co-Authored-By: Jeremy Rosenfeld <10262289+JeremyIR@users.noreply.github.com>

Remove old unit test, update input file for PID-5.7 test

717e1e1

Co-authored-by: tjohnson@flexion.us Co-authored-by: jrosenfeld@flexion.us

Merge branch 'main' into bug/940-eliminate-warnings-for-missing-fields

b6748a0

github-actions bot reviewed Dec 12, 2024

View reviewed changes

...iary/etor/ruleengine/transformation/custom/CopyOrcOrderProviderToObrOrderProviderTest.groovy Outdated Show resolved Hide resolved

shared/src/main/java/gov/hhs/cdc/trustedintermediary/external/hapi/HapiHelper.java Outdated Show resolved Hide resolved

jbiskie added 3 commits December 12, 2024 15:22

Improve readability and null checks in removePID3_5Value and add null…

8faf0f4

… check in getObr16ExtensionPractitioner

Fix possible null value for getPIDPatient

f074b67

Co-authored-by: tjohnson7021 Co-authored-by: jeremyir

Add unit tests for removePID5_7Value Co-authored-by: jeremyir

279d566

basiliskus reviewed Dec 13, 2024

View reviewed changes

...termediary/etor/ruleengine/transformation/custom/CopyOrcOrderProviderToObrOrderProvider.java Show resolved Hide resolved

JeremyIR requested a review from basiliskus December 13, 2024 20:55

jbiskie marked this pull request as ready for review December 13, 2024 21:18

basiliskus reviewed Dec 13, 2024

View reviewed changes

jbiskie requested a review from basiliskus December 18, 2024 16:29

basiliskus approved these changes Dec 18, 2024

View reviewed changes

Jeremy Rosenfeld and others added 7 commits December 18, 2024 10:11

add additional test coverage for removePID3_5Value method

f37495e

Using VS Code TOC generation to generate a table of contents for the …

a37b7be

…scriptes/README.md

Update env template and readme for TI scripts

d9712ee

Update setup rs script to use correct Flexion Org RS SFTP transport

fb08b0b

- This is based on if you're running TI in docker vs gradle

Update the README for scripts

982a50b

Updates for the scripts/README.md

48248c6

Update setup script to remove code duplication and update variables only

4ad09d2

saquino0827 and others added 11 commits December 18, 2024 10:11

Last attempt to automate rs configuration depending on environment setup

aafe5eb

Update to the script setup

8a85d61

- template update to rename variables - README update to reference where local set up of RS and TI are - update local variables to be lower case

Fix linting error

5174441

Remove obvious comments; clean up test

d6bbf8f

Refactor the clearing of patient identifier assigner into setPID3_4Va…

0710b70

…lue, refactor PID-3.4 testing into discrete unit tests

Refactor getObr16Extension to HapiHelper, add unit tests

f2c0f59

Update Terraform azurerm to v4.14.0 (#1659)

6be3152

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Refactor getObr16ExtensionPractitioner into HapiHelper, add unit tests

e626b0e

Refactor getOrc12ExtensionPractitioner to HapiHelper, add unit tests

cc2b9d6

jorg3lopez force-pushed the bug/940-eliminate-warnings-for-missing-fields branch from c996bb6 to cc2b9d6 Compare December 18, 2024 18:14

jorg3lopez had a problem deploying to pr December 18, 2024 18:16 — with GitHub Actions Failure

jbiskie mentioned this pull request Dec 18, 2024

Eliminate warnings for missing fields (fix unsigned commits) #1664

Merged

1 task

jbiskie closed this Dec 19, 2024

jbiskie deleted the bug/940-eliminate-warnings-for-missing-fields branch December 20, 2024 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eliminate warnings for missing fields #1656

Eliminate warnings for missing fields #1656

jbiskie commented Dec 12, 2024 •

edited

Loading

github-actions bot commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

basiliskus Dec 13, 2024 •

edited

Loading

jbiskie Dec 16, 2024

basiliskus Dec 13, 2024

jbiskie Dec 16, 2024

basiliskus Dec 13, 2024

jbiskie Dec 18, 2024 •

edited

Loading

basiliskus left a comment

sonarqubecloud bot commented Dec 18, 2024

jbiskie commented Dec 19, 2024

Eliminate warnings for missing fields #1656

Eliminate warnings for missing fields #1656

Conversation

jbiskie commented Dec 12, 2024 • edited Loading

Description

Issue

Testing instructions

Checklist

github-actions bot commented Dec 12, 2024

PR Reviewer Guide 🔍

github-actions bot commented Dec 12, 2024

PR Code Suggestions ✨

basiliskus Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

jbiskie Dec 16, 2024

Choose a reason for hiding this comment

basiliskus Dec 13, 2024

Choose a reason for hiding this comment

jbiskie Dec 16, 2024

Choose a reason for hiding this comment

basiliskus Dec 13, 2024

Choose a reason for hiding this comment

jbiskie Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

basiliskus left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 18, 2024

Quality Gate passed

jbiskie commented Dec 19, 2024

jbiskie commented Dec 12, 2024 •

edited

Loading

basiliskus Dec 13, 2024 •

edited

Loading

jbiskie Dec 18, 2024 •

edited

Loading