NIAD-3216: When DiagnosticReport doesn't contain a Specimen reference, we are sending a value of DUMMY (#1326)

* Remove some instances of the word dummy

* test adjustments

* test adjustments

* code refactoring

* adding test coverage

* refactoring and test

* changelog update

* code and test refactoring

* code and test refactoring

---------

Co-authored-by: ORybak5 <[email protected]>

Author: morsh3d

CHANGELOG.md

@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * The GP2GP Adaptor now populates the ObservationStatement / confidentialityCode field when the .meta.security field of an Uncategorized Data Observation contains NOPAT
 * When List.meta.security field contains NOPAT, the GP2GP Adaptor will now populate the CompoundStatement.confidentialityCode
 
+### Fixed
+* When DiagnosticReport doesn't contain a Specimen reference, instead of "DUMMY" "NOT-PRESENT" value is used
+
 ### Update
 
 * [GP Connect 1.6.1] The GP2GP Adaptor is now able to identify e-referrals by using either `https://fhir.nhs.uk/Id/ubr-number` or `https://fhir.nhs.uk/Id/UBRN` when provided as an identifier system URL.

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java

@@ -52,7 +52,7 @@
 @Slf4j
 public class DiagnosticReportMapper {
 
-    public static final String DUMMY_SPECIMEN_ID_PREFIX = "DUMMY-SPECIMEN-";
+    public static final String NOT_PRESENT_SPECIMEN_ID_PREFIX = "NOT-PRESENT-SPECIMEN-";
     public static final String DUMMY_OBSERVATION_ID_PREFIX = "DUMMY-OBSERVATION-";
 
     private static final Mustache DIAGNOSTIC_REPORT_COMPOUND_STATEMENT_TEMPLATE =
@@ -142,7 +142,7 @@ private List<Specimen> fetchSpecimens(DiagnosticReport diagnosticReport, List<Ob
 
         // At least one specimen is required to exist for any DiagnosticReport, according to the mim
         if (!diagnosticReport.hasSpecimen() || hasObservationsWithoutSpecimen(observations)) {
-            specimens.add(generateDummySpecimen(diagnosticReport));
+            specimens.add(generateNotPresentSpecimen(diagnosticReport));
         }
 
         var inputBundleHolder = messageContext.getInputBundleHolder();
@@ -168,41 +168,38 @@ private boolean hasObservationsWithoutSpecimen(List<Observation> observations) {
     /**
      * For correct display in EMIS, any observation without a specimen must be assigned a dummy specimen.
      */
-    private List<Observation> assignDummySpecimensToObservationsWithNoSpecimen(
-            List<Observation> observations, List<Specimen> specimens) {
+    List<Observation> assignDummySpecimensToObservationsWithNoSpecimen(List<Observation> observations, List<Specimen> specimens) {
 
         List<Observation> filingComments = getFilingComments(observations);
-        observations = new ArrayList<>(stripFilingComments(observations));
+        List<Observation> nonFilingObservations = new ArrayList<>(stripFilingComments(observations));
 
-        if (!hasObservationsWithoutSpecimen(observations)) {
-            observations.addAll(filingComments);
-            return observations;
+        if (!hasObservationsWithoutSpecimen(nonFilingObservations)) {
+            nonFilingObservations.addAll(filingComments);
+            return nonFilingObservations;
         }
 
         // The assumption was made that all test results without a specimen will have the same dummy specimen referenced
-        Specimen dummySpecimen = specimens.stream()
-                .filter(specimen -> specimen.getId().contains(DUMMY_SPECIMEN_ID_PREFIX))
-                .toList().getFirst();
+        Specimen notPresentSpecimen = specimens.stream()
+            .filter(specimen -> specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX))
+            .toList().getFirst();
 
-        Reference dummySpecimenReference = new Reference(dummySpecimen.getId());
+        Reference notPresentSpecimenReference = new Reference(notPresentSpecimen.getId());
 
-        for (Observation observation : observations) {
-            if (!observation.hasSpecimen() && !isFilingComment(observation)) {
-                observation.setSpecimen(dummySpecimenReference);
-            }
-        }
+        nonFilingObservations.stream()
+            .filter(obs -> !obs.hasSpecimen() && !isFilingComment(obs))
+            .forEach(obs -> obs.setSpecimen(notPresentSpecimenReference));
 
-        observations.addAll(filingComments);
-        return observations;
+        nonFilingObservations.addAll(filingComments);
+        return nonFilingObservations;
     }
 
-    private Specimen generateDummySpecimen(DiagnosticReport diagnosticReport) {
+    private Specimen generateNotPresentSpecimen(DiagnosticReport diagnosticReport) {
         Specimen specimen = new Specimen();
 
-        specimen.setId(DUMMY_SPECIMEN_ID_PREFIX + randomIdGeneratorService.createNewId());
+        specimen.setId(NOT_PRESENT_SPECIMEN_ID_PREFIX + randomIdGeneratorService.createNewId());
 
         return specimen
-            .setAccessionIdentifier(new Identifier().setValue("DUMMY"))
+            .setAccessionIdentifier(new Identifier().setValue("NOT PRESENT"))
             .setCollection(new Specimen.SpecimenCollectionComponent().setCollected(new DateTimeType(diagnosticReport.getIssued())))
             .setType(new CodeableConcept().setText("UNKNOWN"));
     }
@@ -247,7 +244,7 @@ private List<String> getSpecimenIdsWithoutObservation(List<Specimen> specimens,
         List<String> nonOrphanSpecimenIDList = new ArrayList<>();
         for (Specimen specimen : specimens) {
             // Dummy Specimens should not have a dummy observation attached.
-            if (!specimen.getId().contains(DUMMY_SPECIMEN_ID_PREFIX)) {
+            if (!specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX)) {
                 specimenIDList.add(specimen.getId());
             }
         }

service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java

@@ -1,17 +1,21 @@
 package uk.nhs.adaptors.gp2gp.ehr.mapper.diagnosticreport;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+import static org.junit.Assert.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.when;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.stream.Stream;
 
 import org.hl7.fhir.dstu3.model.Bundle;
 import org.hl7.fhir.dstu3.model.CodeableConcept;
+import org.hl7.fhir.dstu3.model.Coding;
 import org.hl7.fhir.dstu3.model.DiagnosticReport;
 import org.hl7.fhir.dstu3.model.IdType;
 import org.hl7.fhir.dstu3.model.Observation;
@@ -54,12 +58,15 @@
 @ExtendWith(MockitoExtension.class)
 @MockitoSettings(strictness = Strictness.LENIENT)
 class DiagnosticReportMapperTest {
+
     private static final String TEST_FILE_DIRECTORY = "/ehr/mapper/diagnosticreport/";
 
     private static final String INPUT_JSON_BUNDLE = "fhir_bundle.json";
     private static final String INPUT_JSON_BUNDLE_WITH_FILING_COMMENTS = "fhir_bundle_with_filing_comments.json";
 
     private static final String TEST_ID = "5E496953-065B-41F2-9577-BE8F2FBD0757";
+    public static final String NOT_PRESENT_SPECIMEN_ID_PREFIX = "NOT-PRESENT-SPECIMEN-";
+    private static final String COMMENT_NOTE = "37331000000100";
 
     private static final String INPUT_JSON_REQUIRED_DATA = "diagnostic-report-with-required-data.json";
     private static final String INPUT_JSON_EMPTY_SPECIMENS = "diagnostic-report-with-empty-specimens.json";
@@ -128,6 +135,75 @@ public void tearDown() {
         messageContext.resetMessageContext();
     }
 
+    @Test
+    void shouldAssignDummySpecimenOnlyToNonFilingObservationsWithoutSpecimen() {
+
+        Observation obsWithoutSpecimen = new Observation();
+        obsWithoutSpecimen.setId("obs1");
+
+        Observation obsWithSpecimen = new Observation();
+        obsWithSpecimen.setId("obs2");
+        obsWithSpecimen.setSpecimen(new Reference("real-specimen"));
+
+        Observation filingCommentObs = new Observation();
+        filingCommentObs.setId("obs3");
+        filingCommentObs.getCode().addCoding(new Coding().setCode(COMMENT_NOTE));
+
+        List<Observation> observations = List.of(obsWithoutSpecimen, obsWithSpecimen, filingCommentObs);
+
+        Specimen dummySpecimen = new Specimen();
+        dummySpecimen.setId(NOT_PRESENT_SPECIMEN_ID_PREFIX + "123");
+        List<Specimen> specimens = List.of(dummySpecimen);
+
+        List<Observation> result = mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens);
+
+        assertThat(obsWithoutSpecimen.getSpecimen())
+            .isNotNull()
+            .extracting(Reference::getReference)
+            .isEqualTo(dummySpecimen.getId());
+
+        assertThat(obsWithSpecimen.getSpecimen())
+            .extracting(Reference::getReference)
+            .isEqualTo("real-specimen");
+
+        assertFalse(filingCommentObs.hasSpecimen());
+        assertThat(result).containsExactlyInAnyOrder(obsWithoutSpecimen, obsWithSpecimen, filingCommentObs);
+    }
+
+    @Test
+    void shouldThrowIfNoDummySpecimenFound() {
+
+        Observation obsWithoutSpecimen = new Observation();
+        List<Observation> observations = List.of(obsWithoutSpecimen);
+
+        List<Specimen> specimens = List.of();
+
+        assertThatThrownBy(() -> mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens))
+            .isInstanceOf(NoSuchElementException.class);
+    }
+
+    @Test
+    void shouldAssignDummySpecimenToObservationsWithoutSpecimen() {
+
+        Observation obsWithoutSpecimen = new Observation();
+        Observation obsWithSpecimen = new Observation();
+        obsWithSpecimen.setSpecimen(new Reference("real-specimen"));
+
+        List<Observation> observations = List.of(obsWithoutSpecimen, obsWithSpecimen);
+
+        Specimen dummySpecimen = new Specimen();
+        dummySpecimen.setId("dummy-" + NOT_PRESENT_SPECIMEN_ID_PREFIX);
+        Specimen realSpecimen = new Specimen();
+        realSpecimen.setId("real-specimen");
+        List<Specimen> specimens = List.of(realSpecimen, dummySpecimen);
+
+        List<Observation> result = mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens);
+
+        assertThat(result).hasSize(2);
+        assertThat(result.get(0).getSpecimen().getReference()).contains(dummySpecimen.getId());
+        assertThat(result.get(1).getSpecimen().getReference()).contains("real-specimen");
+    }
+
     @ParameterizedTest
     @MethodSource("resourceFileParams")
     void When_MappingDiagnosticReportJson_Expect_CompoundStatementXmlOutput(String inputJson, String outputXml) {
@@ -250,12 +326,14 @@ void When_DiagnosticReport_With_ObservationEffectivePeriodAndCommentNote_Expect_
     }
 
     @Test
-    void When_DiagnosticReport_With_NoReferencedSpecimenAndFilingCommentWithNoComment_Expect_MatchesSnapshotXml() {
+    void When_DR_With_NoReferencedSpecimenAndFilingCommentWithNoComment_Expect_MatchesSnapshotXmlIncludesSpecimenRoleWithNotPresentTag() {
         final String diagnosticReportFileName = "diagnostic-report-with-no-specimen.json";
         final DiagnosticReport diagnosticReport = getDiagnosticReportResourceFromJson(diagnosticReportFileName);
         final Bundle bundle = getBundleResourceFromJson(INPUT_JSON_BUNDLE);
         final InputBundle inputBundle = new InputBundle(bundle);
         final String expectedXml = getXmlStringFromFile(TEST_FILE_DIRECTORY, "diagnostic-report-with-no-specimen.xml");
+        final List<String> expectedXPaths = Collections.singletonList(
+            "/component/CompoundStatement/component/CompoundStatement/specimen/specimenRole/id[@extension=\"NOT PRESENT\"]");
 
         when(specimenMapper.mapSpecimenToCompoundStatement(
             any(Specimen.class),
@@ -288,6 +366,7 @@ void When_DiagnosticReport_With_NoReferencedSpecimenAndFilingCommentWithNoCommen
         final String actualXml = mapper.mapDiagnosticReportToCompoundStatement(diagnosticReport);
 
         assertThat(actualXml).isEqualToIgnoringWhitespace(expectedXml);
+        assertThatXml(actualXml).containsAllXPaths(expectedXPaths);
     }
 
     /**
@@ -307,7 +386,7 @@ void When_DiagnosticReport_Has_SpecimenAndUnlinkedTestResult_Expect_ADummySpecim
 
         // This checks that the unlinked test result is given a dummy specimen.
         assertThat(actualXml).containsIgnoringWhitespaces(
-                "<!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 "
+                "<!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 "
                         + "with linked Observations: Observation/TestResult-WithoutSpecimenReference-->");
     }
 
@@ -408,7 +487,7 @@ void When_DiagnosticReport_Has_SpecimenALinkedTestResultAndAnUnlinkedTestResult_
         final String actualXml = mapper.mapDiagnosticReportToCompoundStatement(diagnosticReport);
         // This checks that the unlinked observation is given a dummy specimen.
         assertThat(actualXml).containsIgnoringWhitespaces(
-                "<!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 "
+                "<!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 "
                         + "with linked Observations: Observation/TestResult-WithoutSpecimenReference-->");
 
     }

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-coded-diagnosis.xml

@@ -30,6 +30,6 @@ Status: unknown</text>
         <availabilityTime value="20100225154100"/>
     </NarrativeStatement>
 </component>
-            <!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757-->
+            <!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757-->
     </CompoundStatement>
 </component>

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-conclusion.xml

@@ -30,6 +30,6 @@ Status: unknown</text>
         <availabilityTime value="20100225154100"/>
     </NarrativeStatement>
 </component>
-            <!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
+            <!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
     </CompoundStatement>
 </component>

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-extension-id.xml

@@ -21,6 +21,6 @@ Status: unknown</text>
         <availabilityTime value="20100225154100"/>
     </NarrativeStatement>
 </component>
-            <!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
+            <!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
     </CompoundStatement>
 </component>

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-coded-diagnosis.xml

@@ -30,6 +30,6 @@ Status: unknown</text>
         <availabilityTime value="20100225154100"/>
     </NarrativeStatement>
 </component>
-        <!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
+        <!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
     </CompoundStatement>
 </component>

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-results.xml

@@ -62,6 +62,6 @@ CommentDate:20100225154100
         <availabilityTime value="20100225154100"/>
     </NarrativeStatement>
 </component>
-            <!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
+            <!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
     </CompoundStatement>
 </component>

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml

@@ -48,7 +48,7 @@
         </component>
         <component typeCode="COMP" contextConductionInd="true">
             <CompoundStatement classCode="CLUSTER" moodCode="EVN">
-                <id root="II-for-Specimen-DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757"/>
+                <id root="II-for-Specimen-NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757"/>
                 <code code="123038009" codeSystem="2.16.840.1.113883.2.1.3.2.4.15" displayName="specimen (specimen)"/>
                 <statusCode code="COMPLETE"/>
                 <effectiveTime>
@@ -58,7 +58,7 @@
                 <specimen typeCode="SPC">
                     <specimenRole classCode="SPEC">
                         <id root="5E496953-065B-41F2-9577-BE8F2FBD0757"/>
-                        <id root="2.16.840.1.113883.2.1.4.5.2" extension="DUMMY"/>
+                        <id root="2.16.840.1.113883.2.1.4.5.2" extension="NOT PRESENT"/>
                         <effectiveTime>
                             <center value="20010330162700"/>
                         </effectiveTime>

service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-participant.xml

@@ -30,7 +30,7 @@ Participants: TEMPLE SOWERBY MEDICAL PRACTICE</text>
         <availabilityTime value="20100225154100"/>
     </NarrativeStatement>
 </component>
-            <!-- Mapped Specimen with id: DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
+            <!-- Mapped Specimen with id: NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757 -->
             <Participant typeCode="AUT" contextControlCode="OP">
     <agentRef classCode="AGNT">
         <id root="II-for-Organization/5E496953-065B-41F2-9577-BE8F2FBD0757"/>