From b9d8c0c13be4f80074aa066f501ebd43693461ee Mon Sep 17 00:00:00 2001 From: Yasmin Morshed Date: Wed, 10 Sep 2025 16:28:38 +0100 Subject: [PATCH 1/9] Remove some instances of the word dummy --- .../ehr/mapper/diagnosticreport/DiagnosticReportMapper.java | 2 +- .../diagnosticreport/diagnostic-report-with-no-specimen.xml | 2 +- .../specimen/expected_output_default_empty_specimen.xml | 2 +- ...expected_output_default_specimen_and_default_observation.xml | 2 +- .../diagnosticreport/specimen/input_default_specimen.json | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index 266dd9ed51..92c29603ff 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java @@ -202,7 +202,7 @@ private Specimen generateDummySpecimen(DiagnosticReport diagnosticReport) { specimen.setId(DUMMY_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")); } diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml index 9fe37d75a0..daf2240c54 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml @@ -58,7 +58,7 @@ - +
diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_empty_specimen.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_empty_specimen.xml index af084d6448..d231f29ded 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_empty_specimen.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_empty_specimen.xml @@ -10,7 +10,7 @@ - +
diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_specimen_and_default_observation.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_specimen_and_default_observation.xml index ac97da2af0..0e41bd8ecd 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_specimen_and_default_observation.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/expected_output_default_specimen_and_default_observation.xml @@ -10,7 +10,7 @@ - +
diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json index 6884715794..50f1cf36d1 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json @@ -7,7 +7,7 @@ ] }, "accessionIdentifier": { - "value": "DUMMY" + "value": "NOT PRESENT" }, "type": { "text": "UNKNOWN" From 8c7df261b4c11018aeddfdb215b479c97969fe68 Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Wed, 17 Sep 2025 13:48:20 +0100 Subject: [PATCH 2/9] test adjustments --- .../mapper/diagnosticreport/DiagnosticReportMapperTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java index 58c7da6e11..05abdcc1cb 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java @@ -250,12 +250,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 expectedXPaths = Collections.singletonList( + "/component/CompoundStatement/component/CompoundStatement/specimen/specimenRole/id[@extension=\"NOT PRESENT\"]"); when(specimenMapper.mapSpecimenToCompoundStatement( any(Specimen.class), @@ -288,6 +290,7 @@ void When_DiagnosticReport_With_NoReferencedSpecimenAndFilingCommentWithNoCommen final String actualXml = mapper.mapDiagnosticReportToCompoundStatement(diagnosticReport); assertThat(actualXml).isEqualToIgnoringWhitespace(expectedXml); + assertThatXml(actualXml).containsAllXPaths(expectedXPaths); } /** From 22c1e99aea1016192f225b420228a7ca7a3d61ce Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Fri, 19 Sep 2025 11:56:11 +0100 Subject: [PATCH 3/9] test adjustments --- .../mapper/diagnosticreport/DiagnosticReportMapper.java | 8 ++++---- .../diagnosticreport/DiagnosticReportMapperTest.java | 5 +++-- .../diagnostic-report-with-coded-diagnosis.xml | 2 +- .../diagnostic-report-with-conclusion.xml | 2 +- .../diagnostic-report-with-extension-id.xml | 2 +- .../diagnostic-report-with-multiple-coded-diagnosis.xml | 2 +- .../diagnostic-report-with-multiple-results.xml | 2 +- .../diagnostic-report-with-no-specimen.xml | 2 +- .../diagnostic-report-with-participant.xml | 2 +- .../diagnostic-report-with-required-data.xml | 2 +- .../diagnostic-report-with-status-narrative.xml | 2 +- .../diagnosticreport/specimen/input_default_specimen.json | 2 +- 12 files changed, 17 insertions(+), 16 deletions(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index 92c29603ff..54f1580346 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/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 = @@ -181,7 +181,7 @@ private List assignDummySpecimensToObservationsWithNoSpecimen( // 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)) + .filter(specimen -> specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX)) .toList().getFirst(); Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); @@ -199,7 +199,7 @@ private List assignDummySpecimensToObservationsWithNoSpecimen( private Specimen generateDummySpecimen(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("NOT PRESENT")) @@ -247,7 +247,7 @@ private List getSpecimenIdsWithoutObservation(List specimens, List 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()); } } diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java index 05abdcc1cb..aa869ea463 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java @@ -54,6 +54,7 @@ @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"; @@ -310,7 +311,7 @@ void When_DiagnosticReport_Has_SpecimenAndUnlinkedTestResult_Expect_ADummySpecim // This checks that the unlinked test result is given a dummy specimen. assertThat(actualXml).containsIgnoringWhitespaces( - ""); } @@ -411,7 +412,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( - ""); } diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-coded-diagnosis.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-coded-diagnosis.xml index 788650a164..58dffbdeb7 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-coded-diagnosis.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-coded-diagnosis.xml @@ -30,6 +30,6 @@ Status: unknown - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-conclusion.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-conclusion.xml index ae32c6c163..c0fb782802 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-conclusion.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-conclusion.xml @@ -30,6 +30,6 @@ Status: unknown - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-extension-id.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-extension-id.xml index 911c174646..ca21824539 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-extension-id.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-extension-id.xml @@ -21,6 +21,6 @@ Status: unknown - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-coded-diagnosis.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-coded-diagnosis.xml index d463c9db9c..b19880215f 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-coded-diagnosis.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-coded-diagnosis.xml @@ -30,6 +30,6 @@ Status: unknown - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-results.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-results.xml index 737f71cac4..44f4c2eb52 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-results.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-multiple-results.xml @@ -62,6 +62,6 @@ CommentDate:20100225154100 - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml index daf2240c54..c25c429ac7 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-no-specimen.xml @@ -48,7 +48,7 @@ - + diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-participant.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-participant.xml index 9da82f5740..c870dddc41 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-participant.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-participant.xml @@ -30,7 +30,7 @@ Participants: TEMPLE SOWERBY MEDICAL PRACTICE - + diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-required-data.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-required-data.xml index c3887c0569..4221f5fc8b 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-required-data.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-required-data.xml @@ -20,6 +20,6 @@ Status: unknown - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-status-narrative.xml b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-status-narrative.xml index c3887c0569..4221f5fc8b 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-status-narrative.xml +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/diagnostic-report-with-status-narrative.xml @@ -20,6 +20,6 @@ Status: unknown - + \ No newline at end of file diff --git a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json index 50f1cf36d1..cbfcccd5f9 100644 --- a/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json +++ b/service/src/test/resources/ehr/mapper/diagnosticreport/specimen/input_default_specimen.json @@ -1,6 +1,6 @@ { "resourceType": "Specimen", - "id": "DUMMY-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757", + "id": "NOT-PRESENT-SPECIMEN-5E496953-065B-41F2-9577-BE8F2FBD0757", "meta": { "profile": [ "https://fhir.nhs.uk/STU3/StructureDefinition/CareConnect-GPC-Specimen-1" From a23ec62f2e376e152a74fdd901892ee7af676177 Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Mon, 22 Sep 2025 10:29:01 +0100 Subject: [PATCH 4/9] code refactoring --- .../DiagnosticReportMapper.java | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index 54f1580346..e39692b40b 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java @@ -169,31 +169,27 @@ private boolean hasObservationsWithoutSpecimen(List observations) { * For correct display in EMIS, any observation without a specimen must be assigned a dummy specimen. */ private List assignDummySpecimensToObservationsWithNoSpecimen( - List observations, List specimens) { + List observations, List specimens) { List filingComments = getFilingComments(observations); - observations = new ArrayList<>(stripFilingComments(observations)); + List nonFilingObservations = new ArrayList<>(stripFilingComments(observations)); - if (!hasObservationsWithoutSpecimen(observations)) { - observations.addAll(filingComments); - return observations; - } - - // The assumption was made that all test results without a specimen will have the same dummy specimen referenced - Specimen dummySpecimen = specimens.stream() + if (hasObservationsWithoutSpecimen(nonFilingObservations)) { + Specimen dummySpecimen = specimens.stream() .filter(specimen -> specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX)) - .toList().getFirst(); + .findFirst() + .orElseThrow(() -> new IllegalStateException( + "No dummy specimen found with prefix: " + NOT_PRESENT_SPECIMEN_ID_PREFIX)); - Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); + Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); - for (Observation observation : observations) { - if (!observation.hasSpecimen() && !isFilingComment(observation)) { - observation.setSpecimen(dummySpecimenReference); - } + nonFilingObservations.stream() + .filter(obs -> !obs.hasSpecimen()) + .forEach(obs -> obs.setSpecimen(dummySpecimenReference)); } - observations.addAll(filingComments); - return observations; + nonFilingObservations.addAll(filingComments); + return nonFilingObservations; } private Specimen generateDummySpecimen(DiagnosticReport diagnosticReport) { From 1dfdbb6442f924f4b62b50e2afcb8acd60e2a0df Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Mon, 22 Sep 2025 10:46:12 +0100 Subject: [PATCH 5/9] adding test coverage --- .../DiagnosticReportMapper.java | 2 +- .../DiagnosticReportMapperTest.java | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index e39692b40b..3cdeb3d03c 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java @@ -168,7 +168,7 @@ private boolean hasObservationsWithoutSpecimen(List observations) { /** * For correct display in EMIS, any observation without a specimen must be assigned a dummy specimen. */ - private List assignDummySpecimensToObservationsWithNoSpecimen( + protected List assignDummySpecimensToObservationsWithNoSpecimen( List observations, List specimens) { List filingComments = getFilingComments(observations); diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java index aa869ea463..e21864e821 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java @@ -1,6 +1,7 @@ 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.jupiter.api.Assertions.assertAll; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -61,6 +62,7 @@ class DiagnosticReportMapperTest { 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 INPUT_JSON_REQUIRED_DATA = "diagnostic-report-with-required-data.json"; private static final String INPUT_JSON_EMPTY_SPECIMENS = "diagnostic-report-with-empty-specimens.json"; @@ -129,6 +131,44 @@ public void tearDown() { messageContext.resetMessageContext(); } + @Test + void shouldAssignDummySpecimenToObservationsWithoutSpecimen() { + + Observation obsWithoutSpecimen = new Observation(); + Observation obsWithSpecimen = new Observation(); + obsWithSpecimen.setSpecimen(new Reference("real-specimen")); + + List 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 specimens = List.of(realSpecimen, dummySpecimen); + + List 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"); + } + + @Test + void shouldThrowIfNoDummySpecimenFound() { + + Observation obsWithoutSpecimen = new Observation(); + List observations = List.of(obsWithoutSpecimen); + + Specimen realSpecimen = new Specimen(); + realSpecimen.setId("real-specimen"); + List specimens = List.of(realSpecimen); + + assertThatThrownBy(() -> + mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining(NOT_PRESENT_SPECIMEN_ID_PREFIX); + } + @ParameterizedTest @MethodSource("resourceFileParams") void When_MappingDiagnosticReportJson_Expect_CompoundStatementXmlOutput(String inputJson, String outputXml) { From 43fa507b6d92fb2277fb4bcbd613c73ad271c492 Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Mon, 22 Sep 2025 11:21:39 +0100 Subject: [PATCH 6/9] refactoring and test --- .../mapper/diagnosticreport/DiagnosticReportMapper.java | 7 ++++--- .../diagnosticreport/DiagnosticReportMapperTest.java | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index 3cdeb3d03c..53b07ea746 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java @@ -35,6 +35,7 @@ import uk.nhs.adaptors.gp2gp.common.service.ConfidentialityService; import uk.nhs.adaptors.gp2gp.common.service.RandomIdGeneratorService; +import uk.nhs.adaptors.gp2gp.ehr.exception.EhrMapperException; import uk.nhs.adaptors.gp2gp.ehr.mapper.CommentType; import uk.nhs.adaptors.gp2gp.ehr.mapper.IdMapper; import uk.nhs.adaptors.gp2gp.ehr.mapper.MessageContext; @@ -168,7 +169,7 @@ private boolean hasObservationsWithoutSpecimen(List observations) { /** * For correct display in EMIS, any observation without a specimen must be assigned a dummy specimen. */ - protected List assignDummySpecimensToObservationsWithNoSpecimen( + List assignDummySpecimensToObservationsWithNoSpecimen( List observations, List specimens) { List filingComments = getFilingComments(observations); @@ -178,8 +179,8 @@ protected List assignDummySpecimensToObservationsWithNoSpecimen( Specimen dummySpecimen = specimens.stream() .filter(specimen -> specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX)) .findFirst() - .orElseThrow(() -> new IllegalStateException( - "No dummy specimen found with prefix: " + NOT_PRESENT_SPECIMEN_ID_PREFIX)); + .orElseThrow(() -> new EhrMapperException( + "No not present specimen found with prefix: " + NOT_PRESENT_SPECIMEN_ID_PREFIX)); Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java index e21864e821..0df612b152 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java @@ -32,6 +32,7 @@ import org.mockito.quality.Strictness; import org.mockito.stubbing.Answer; +import uk.nhs.adaptors.gp2gp.ehr.exception.EhrMapperException; import uk.nhs.adaptors.gp2gp.ehr.mapper.AgentDirectory; import uk.nhs.adaptors.gp2gp.ehr.mapper.CodeableConceptCdMapper; import uk.nhs.adaptors.gp2gp.ehr.mapper.IdMapper; @@ -165,7 +166,7 @@ void shouldThrowIfNoDummySpecimenFound() { assertThatThrownBy(() -> mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens)) - .isInstanceOf(IllegalStateException.class) + .isInstanceOf(EhrMapperException.class) .hasMessageContaining(NOT_PRESENT_SPECIMEN_ID_PREFIX); } From e3ee7a6005828b3e00da56e1e3679491fa6a185f Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Mon, 22 Sep 2025 11:28:00 +0100 Subject: [PATCH 7/9] changelog update --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f17a77344a..cbb8b1c168 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,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. From 0356fd47023f112f81c64129f01389f85e12878f Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Mon, 22 Sep 2025 13:44:51 +0100 Subject: [PATCH 8/9] code and test refactoring --- .../DiagnosticReportMapper.java | 28 +++++++++---------- .../DiagnosticReportMapperTest.java | 18 ------------ 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index 53b07ea746..9cf4806d98 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java @@ -35,7 +35,6 @@ import uk.nhs.adaptors.gp2gp.common.service.ConfidentialityService; import uk.nhs.adaptors.gp2gp.common.service.RandomIdGeneratorService; -import uk.nhs.adaptors.gp2gp.ehr.exception.EhrMapperException; import uk.nhs.adaptors.gp2gp.ehr.mapper.CommentType; import uk.nhs.adaptors.gp2gp.ehr.mapper.IdMapper; import uk.nhs.adaptors.gp2gp.ehr.mapper.MessageContext; @@ -169,25 +168,26 @@ private boolean hasObservationsWithoutSpecimen(List observations) { /** * For correct display in EMIS, any observation without a specimen must be assigned a dummy specimen. */ - List assignDummySpecimensToObservationsWithNoSpecimen( - List observations, List specimens) { + List assignDummySpecimensToObservationsWithNoSpecimen(List observations, List specimens) { List filingComments = getFilingComments(observations); List nonFilingObservations = new ArrayList<>(stripFilingComments(observations)); - if (hasObservationsWithoutSpecimen(nonFilingObservations)) { - Specimen dummySpecimen = specimens.stream() - .filter(specimen -> specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX)) - .findFirst() - .orElseThrow(() -> new EhrMapperException( - "No not present specimen found with prefix: " + NOT_PRESENT_SPECIMEN_ID_PREFIX)); + if (!hasObservationsWithoutSpecimen(nonFilingObservations)) { + nonFilingObservations.addAll(filingComments); + return nonFilingObservations; + } - Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); + // 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(NOT_PRESENT_SPECIMEN_ID_PREFIX)) + .toList().getFirst(); - nonFilingObservations.stream() - .filter(obs -> !obs.hasSpecimen()) - .forEach(obs -> obs.setSpecimen(dummySpecimenReference)); - } + Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); + + nonFilingObservations.stream() + .filter(obs -> !obs.hasSpecimen() && !isFilingComment(obs)) + .forEach(obs -> obs.setSpecimen(dummySpecimenReference)); nonFilingObservations.addAll(filingComments); return nonFilingObservations; diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java index 0df612b152..5396027950 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java @@ -1,7 +1,6 @@ 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.jupiter.api.Assertions.assertAll; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -32,7 +31,6 @@ import org.mockito.quality.Strictness; import org.mockito.stubbing.Answer; -import uk.nhs.adaptors.gp2gp.ehr.exception.EhrMapperException; import uk.nhs.adaptors.gp2gp.ehr.mapper.AgentDirectory; import uk.nhs.adaptors.gp2gp.ehr.mapper.CodeableConceptCdMapper; import uk.nhs.adaptors.gp2gp.ehr.mapper.IdMapper; @@ -154,22 +152,6 @@ void shouldAssignDummySpecimenToObservationsWithoutSpecimen() { assertThat(result.get(1).getSpecimen().getReference()).contains("real-specimen"); } - @Test - void shouldThrowIfNoDummySpecimenFound() { - - Observation obsWithoutSpecimen = new Observation(); - List observations = List.of(obsWithoutSpecimen); - - Specimen realSpecimen = new Specimen(); - realSpecimen.setId("real-specimen"); - List specimens = List.of(realSpecimen); - - assertThatThrownBy(() -> - mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens)) - .isInstanceOf(EhrMapperException.class) - .hasMessageContaining(NOT_PRESENT_SPECIMEN_ID_PREFIX); - } - @ParameterizedTest @MethodSource("resourceFileParams") void When_MappingDiagnosticReportJson_Expect_CompoundStatementXmlOutput(String inputJson, String outputXml) { From ba00636e1569ac39b15253a798157e6ffcd71cc7 Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Mon, 22 Sep 2025 14:15:41 +0100 Subject: [PATCH 9/9] code and test refactoring --- .../DiagnosticReportMapper.java | 10 ++-- .../DiagnosticReportMapperTest.java | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java index 9cf4806d98..8cee1400c9 100644 --- a/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java +++ b/service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapper.java @@ -142,7 +142,7 @@ private List fetchSpecimens(DiagnosticReport diagnosticReport, List assignDummySpecimensToObservationsWithNoSpecimen(List specimen.getId().contains(NOT_PRESENT_SPECIMEN_ID_PREFIX)) .toList().getFirst(); - Reference dummySpecimenReference = new Reference(dummySpecimen.getId()); + Reference notPresentSpecimenReference = new Reference(notPresentSpecimen.getId()); nonFilingObservations.stream() .filter(obs -> !obs.hasSpecimen() && !isFilingComment(obs)) - .forEach(obs -> obs.setSpecimen(dummySpecimenReference)); + .forEach(obs -> obs.setSpecimen(notPresentSpecimenReference)); nonFilingObservations.addAll(filingComments); return nonFilingObservations; } - private Specimen generateDummySpecimen(DiagnosticReport diagnosticReport) { + private Specimen generateNotPresentSpecimen(DiagnosticReport diagnosticReport) { Specimen specimen = new Specimen(); specimen.setId(NOT_PRESENT_SPECIMEN_ID_PREFIX + randomIdGeneratorService.createNewId()); diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java index 5396027950..7d491c5b9b 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/diagnosticreport/DiagnosticReportMapperTest.java +++ b/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; @@ -62,6 +66,7 @@ class DiagnosticReportMapperTest { 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"; @@ -130,6 +135,53 @@ 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 observations = List.of(obsWithoutSpecimen, obsWithSpecimen, filingCommentObs); + + Specimen dummySpecimen = new Specimen(); + dummySpecimen.setId(NOT_PRESENT_SPECIMEN_ID_PREFIX + "123"); + List specimens = List.of(dummySpecimen); + + List 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 observations = List.of(obsWithoutSpecimen); + + List specimens = List.of(); + + assertThatThrownBy(() -> mapper.assignDummySpecimensToObservationsWithNoSpecimen(observations, specimens)) + .isInstanceOf(NoSuchElementException.class); + } + @Test void shouldAssignDummySpecimenToObservationsWithoutSpecimen() {