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-955: Add support for person attribute of type Location #620

Merged
merged 2 commits into from
Sep 18, 2024
Merged

RESTWS-955: Add support for person attribute of type Location #620

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ade1123
RESTWS-955: Add support for person attribute of type Location
mherman22 Sep 18, 2024
e17b743
fix according to reviews
mherman22 Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions ...nmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import io.swagger.models.properties.RefProperty;
import io.swagger.models.properties.StringProperty;
import org.openmrs.Attributable;
import org.openmrs.Concept;
import org.openmrs.Location;
import org.openmrs.Person;
import org.openmrs.PersonAttribute;
import org.openmrs.PersonAttributeType;
Expand All @@ -37,6 +37,7 @@
import org.openmrs.module.webservices.rest.web.response.ConversionException;
import org.openmrs.module.webservices.rest.web.response.ResponseException;
import org.openmrs.util.OpenmrsClassLoader;
import java.util.UUID;

/**
* {@link Resource} for PersonAttributes, supporting standard CRUD operations
Expand Down Expand Up @@ -96,9 +97,13 @@ public void setHydratedObject(PersonAttribute personAttribute, String attributab
throw new APIException("Could not convert value to Attributable", e);
}
}

@PropertySetter("value")
public void setValue(PersonAttribute personAttribute, String value) {
if (setLocationIfUUID(personAttribute, value)) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd probably just inline setLocationIfUUID() here. I don't see a lot of value in having a separate method.


PersonAttributeType attributeType = personAttribute.getAttributeType();
if (attributeType == null) {
personAttribute.setValue(value);
Expand Down Expand Up @@ -134,6 +139,26 @@ public void setValue(PersonAttribute personAttribute, String value) {
}
}
}

private boolean setLocationIfUUID(PersonAttribute personAttribute, String value) {
if (isValidUUID(value)) {
Location location = Context.getLocationService().getLocationByUuid(value);
if (location != null) {
personAttribute.setValue(location.getUuid());
return true;
}
}
return false;
}

private boolean isValidUUID(String value) {
Copy link
Member

Choose a reason for hiding this comment

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

Please use RestUtil.isValidUuid() for this.

try {
UUID.fromString(value);
return true;
} catch (IllegalArgumentException e) {
return false;
}
}

/**
* @see org.openmrs.module.webservices.rest.web.resource.impl.BaseDelegatingResource#getUpdatableProperties()
Expand Down Expand Up @@ -297,10 +322,10 @@ public String getDisplayString(PersonAttribute pa) {
String value = pa.toString();
return value == null ? "" : value;
}

/**
* Gets the hydrated object of person attribute.
*
*
* @param pa the person attribute.
* @return an object containing the hydrated object.
*/
Expand All @@ -310,8 +335,7 @@ public Object getValue(PersonAttribute pa) {
if (value == null) {
return null;
}

return ConversionUtil.convertToRepresentation(value, Representation.REF);
}

}
19 changes: 13 additions & 6 deletions .../module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@
import static org.hamcrest.Matchers.equalToIgnoringCase;

public class PersonAttributeResource1_8Test extends BaseModuleWebContextSensitiveTest {

public static final String PERSON_ATTRIBUTE_JSON = "{" + " \"value\": \"Bangalore\"," + " \"attributeType\": {"
+ " \"uuid\": \"54fc8400-1683-4d71-a1ac-98d40836ff7c\"" + " }" + "}";


public static final String PERSON_ATTRIBUTE_JSON = "{\"value\": \"Bangalore\", \"uuid\": \"592349ed-d012-4552-a274-d5d8e73b9401\", \"attributeType\": { \"uuid\": \"54fc8400-1683-4d71-a1ac-98d40836ff7c\" }}";

public static final String PERSON_ATTRIBUTE_JSON_WITHOUT_UUID = "{\"value\": \"Bangalore\", \"uuid\": \"\", \"attributeType\": { \"uuid\": \"54fc8400-1683-4d71-a1ac-98d40836ff7c\" }}";

private SimpleObject personAttributeSimpleObject = new SimpleObject();

private PersonAttributeResource1_8 resource;
Expand Down Expand Up @@ -65,6 +66,14 @@ public void shouldCreatePersonAttribute() throws Exception {
Assert.assertEquals("Bangalore", created.get("value"));
}

@Test
public void getValue_shouldFallBackToHydratedObjectWhenUuidIsEmpty() throws Exception {
personAttributeSimpleObject.putAll(new ObjectMapper().readValue(PERSON_ATTRIBUTE_JSON_WITHOUT_UUID, HashMap.class));
SimpleObject created = (SimpleObject) resource.create("da7f524f-27ce-4bb2-86d6-6d1d05312bd5",
personAttributeSimpleObject, new RequestContext());
Assert.assertEquals("Bangalore", created.get("value"));
}

@Test
public void getDisplayString_shouldGetDisplayStringForConcept() {
// arrange
Expand Down Expand Up @@ -138,11 +147,9 @@ public void setValue_shouldSetProperAttributableIdIfFound() {

PersonAttribute attribute = new PersonAttribute(type, null);
attribute.setAttributeType(type);

Assert.assertNull(attribute.getValue());

resource.setValue(attribute, location.getUuid());

Assert.assertEquals(location.getUuid(), attribute.getValue());
}

Expand Down
Loading