Skip to content
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

Update OML mappings #15910

Merged
merged 16 commits into from
Oct 8, 2024
Merged

Conversation

JFisk42
Copy link
Collaborator

@JFisk42 JFisk42 commented Sep 17, 2024

This PR updates the OML mappings to use the new catchall mappings.

Test Steps:

  1. OML Related tests should pass:
    1. ./gradlew :prime-router:testIntegration --tests "gov.cdc.prime.router.datatests.mappinginventory.omlo21.OMLO21Full" -Pforcetest
    2. ./gradlew :prime-router:testIntegration --tests "gov.cdc.prime.router.datatests.TranslationTests" -Pforcetest
  2. All other tests should pass as well.

Changes

  • OML FHIR -> HL7 mappings are no longer dependent upon deprecated mapping files (hl7_mapping/common; hl7_mapping/v251; etc.)
  • Unused FHIR -> HL7 mappings have been removed
  • Created end-to-end tests for OML HL7 -> FHIR -> HL7 mappings.
    • OML HL7 -> FHIR catchall mappings were updated to better support a maximally configured OML message.
      • ServiceRequest.specimen and ServiceRequest.supportingInfo were added to support this. Specimen.collection and Patient.relationship were updated as well.
        • The vast majority of test updates on this PR are related to these catchall updates.
      • OML support for the CE to CWE casting was added.
  • Updated the OML data test files from translation-test-config.csv to reflect updates in the mappings
    • The .fhir file in the FHIR -> HL7 scenario will need to be verified and communicated with ETOR to ensure no breaking changes have been made. The significant differences here reflect the new and different fields being mapped in the OML HL7 -> FHIR conversion. #TODO

Checklist

Testing

  • Tested locally?
  • Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?
  • (For Changes to /frontend-react/...) Ran npm run lint:write?
  • Added tests?

Process

  • Are there licensing issues with any new dependencies introduced?
  • Includes a summary of what a code reviewer should test/verify?
  • Updated the release notes?
  • Database changes are submitted as a separate PR?
  • DevOps team has been notified if PR requires ops support?

To Be Done

Follow up issues still need to be logged:

Copy link

github-actions bot commented Sep 17, 2024

Dependency Review

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

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

Copy link

github-actions bot commented Sep 18, 2024

Test Results

1 232 tests  ±0   1 228 ✅ ±0   7m 34s ⏱️ -32s
  162 suites ±0       4 💤 ±0 
  162 files   ±0       0 ❌ ±0 

Results for commit fa0014e. ± Comparison against base commit 1bf38e8.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Sep 18, 2024

Integration Test Results

 54 files  +1   54 suites  +1   29m 33s ⏱️ + 1m 31s
413 tests +3  404 ✅ +3  9 💤 ±0  0 ❌ ±0 
416 runs  +3  407 ✅ +3  9 💤 ±0  0 ❌ ±0 

Results for commit fa0014e. ± Comparison against base commit 1bf38e8.

This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both.
gov.cdc.prime.router.datatests.mappinginventory.orcobr.ORCOBRToServiceRequestDiagnosticReportTests ‑ test correctly handles the effective when OBR8 is not populated()
gov.cdc.prime.router.datatests.mappinginventory.omlo21.OMLO21Full ‑ test OML_O21 all segments()
gov.cdc.prime.router.datatests.mappinginventory.orcobr.ORCOBRToServiceRequestDiagnosticReportTests ‑ test correctly handles the effectiveDateTime when OBR8 is not populated()
gov.cdc.prime.router.datatests.mappinginventory.orcobr.ORCOBRToServiceRequestDiagnosticReportTests ‑ test correctly handles the effectivePeriod when OBR8 is populated()
gov.cdc.prime.router.datatests.mappinginventory.orcobr.ORCOBRToServiceRequestDiagnosticReportTests ‑ test populates OML specific fields for OML messages()

♻️ This comment has been updated with latest results.

Copy link

sonarcloud bot commented Sep 25, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
40.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@JFisk42 JFisk42 changed the base branch from master to OML-Updates-Base September 27, 2024 01:29
@JFisk42 JFisk42 marked this pull request as ready for review September 27, 2024 01:31
@JFisk42 JFisk42 requested a review from a team as a code owner September 27, 2024 01:31
@@ -109,23 +107,16 @@
}
],
"name" : "NATUS",
"endpoint" : "urn:dns:natus.health.state.mn.us",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#TODO: I arrived at the contents of this file by using the fhirdata cli tool to generate the fhir from the expected hl7. I'm not sure how important some of these removed fields are to ETOR but I imagine they would be delighted if those one day showed up in the HL7 (and thus good to preserve in this file). When logging the ticket for the OML second pass I will also go through and see if any of these are low hanging fields to add that are mapped in the mapping inventory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple points here: Logged a ticket for the follow up OML items. Posted in the ETOR channel to alert them to the changes coming down the pipe. Also remembered that a number of fields are enriched from ETOR-TI itself.

@mkalish mkalish self-assigned this Sep 27, 2024
@JFisk42
Copy link
Collaborator Author

JFisk42 commented Sep 27, 2024

For some reason after pushing the last commit to fix the build, GitHub decided the backend didn't need to be built :/

Copy link
Collaborator Author

@JFisk42 JFisk42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to post these comments on Friday.

Copy link
Collaborator

@mkalish mkalish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic stuff! Only comment would be a few new test cases or test enhancements

Copy link
Collaborator

@arnejduranovic arnejduranovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good and new tickets appear ready for refinement, great work!

@JFisk42 JFisk42 merged commit 9ae7f66 into OML-Updates-Base Oct 8, 2024
8 checks passed
@JFisk42 JFisk42 deleted the platform/josh/14992-update-OML-mappings branch October 8, 2024 00:54
JFisk42 added a commit that referenced this pull request Oct 25, 2024
* Update OML FHIR -> HL7 Mappings

* Update OML HL7 -> FHIR mappings

* Add OML mapping integration tests

* Removing unused FHIR -> HL7 mappings

* Adding OML to CWE/CE override

* Adjusting existing tests

* Adding tests for new misc mappings

* Update existing OML integration tests

* Fixing integration tests after rebase

* expected oru changes

* Specimen.collection conditional update

* Updating formatting for cleaner diff

* Missed e2e test updates

* PRT <-> Device mappings will be taken care of in the next PR

* Changed line endings from CRLF to LF

* Removing OML use of HL7v2Name url
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform Platform Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants