Skip to content

TRUNK-6284: Add discontinue_reason and discontinue_reason_non_coded t…#5998

Open
0abhi007 wants to merge 2 commits intoopenmrs:masterfrom
0abhi007:TRUNK-6284-discontinue-reason
Open

TRUNK-6284: Add discontinue_reason and discontinue_reason_non_coded t…#5998
0abhi007 wants to merge 2 commits intoopenmrs:masterfrom
0abhi007:TRUNK-6284-discontinue-reason

Conversation

@0abhi007
Copy link
Copy Markdown

@0abhi007 0abhi007 commented Mar 30, 2026

Description of what I changed

JIRA: https://openmrs.atlassian.net/browse/TRUNK-6284?focusedCommentId=193037

Added discontinue_reason and discontinueReasonNonCoded fields to the Order domain object.

  • Added discontinueReason (Concept) field to Order.java
  • Added discontinueReasonNonCoded (String) field to Order.java
  • Added getters, setters, and updated copyHelper method
  • Added Liquibase migration to add the new columns to the orders table

Issue I worked on

TRUNK-6284

@0abhi007
Copy link
Copy Markdown
Author

Hi @dkayiwa @ibacher @mseaton @wikumChamith - could you please review this PR when you get a chance?

This implements TRUNK-6284 - adds structured discontinuation reason support to the Order domain.

Thank you!

@vaalemax
Copy link
Copy Markdown

The implementation looks clean.
One thing to verify: does the copyHelper method handle the case where discontinueReason is null, to avoid NullPointerExceptions when copying Orders that were discontinued without a structured reason?
Also, it would be helpful to have at least one unit test covering the new fields in the Order domain.

Copy link
Copy Markdown
Contributor

@jwnasambu jwnasambu left a comment

Choose a reason for hiding this comment

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

Kindly attach the correct ticket link on this PR please!

@0abhi007
Copy link
Copy Markdown
Author

The implementation looks clean. One thing to verify: does the copyHelper method handle the case where discontinueReason is null, to avoid NullPointerExceptions when copying Orders that were discontinued without a structured reason? Also, it would be helpful to have at least one unit test covering the new fields in the Order domain.

The implementation looks clean. One thing to verify: does the copyHelper method handle the case where discontinueReason is null, to avoid NullPointerExceptions when copying Orders that were discontinued without a structured reason? Also, it would be helpful to have at least one unit test covering the new fields in the Order domain.

Thanks a lot for reviewing this, @vaalemax.

I will double‑check copyHelper to ensure it safely handles a null discontinueReason to avoid any potential NPEs when copying discontinued Orders without a structured reason.

I will also add unit tests covering the new discontinueReason and discontinueReasonNonCoded fields in the Order domain and update the PR accordingly.

@0abhi007
Copy link
Copy Markdown
Author

Kindly attach the correct ticket link on this PR please!

Thanks for the review, @jwnasambu. I have attached the correct ticket link here:
https://openmrs.atlassian.net/browse/TRUNK-6284?focusedCommentId=193037

@0abhi007 0abhi007 requested a review from jwnasambu March 30, 2026 22:02
@sonarqubecloud
Copy link
Copy Markdown

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