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

RA-1641: inactive conditions should be displayed #176

Merged
merged 1 commit into from Dec 11, 2019

Conversation

mozzy11
Copy link
Member

@mozzy11 mozzy11 commented Dec 5, 2019

Ticket : https://issues.openmrs.org/browse/RA-1641

added a handler to return all Conditions

@mozzy11
Copy link
Member Author

mozzy11 commented Dec 6, 2019

cc @mseaton

Copy link
Member

@mogoodrich mogoodrich left a comment

Choose a reason for hiding this comment

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

Thanks @mozzy11 ! This looks good, but looking at it a little closer I'm wondering if instead of adding the getAllConditionHistory method we might just want to change the existing "getConditionHistory" to use the "getAllConditions" service method.

A "history" implies to me that it would get all conditions, not just the active conditions, and I'm guessing this endpoint is currently only being used in that one place in coreapps. What do you think?

@mozzy11
Copy link
Member Author

mozzy11 commented Dec 10, 2019

true @mogoodrich , let work on that

@mozzy11
Copy link
Member Author

mozzy11 commented Dec 11, 2019

Hi @mogoodrich , i have updated the PR as per your suggestion

Copy link
Member

@mogoodrich mogoodrich left a comment

Choose a reason for hiding this comment

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

LGTM!

@mogoodrich mogoodrich merged commit d957aee into openmrs:master Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants