-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
TRUNK-5007: Enable the ExistingOrNewVisitAssignmentHandler to handle wild cards for encounter type to visit type. #4603
base: master
Are you sure you want to change the base?
Conversation
2f0c3cf
to
f2c409d
Compare
When ready for review, remember to always hit the Request Code Review button as advised at https://wiki.openmrs.org/display/docs/Pull+Request+Tips |
|
||
String[] mappings = value.split(","); | ||
for (String mapping : mappings) { | ||
int index = mapping.indexOf(':'); | ||
if (index > 0) { | ||
String encounterTypeIdOrUuid = mapping.substring(0, index).trim(); | ||
if (targetEncounterTypeId.equals(encounterTypeIdOrUuid) | ||
|| encounterType.getUuid().equals(encounterTypeIdOrUuid)) { | ||
if ("default".equals(encounterTypeIdOrUuid) || targetEncounterTypeId.equals(encounterTypeIdOrUuid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe what you are trying to achieve with default
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I understood from the ticket TRUNK-5007 and ticket EA-116 is that the default
wildcard should be used if no specific encounter type is matched and beforeCreateEncounter()
from ExistingOrNewVisitAssignmentHandler
should be able to handle the default
wildcard EncounterType
from the globalProperty
mappings value.
new ExistingOrNewVisitAssignmentHandler().beforeCreateEncounter(encounter); | ||
|
||
assertNotNull(encounter.getVisit()); | ||
assertEquals(Context.getVisitService().getVisitType(1), encounter.getVisit().getVisitType()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you fetch the visit type by uuid?
new ExistingOrNewVisitAssignmentHandler().beforeCreateEncounter(encounter); | ||
|
||
assertNotNull(encounter.getVisit()); | ||
assertEquals(Context.getVisitService().getVisitType(1), encounter.getVisit().getVisitType()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see any visitTypeId value of 1
in the above global property. Do you mind explaining what you are trying to test here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the VisitType
by uuid is the best way to test this because the test gp mappings are overridden by the VisitType
uuid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind explaining this a bit more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provided a gp with overridden mappings which also included the default
wildcard EncounterType
with the valid VisitType
uuid to test if beforeCreateEncounter()
can resolve the default
wildcard if it is provided in an overridden gp mapping.
…wild cards for encounter type to visit type.
f2c409d
to
86cd0c2
Compare
Issue I worked on
see https://issues.openmrs.org/browse/TRUNK-5007
Checklist: I completed these to help reviewers :)
My IDE is configured to follow the code style of this project.
No? Unsure? -> configure your IDE, format the code and add the changes with
git add . && git commit --amend
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)
No? -> write tests and add them to this commit
git add . && git commit --amend
I ran
mvn clean package
right before creating this pull request andadded all formatting changes to my commit.
No? -> execute above command
All new and existing tests passed.
No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.
No? Unsure? -> execute command
git pull --rebase upstream master