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

LUI-128 Add new Provider Attribute Type #91

Merged
merged 1 commit into from
Jan 11, 2018
Merged

LUI-128 Add new Provider Attribute Type #91

merged 1 commit into from
Jan 11, 2018

Conversation

BartlomiejRasztabiga
Copy link
Member

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 25.298% when pulling bf7c286 on BartlomiejRasztabiga:LUI-128 into 03eb6e3 on openmrs:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 25.286% when pulling 896b134 on BartlomiejRasztabiga:LUI-128 into 03eb6e3 on openmrs:master.

return datatype.deserialize(formFieldValue);
}

private boolean isInteger(String value) {
return value.matches("-?\\d+");
Copy link
Member

Choose a reason for hiding this comment

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

There should be a java library for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

StringUtils.isNumeric() would be ok?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 25.281% when pulling b9093aa on BartlomiejRasztabiga:LUI-128 into 03eb6e3 on openmrs:master.

if (StringUtils.isNumeric(formFieldValue)) {
BaseOpenmrsObject metadata = null;
PropertyEditor editor;

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 add some unit tests for these?

Copy link
Member Author

Choose a reason for hiding this comment

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

@dkayiwa Done :) I've managed to get 100% coverage

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 25.328% when pulling 1e4d606 on BartlomiejRasztabiga:LUI-128 into 03eb6e3 on openmrs:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 25.416% when pulling 04c7462 on BartlomiejRasztabiga:LUI-128 into 03eb6e3 on openmrs:master.

@Test
public void getValue_givenLocationId_shouldCallDeserializeWithLocationUuid() throws Exception {
List<Location> locations = Context.getLocationService().getAllLocations();
Location testedLocation = locations.get(0);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of getting all and then the first, why not just get for a particular id? You can pick them from here: https://github.com/openmrs/openmrs-core/blob/master/api/src/test/resources/org/openmrs/include/standardTestDataset.xml

Copy link
Member Author

Choose a reason for hiding this comment

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

@dkayiwa Done :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 25.416% when pulling b8ad8bd on BartlomiejRasztabiga:LUI-128 into 03eb6e3 on openmrs:master.

@dkayiwa dkayiwa merged commit ed5bcda into openmrs:master Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants