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.

Sign up for GitHub

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

Jump to bottom

RESTWS-707: Fix bug in encounterRole endpoint #332

Merged
merged 1 commit into from
May 11, 2018
Merged

RESTWS-707: Fix bug in encounterRole endpoint #332

merged 1 commit into from
May 11, 2018

Conversation

larrystone
Copy link
Contributor

Fix bug with encounterRole endpoint causing empty set [ ] to be returned when a query paramter in provider in API call
e.g v1/encounterRole?q=unknown

Issue: RESTWS-707

@dkayiwa
Copy link
Member

dkayiwa commented May 10, 2018

@larrystone did you take a look at the ticket description? It has the exact classes and changes that you need.

@coveralls
Copy link

coveralls commented May 10, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.01%) to 41.649% when pulling 535e5dc on larrystone:RESTWS-707 into 5d656d7 on openmrs:master.

dkayiwa
dkayiwa reviewed May 10, 2018
View reviewed changes
omod-1.11/src/test/java/org/openmrs/module/webservices/rest/web/RestTestConstants1_11.java
@@ -12,4 +12,6 @@
public class RestTestConstants1_11 {

public static final String DRUG_UUID = "05ec820a-d297-44e3-be6e-698531d9dd3f";

Copy link
Member

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's been used in the test files

Copy link
Member

Choose a reason for hiding this comment

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

Can you point me to the exact line?

dkayiwa
dkayiwa reviewed May 10, 2018
View reviewed changes
...mrs/module/webservices/rest/web/v1_0/controller/openmrs1_11/EncounterRoleController1_11.java Outdated
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.web.bind.annotation.RequestMethod;

public class EncounterRoleController1_11 extends MainResourceControllerTest {
Copy link
Member

Choose a reason for hiding this comment

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

Can you update this class name to be like the one i put in the ticket description?

@larrystone larrystone force-pushed the RESTWS-707 branch 2 times, most recently from 36fe583 to 41b70a6 Compare May 10, 2018 19:41
dkayiwa
dkayiwa reviewed May 10, 2018
View reviewed changes
omod-1.9/src/test/java/org/openmrs/module/webservices/rest/web/RestTestConstants1_9.java Outdated
@@ -51,8 +51,6 @@

public static final String CONCEPT_UUID = "c607c80f-1ea9-4da3-bb88-6276ce8868dd";

public final static String ENCOUNTER_ROLE_UUID = "a0b03050-c99b-11e0-9572-0800200c9a66";
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was being duplicated, I already have it in the RestTestConstans1_11.java file

@dkayiwa
Copy link
Member

dkayiwa commented May 10, 2018

Did you take a look at the travis failure?

@larrystone
Copy link
Contributor Author

I am looking into it.I have a feeling it's because I am on a slow network. I will try using a LAN network tomorrow. I also have some files that were autogenerated when I ran mvn clean install. is it okay to add them to my git files?
screen shot 2018-05-10 at 8 58 58 pm

@dkayiwa
Copy link
Member

dkayiwa commented May 10, 2018

Just leave the auto modified files.
As for the travis failure, the log should give you a clue in regards to what you have not done correctly.

@larrystone
Copy link
Contributor Author

I will look into it. Thanks @dkayiwa

@larrystone
Copy link
Contributor Author

larrystone commented May 10, 2018 •
edited
Loading

screen shot 2018-05-10 at 10 39 38 pm

This is the error I am getting now yet all tests are passing locally

screen shot 2018-05-10 at 10 53 37 pm

@larrystone larrystone changed the title RESRWS-707: Fix bug in encounterRole endpoint RESTWS-707: Fix bug in encounterRole endpoint May 10, 2018
@larrystone larrystone force-pushed the RESTWS-707 branch 4 times, most recently from 8667f4e to a3006a2 Compare May 11, 2018 00:21
@larrystone
Copy link
Contributor Author

larrystone commented May 11, 2018 •
edited
Loading

@dkayiwa The tests are now passing but I noticed coverage decreased even though the file I created had 100% coverage. Please is there anything I should do about this?
screen shot 2018-05-11 at 1 41 13 am

@larrystone larrystone force-pushed the RESTWS-707 branch 2 times, most recently from 8c2746a to 60b22c7 Compare May 11, 2018 06:33
@dkayiwa dkayiwa merged commit d76b294 into openmrs:master May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkayiwa dkayiwa dkayiwa left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@larrystone @dkayiwa @coveralls