RESTWS-940: PersonAttributeResource should not try to hydrate the attribute itself

omod-1.8/src/main/java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8.java

@@ -65,7 +65,7 @@ public DelegatingResourceDescription getRepresentationDescription(Representation
 			description.addProperty("display");
 			description.addProperty("uuid");
 			description.addProperty("value");
-			description.addProperty("attributeType", Representation.REF);
+			description.addProperty("attributeType", Representation.FULL);
 			description.addProperty("voided");
 			description.addProperty("auditInfo");
 			description.addProperty("hydratedObject");
@@ -284,17 +284,18 @@ public void purge(PersonAttribute delegate, RequestContext context) throws Respo
 	 * Gets the display string for a person attribute.
 	 * 
 	 * @param pa the person attribute.
-	 * @return attribute type + value (for concise display purposes)
+	 * @return value (for concise display purposes)
 	 */
 	@PropertyGetter("display")
 	public String getDisplayString(PersonAttribute pa) {
-		if (pa.getAttributeType() == null)
-			return "";
-		if (Concept.class.getName().equals(pa.getAttributeType().getFormat()) && pa.getValue() != null) {
-			Concept concept = Context.getConceptService().getConcept(pa.getValue());
-			return concept == null ? null : concept.getDisplayString();
+		// a PersonAttribute without a type cannot really be converted
+		if (pa.getAttributeType() == null) {
+			return pa.getValue() != null ? pa.getValue() : "";
 		}
-		return pa.getAttributeType().getName() + " = " + pa.getValue();
+
+		// PersonAttribute#toString() calls PersonAttribute#hydrateObject() and Attributable#getDisplayString()
+		String value = pa.toString();
+		return value == null ? "" : value;
 	}
 
 	/**

omod-1.8/src/test/java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8Test.java

@@ -14,9 +14,11 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.openmrs.Attributable;
+import org.openmrs.Concept;
 import org.openmrs.Location;
 import org.openmrs.PersonAttribute;
 import org.openmrs.PersonAttributeType;
+import org.openmrs.api.ConceptService;
 import org.openmrs.api.LocationService;
 import org.openmrs.api.PersonService;
 import org.openmrs.api.context.Context;
@@ -28,6 +30,9 @@
 
 import java.util.HashMap;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalToIgnoringCase;
+
 public class PersonAttributeResource1_8Test extends BaseModuleWebContextSensitiveTest {
 
 	public static final String PERSON_ATTRIBUTE_JSON = "{" + "    \"value\": \"Bangalore\"," + "    \"attributeType\": {"
@@ -36,6 +41,9 @@ public class PersonAttributeResource1_8Test extends BaseModuleWebContextSensitiv
 	private SimpleObject personAttributeSimpleObject = new SimpleObject();
 
 	private PersonAttributeResource1_8 resource;
+
+	@Autowired
+	private ConceptService conceptService;
 
 	@Autowired
 	private PersonService personService;
@@ -56,6 +64,64 @@ public void shouldCreatePersonAttribute() throws Exception {
 		    personAttributeSimpleObject, new RequestContext());
 		Assert.assertEquals("Bangalore", created.get("value"));
 	}
+
+	@Test
+	public void getDisplayString_shouldGetDisplayStringForConcept() {
+		// arrange
+		PersonAttributeType type = new PersonAttributeType();
+		type.setFormat("org.openmrs.Concept");
+		type.setName("Some Concept");
+		type.setDescription("Some Attribute Type Description");
+		type.setSearchable(false);
+		type = personService.savePersonAttributeType(type);
+
+		Concept trueConcept = conceptService.getTrueConcept();
+
+		PersonAttribute attribute = new PersonAttribute(type, String.valueOf(trueConcept.getId()));
+
+		// act
+		String displayString = resource.getDisplayString(attribute);
+
+		assertThat(displayString, equalToIgnoringCase("Yes"));
+	}
+
+	@Test
+	public void getDisplayString_shouldGetDisplayStringForLocation() {
+		// arrange
+		PersonAttributeType type = new PersonAttributeType();
+		type.setFormat("org.openmrs.Location");
+		type.setName("Some Location");
+		type.setDescription("Some Attribute Type Description");
+		type.setSearchable(false);
+		type = personService.savePersonAttributeType(type);
+
+		Location location = locationService.getLocation(1);
+
+		PersonAttribute attribute = new PersonAttribute(type, String.valueOf(location.getId()));
+
+		// act
+		String displayString = resource.getDisplayString(attribute);
+
+		assertThat(displayString, equalToIgnoringCase("Unknown Location"));
+	}
+
+	@Test
+	public void getDisplayString_shouldGetDisplayStringForString() {
+		// arrange
+		PersonAttributeType type = new PersonAttributeType();
+		type.setFormat("java.lang.String");
+		type.setName("Some String");
+		type.setDescription("Some Attribute Type Description");
+		type.setSearchable(false);
+		type = personService.savePersonAttributeType(type);
+
+		PersonAttribute attribute = new PersonAttribute(type, "A Value");
+
+		// act
+		String displayString = resource.getDisplayString(attribute);
+
+		assertThat(displayString, equalToIgnoringCase("A Value"));
+	}
 
 	@Test
 	public void setValue_shouldSetProperAttributableIdIfFound() {

omod-1.9/src/test/java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_9/CreateUpdatePatientResource1_9Test.java

@@ -70,7 +70,7 @@ public void shouldUpdatePatient() throws Exception {
 		Map preferredAddress = (Map) person.get("preferredAddress");
 		Assert.assertEquals("address 1", preferredAddress.get("display"));
 		List<Map> attributes = (List<Map>) person.get("attributes");
-		Assert.assertEquals("Race = Muslim", attributes.get(0).get("display"));
+		Assert.assertEquals("Muslim", attributes.get(0).get("display"));
 	}
 
 }