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

added rs e2e test files and assertions for truncation logic #1487

Merged

Conversation

GilmoreA6
Copy link
Contributor

Add a PR title

Added two new files and relevant assertions to test leading zero truncation logic in RS e2e tests.

Issue

#1454

Checklist

  • I have added tests to cover my changes
  • I have added logging where useful (with appropriate log level)
  • I have added JavaDocs where required
  • I have updated the documentation accordingly

Note: You may remove items that are not applicable

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1454 - Partially compliant

Fully compliant requirements:

  • OBX-5 fields with NM data types will have leading zeroes truncated
  • Assertions added to RS e2e tests that check leading zeroes are truncated appropriately

Not compliant requirements:

  • A sample ORU message sent by ReportStream staging to the UCSD receiver is filed successfully by their EHR test environment with all OBX-5 values preserved with the exception of leading zeros.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible bug
Correct the expected value in the assertion rule to match the actual HL7 message content

Ensure that the rule for 'OBX-5' value in the 'CDPH leading zeros' assertion matches
the actual value in the HL7 message. The current rule expects '5.100' but the HL7
message contains '05.100'.

rs-e2e/src/main/resources/assertion_definitions.json [46]

-"OBX-5 = '5.100'"
+"OBX-5 = '05.100'"
Suggestion importance[1-10]: 10

Why: The suggestion correctly identifies a mismatch between the expected value in the assertion rule and the actual value in the HL7 message, which is crucial for accurate data validation. Fixing this ensures the assertions will correctly validate the data as intended.

10

@GilmoreA6 GilmoreA6 marked this pull request as ready for review October 28, 2024 16:59
Copy link

Copy link
Member

@halprin halprin left a comment

Choose a reason for hiding this comment

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

Looks kick-ass to me. @basiliskus, mind taking a look?

Copy link
Contributor

@basiliskus basiliskus left a comment

Choose a reason for hiding this comment

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

Looks good! It's a different approach in terms of setting the conditions to test but it completely makes sense to me for this kind of use case

@GilmoreA6
Copy link
Contributor Author

Looks good! It's a different approach in terms of setting the conditions to test but it completely makes sense to me for this kind of use case

@basiliskus ya, I considered adding a new rule type that would check a regex and see if it matched and then we could apply it broadly but decided that could be a future enhancement and it made more sense to have specific test cases for right now

@GilmoreA6 GilmoreA6 merged commit e0042dd into main Oct 28, 2024
17 checks passed
@GilmoreA6 GilmoreA6 deleted the 1452-add-rse2e-tests-and-assertions-for-truncation-logic branch October 28, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants