FM2-126: Fix handling of concepts in PersonAttributes (#567)

icrc-jofrancisco · Jose Francisco · parthfloyd · ibacher · commit 51b05f79466f · 2025-08-01T15:06:12.000-04:00
Co-authored-by: Jose Francisco &lt;josecarlos.francisco@merkle.com&gt;
Co-authored-by: Parth Panchal &lt;parth6606@gmail.com&gt;
diff --git a/api/src/main/java/org/openmrs/module/fhir2/api/translators/impl/PersonAttributeTranslatorImpl.java b/api/src/main/java/org/openmrs/module/fhir2/api/translators/impl/PersonAttributeTranslatorImpl.java
@@ -75,7 +75,7 @@ public class PersonAttributeTranslatorImpl implements PersonAttributeTranslator
 	
 	@Override
 	public Extension toFhirResource(@Nonnull PersonAttribute personAttribute) {
-		if (personAttribute == null || personAttribute.getAttributeType() == null) {
+		if (personAttribute == null || personAttribute.getVoided() || personAttribute.getAttributeType() == null) {
 			return null;
 		}
 		
@@ -204,7 +204,14 @@ protected CodeableConcept buildCodeableConcept(String conceptId) {
 			return null;
 		}
 		
-		Concept concept = conceptService.get(conceptId);
+		Concept concept;
+		try {
+			concept = conceptService.get(Integer.parseInt(conceptId));
+		}
+		catch (NumberFormatException e) {
+			return null;
+		}
+		
 		if (concept != null) {
 			return conceptTranslator.toFhirResource(concept);
 		}
diff --git a/api/src/test/java/org/openmrs/module/fhir2/api/translators/impl/PersonAttributeTranslatorImplTest.java b/api/src/test/java/org/openmrs/module/fhir2/api/translators/impl/PersonAttributeTranslatorImplTest.java
@@ -185,7 +185,7 @@ public void shouldTranslateConceptPersonAttributeToFhirExtension() {
 		
 		Concept concept = new Concept();
 		concept.setConceptId(Integer.parseInt(CONCEPT_ATTRIBUTE_VALUE));
-		when(conceptService.get(CONCEPT_ATTRIBUTE_VALUE)).thenReturn(concept);
+		when(conceptService.get(Integer.parseInt(CONCEPT_ATTRIBUTE_VALUE))).thenReturn(concept);
 		
 		CodeableConcept codeableConcept = new CodeableConcept();
 		codeableConcept.setText("ConceptText");
@@ -380,4 +380,17 @@ public void shouldTranslateCodeableConceptTypeExtensionToPersonAttribute() {
 		assertThat(result.getValue(), equalTo(CONCEPT_ATTRIBUTE_VALUE));
 	}
 	
+	@Test
+	public void shouldReturnNullWhenPersonAttributeIsVoided() {
+		personAttribute.setVoided(true);
+		Extension result = personAttributeTranslator.toFhirResource(personAttribute);
+		assertThat(result, equalTo(null));
+	}
+	
+	@Test
+	public void shouldReturnNullWhenConceptIdIsNotAnInteger() {
+		CodeableConcept result = personAttributeTranslator.buildCodeableConcept("notAnInt");
+		assertThat(result, equalTo(null));
+	}
+	
 }
diff --git a/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r3/PatientFhirResourceProviderIntegrationTest.java b/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r3/PatientFhirResourceProviderIntegrationTest.java
@@ -197,7 +197,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {
 		        .filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))
 		        .collect(Collectors.toList());
 		
-		assertThat(personAttributeExtensions, hasSize(3));
+		assertThat(personAttributeExtensions, hasSize(4));
 	}
 	
 	@Test
diff --git a/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r3/PersonFhirResourceProviderIntegrationTest.java b/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r3/PersonFhirResourceProviderIntegrationTest.java
@@ -148,7 +148,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {
 		        .filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))
 		        .collect(Collectors.toList());
 		
-		assertThat(personAttributeExtensions, hasSize(2));
+		assertThat(personAttributeExtensions, hasSize(4));
 	}
 	
 	@Test
diff --git a/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r4/PatientFhirResourceProviderIntegrationTest.java b/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r4/PatientFhirResourceProviderIntegrationTest.java
@@ -205,7 +205,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {
 		        .filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))
 		        .collect(Collectors.toList());
 		
-		assertThat(personAttributeExtensions, hasSize(3));
+		assertThat(personAttributeExtensions, hasSize(4));
 	}
 	
 	@Test
diff --git a/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r4/PersonFhirResourceProviderIntegrationTest.java b/integration-tests/src/test/java/org/openmrs/module/fhir2/providers/r4/PersonFhirResourceProviderIntegrationTest.java
@@ -154,7 +154,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {
 		        .filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))
 		        .collect(Collectors.toList());
 		
-		assertThat(personAttributeExtensions, hasSize(2));
+		assertThat(personAttributeExtensions, hasSize(4));
 	}
 	
 	@Test
diff --git a/test-data/src/main/resources/org/openmrs/module/fhir2/api/dao/impl/FhirPatientDaoImplTest_initial_data.xml b/test-data/src/main/resources/org/openmrs/module/fhir2/api/dao/impl/FhirPatientDaoImplTest_initial_data.xml
@@ -11,6 +11,8 @@
 <dataset>
     <global_property property="patient.identifierRegex" property_value="^0*@SEARCH@([A-Z]+-[0-9])?$" uuid="c92cce6d-7e7d-4c53-a3c9-faaac17f8f8b"/>
     <location location_id="1" name="Test Location" creator="1" date_created="2005-01-01 00:00:00.0" retired="false" uuid="92ab9667-4686-49af-8be8-65a4b58fc49c"/>
+    <concept concept_id="1" retired="false" class_id="1" is_set="false" creator="1" date_created="2005-01-01 00:00:00.0" version="" uuid="4bfea75e-2b8d-4048-9551-b363d72018b1"/>
+    <concept_name concept_id="1" name="Test Concept" locale="en_GB" creator="1" date_created="2005-01-01 00:00:00.0" concept_name_id="1" concept_name_type="FULLY_SPECIFIED" locale_preferred="1" voided="false" uuid="17b09f4e-37c6-46cd-8c3a-89214dd5fe25"/>
     <patient_identifier_type patient_identifier_type_id="1" name="Test Identifier Type" description="Test description" creator="1" date_created="2005-01-01 00:00:00.0" required="false" retired="false" uuid="c5576187-9a67-43a7-9b7c-04db22851211"/>
     <person person_id="2" gender="M" dead="false" creator="1" date_created="2005-01-01 00:00:00.0" voided="false" uuid="61b38324-e2fd-4feb-95b7-9e9a2a4400df"/>
     <person person_id="3" gender="M" dead="true" creator="1" date_created="2005-01-01 00:00:00.0" voided="false" uuid="5c521595-4e12-46b0-8248-b8f2d3697766"/>
@@ -51,7 +53,9 @@
     <person_attribute_type person_attribute_type_id="4" name="test string" format="java.lang.String" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="3b4a243d-4508-4ecf-aa05-d1260a78e196" sort_weight="2"/>
     <person_attribute_type person_attribute_type_id="5" name="test boolean" format="java.lang.Boolean" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="4973eeee-f193-4504-a67e-b8132a39c42a" sort_weight="2"/>
     <person_attribute_type person_attribute_type_id="6" name="test location" format="org.openmrs.Location" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="560683ca-1261-441d-89e2-d11faeacd8c9" sort_weight="2"/>
+    <person_attribute_type person_attribute_type_id="7" name="test concept" format="org.openmrs.Concept" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="9da03a0b-c208-44d3-acd9-425151c8f8e1" sort_weight="2"/>
     <person_attribute person_attribute_id="4" person_id="5" value="test@openmrs.org" person_attribute_type_id="4" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="fbd82a27-56ec-46b4-8660-6442e832dc3e"/>
     <person_attribute person_attribute_id="5" person_id="5" value="true" person_attribute_type_id="5" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="a55f316e-29dd-4ad5-b954-62ef93723658"/>
     <person_attribute person_attribute_id="6" person_id="5" value="1" person_attribute_type_id="6" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="ad0f530d-aebf-4284-b7c1-c103986b1c2b"/>
+    <person_attribute person_attribute_id="7" person_id="5" value="1" person_attribute_type_id="7" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="704f5723-ed90-4d03-ad69-5d626dfb18ee"/>
 </dataset>
diff --git a/test-data/src/main/resources/org/openmrs/module/fhir2/api/dao/impl/FhirPersonDaoImplTest_initial_data.xml b/test-data/src/main/resources/org/openmrs/module/fhir2/api/dao/impl/FhirPersonDaoImplTest_initial_data.xml
@@ -9,6 +9,8 @@
     graphic logo is a trademark of OpenMRS Inc.
 -->
 <dataset>
