Skip to content

Conversation

@sagarkhandagre998
Copy link

@sagarkhandagre998 sagarkhandagre998 commented Dec 14, 2025

Description of what I changed -

This PR addresses an inconsistency in the Encounter API where getOrders() and getDiagnoses() return both active and voided items by default, unlike newer Encounter properties such as Conditions and Allergies.

To improve API flexibility while preserving backward compatibility, this change introduces overloaded methods that allow callers to explicitly control whether voided items are included:

getOrders(boolean includeVoided)
getDiagnoses(boolean includeVoided)

The existing getOrders() and getDiagnoses() methods were left unchanged to avoid breaking legacy code that depends on direct access to the underlying Hibernate-managed collections.

This allows client code and downstream consumers (including REST) to retrieve only non-voided orders and diagnoses without duplicating filtering logic.

Issue I worked on
https://issues.openmrs.org/browse/TRUNK-6418

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.

  • I have added tests to cover my changes.

  • New unit tests verify that:

  •  getOrders(false) excludes voided orders
    
  •  getDiagnoses(false) excludes voided diagnoses
    
  • - Existing behavior of getOrders() and getDiagnoses() remains unchanged.

  • - I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.

  • - All new and existing tests passed.

  • - My pull request is based on the latest changes of the master branch.

…diagnoses

- Add getOrders(boolean includeVoided) method to filter voided orders
- Add getDiagnoses(boolean includeVoided) method to filter voided diagnoses
- Add comprehensive unit tests for both methods
- Add null safety handling for empty collections
- Follow existing OpenMRS coding patterns and conventions (Standard getter on Encounter returns voided Orders and Diagnoses)
}

if (includeVoided) {
return getDiagnoses();
Copy link
Contributor

Choose a reason for hiding this comment

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

Kindly using getDiagnoses() while the rest of the method accesses diagnoses directly wont' it create inconsistent behavior? It will be better to use a single source of truth (getDiagnoses() assigned once at the top. Kindly @wikumChamith correct me please!

Copy link
Author

Choose a reason for hiding this comment

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

@jwnasambu did u check the requirement of this pull request ?

@wikumChamith
Copy link
Member

Can we stick to our existing PR template?

@sagarkhandagre998
Copy link
Author

Can we stick to our existing PR template?

Sure @wikumChamith It's my bed.

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