+    <concept concept_id="1" retired="false" class_id="1" is_set="false" creator="1" date_created="2005-01-01 00:00:00.0" version="" uuid="790e6c1e-1e89-43a7-92d7-4aa22b1f961a"/>
+    <concept_name concept_id="1" name="Test Concept" locale="en_GB" creator="1" date_created="2005-01-01 00:00:00.0" concept_name_id="1" concept_name_type="FULLY_SPECIFIED" locale_preferred="1" voided="false" uuid="80dc810e-68fe-4bef-80d3-786722c53766"/>
     <person person_id="2" gender="M" birthdate="1999-12-20" dead="false" creator="1" date_created="2005-01-01 00:00:00.0" voided="false"  uuid="61b38324-e2fd-4feb-95b7-9e9a2a4400df"/>
     <person person_id="3" gender="M" birthdate="2000-11-18" dead="false" creator="1" date_created="2005-01-01 00:00:00.0" voided="false" void_reason="there is no reason" uuid="5c521595-4e12-46b0-8248-b8f2d3697766"/>
     <person person_id="4" gender="M" birthdate="1993-04-02" dead="false" creator="1" date_created="2005-01-01 00:00:00.0" voided="false" uuid="256ccf6d-6b41-455c-9be2-51ff4386ae76"/>
@@ -22,7 +24,10 @@
     <person_address person_address_id="7" preferred="true" person_id="7" city_village="Peabody" state_province="MA" postal_code="01960" country="FakeAB" creator="1" date_created="2005-01-01 00:00:00.0" voided="false" uuid="da593572-67b7-11ea-bc55-0242ac130003"/>
     <person_attribute_type person_attribute_type_id="2" name="email" format="java.lang.String" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="5a8c170b-178a-4b8f-908e-279b67854d84" sort_weight="2"/>
     <person_attribute_type person_attribute_type_id="3" name="test patient" format="java.lang.Boolean" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="e34d1404-5633-4e5b-9db3-930f31086557" sort_weight="2"/>
+    <person_attribute_type person_attribute_type_id="8" name="test location" format="org.openmrs.Location" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="95a7a38f-9bea-4f7a-9428-775762ee13e4" sort_weight="2"/>
+    <person_attribute_type person_attribute_type_id="9" name="test concept" format="org.openmrs.Concept" searchable="false" creator="1" date_created="2008-08-15 15:53:36.0" retired="false" uuid="1a39a18b-184d-4a5c-9641-e1f7a4a717e5" sort_weight="2"/>
     <person_attribute person_attribute_id="2" person_id="3" value="test@openmrs.org" person_attribute_type_id="2" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="9cc62c8d-5817-4c21-8205-266c5ff344db"/>
     <person_attribute person_attribute_id="3" person_id="3" value="true" person_attribute_type_id="3" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="bcced38f-dad5-4c71-a436-8a486e4288d9"/>
-
+    <person_attribute person_attribute_id="8" person_id="3" value="1" person_attribute_type_id="8" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="4b2f7474-29e7-4205-ae48-0b5f67bf7895"/>
+    <person_attribute person_attribute_id="9" person_id="3" value="1" person_attribute_type_id="9" creator="1" date_created="2008-08-18 12:25:57.0" voided="false" uuid="f830bf48-af63-495a-897e-a779e229d503"/>
 </dataset>

Original file line number	Diff line number	Diff line change
`@@ -197,7 +197,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {`
`197`	`197`	`.filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))`
`198`	`198`	`.collect(Collectors.toList());`
`199`	`199`
`200`		`- assertThat(personAttributeExtensions, hasSize(3));`
	`200`	`+ assertThat(personAttributeExtensions, hasSize(4));`
`201`	`201`	`}`
`202`	`202`
`203`	`203`	`@Test`
Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {`
`148`	`148`	`.filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))`
`149`	`149`	`.collect(Collectors.toList());`
`150`	`150`
`151`		`- assertThat(personAttributeExtensions, hasSize(2));`
	`151`	`+ assertThat(personAttributeExtensions, hasSize(4));`
`152`	`152`	`}`
`153`	`153`
`154`	`154`	`@Test`
Original file line number	Diff line number	Diff line change
`@@ -205,7 +205,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {`
`205`	`205`	`.filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))`
`206`	`206`	`.collect(Collectors.toList());`
`207`	`207`
`208`		`- assertThat(personAttributeExtensions, hasSize(3));`
	`208`	`+ assertThat(personAttributeExtensions, hasSize(4));`
`209`	`209`	`}`
`210`	`210`
`211`	`211`	`@Test`
Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,7 @@ public void shouldReturnPersonAttributesAsExtensions() throws Exception {`
`154`	`154`	`.filter(ext -> ext.getUrl().equals(FhirConstants.OPENMRS_FHIR_EXT_PERSON_ATTRIBUTE))`
`155`	`155`	`.collect(Collectors.toList());`
`156`	`156`
`157`		`- assertThat(personAttributeExtensions, hasSize(2));`
	`157`	`+ assertThat(personAttributeExtensions, hasSize(4));`
`158`	`158`	`}`
`159`	`159`
`160`	`160`	`@Test`