Refactoring DiaryPlanStatementMapper (#914)

* Make Optional<String> return types non-optional

These private methods always return a String, which caused some
brief confusion while reading the code.

By being clearer that the methods do always return a String object
we will hopefully help a future soul not fall into the same trap.

* Refactor ProcedureRequest mapping methods to clariy intent

Separate out method which maps all ProcedureRequests, and the
behaviour which is specific to Diary Entries

* Fix issue with method names after rebasing through GitHub

---------

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

Author: Adrian Clay

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

@@ -50,21 +50,25 @@ public class DiaryPlanStatementMapper {
     private final CodeableConceptCdMapper codeableConceptCdMapper;
     private final ParticipantMapper participantMapper;
 
-    public String mapDiaryProcedureRequestToPlanStatement(ProcedureRequest procedureRequest, Boolean isNested) {
-        if (procedureRequest.getIntent() != ProcedureRequest.ProcedureRequestIntent.PLAN) {
-            return null;
+    public String mapProcedureRequestToPlanStatement(ProcedureRequest procedureRequest, Boolean isNested) {
+        if (procedureRequest.getIntent() == ProcedureRequest.ProcedureRequestIntent.PLAN) {
+            return mapDiaryEntryToPlanStatement(procedureRequest, isNested);
         }
 
+        return null;
+    }
+
+    private String mapDiaryEntryToPlanStatement(ProcedureRequest procedureRequest, Boolean isNested) {
         var idMapper = messageContext.getIdMapper();
         var availabilityTime = buildAvailabilityTime(procedureRequest);
         PlanStatementMapperParametersBuilder builder = PlanStatementMapperParameters.builder()
             .isNested(isNested)
             .id(idMapper.getOrNew(ResourceType.ProcedureRequest, procedureRequest.getIdElement()))
-            .availabilityTime(availabilityTime);
+            .availabilityTime(availabilityTime)
+            .effectiveTime(buildEffectiveTime(procedureRequest))
+            .text(buildText(procedureRequest))
+            .code(buildCode(procedureRequest));
 
-        buildEffectiveTime(procedureRequest).ifPresent(builder::effectiveTime);
-        buildText(procedureRequest).ifPresent(builder::text);
-        builder.code(buildCode(procedureRequest));
         buildParticipant(procedureRequest).ifPresent(builder::participant);
 
         return TemplateUtils.fillTemplate(PLAN_STATEMENT_TEMPLATE, builder.build());
@@ -80,7 +84,7 @@ private String buildAvailabilityTime(ProcedureRequest procedureRequest) {
         );
     }
 
-    private Optional<String> buildEffectiveTime(ProcedureRequest procedureRequest) {
+    private String buildEffectiveTime(ProcedureRequest procedureRequest) {
         DateTimeType date = null;
         if (procedureRequest.hasOccurrenceDateTimeType()) {
             date = procedureRequest.getOccurrenceDateTimeType();
@@ -89,11 +93,11 @@ private Optional<String> buildEffectiveTime(ProcedureRequest procedureRequest) {
             date = occurrencePeriod.hasEnd() ? occurrencePeriod.getEndElement() : occurrencePeriod.getStartElement();
         }
 
-        return Optional.of(formatEffectiveDate(date));
+        return formatEffectiveDate(date);
     }
 
-    private Optional<String> buildText(ProcedureRequest procedureRequest) {
-        return Optional.of(Stream.of(
+    private String buildText(ProcedureRequest procedureRequest) {
+        return Stream.of(
             getSupportingInformation(procedureRequest),
             getEarliestRecallDate(procedureRequest),
             getRequester(procedureRequest),
@@ -102,7 +106,7 @@ private Optional<String> buildText(ProcedureRequest procedureRequest) {
         )
             .filter(Optional::isPresent)
             .map(Optional::get)
-            .collect(Collectors.joining(StringUtils.SPACE)));
+            .collect(Collectors.joining(StringUtils.SPACE));
     }
 
     private String buildCode(ProcedureRequest procedureRequest) {

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

@@ -294,7 +294,7 @@ private Optional<String> mapObservation(Resource resource) {
 
     private Optional<String> mapProcedureRequest(Resource resource) {
         return Optional.ofNullable(
-            diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement((ProcedureRequest) resource, IS_NESTED)
+            diaryPlanStatementMapper.mapProcedureRequestToPlanStatement((ProcedureRequest) resource, IS_NESTED)
         );
     }
 

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

@@ -93,7 +93,7 @@ public void When_MappingProcedureRequestIsNested_Expect_ResourceMappedWithIsNest
         String inputJson = ResourceTestFileUtils.getFileContent(INPUT_PROCEDURE_REQUEST_WITH_ALL_DATA);
         ProcedureRequest inputProcedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
 
-        var mappedXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(inputProcedureRequest, true);
+        var mappedXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(inputProcedureRequest, true);
         assertThat(mappedXml).contains(expectedXml);
     }
 
@@ -102,7 +102,7 @@ public void When_MappingProcedureRequestThatIsNotPlan_Expect_ResourceNotMapped()
         String inputJson = ResourceTestFileUtils.getFileContent(INPUT_PROCEDURE_REQUEST_IS_NOT_PLAN);
         ProcedureRequest inputProcedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
 
-        var mappedXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(inputProcedureRequest, true);
+        var mappedXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(inputProcedureRequest, true);
         assertThat(mappedXml).isNull();
     }
 
@@ -112,7 +112,7 @@ public void When_MappingProcedureRequestWithoutRequiredAuthoredOn_Expect_MapperE
         ProcedureRequest inputProcedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
 
         assertThrows(EhrMapperException.class, ()
-            -> diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(inputProcedureRequest, true));
+            -> diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(inputProcedureRequest, true));
     }
 
     @Test
@@ -150,7 +150,7 @@ public void When_MappingWithOccurrenceWithStartAndEnd_Expect_TextContainsEarlies
             </component>""";
 
         final var procedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
-        var actualXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(procedureRequest, false);
+        var actualXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(procedureRequest, false);
 
         assertThat(actualXml).isEqualTo(expectedXml);
     }
@@ -186,7 +186,7 @@ public void When_MappingWithOccurrenceWithOnlyStart_Expect_TextDoesNotContainEar
                 </component>""";
 
         final var procedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
-        var actualXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(procedureRequest, false);
+        var actualXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(procedureRequest, false);
 
         assertThat(actualXml).isEqualTo(expectedXml);
     }
@@ -227,7 +227,7 @@ public void When_MappingWithDeviceReferenceWhereDeviceHasManufacturer_Expect_Tex
             </component>""";
 
         final var procedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
-        var actualXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(procedureRequest, false);
+        var actualXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(procedureRequest, false);
 
         assertThat(actualXml).isEqualTo(expectedXml);
     }
@@ -268,7 +268,7 @@ public void When_MappingWithDeviceReferenceWhereDeviceHasNoManufacturer_Expect_T
             </component>""";
 
         final var procedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
-        var actualXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(procedureRequest, false);
+        var actualXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(procedureRequest, false);
 
         assertThat(actualXml).isEqualTo(expectedXml);
     }
@@ -281,7 +281,7 @@ public void When_MappingProcedureRequest_Expect_ResourceMapped(String inputJsonP
         String inputJson = ResourceTestFileUtils.getFileContent(inputJsonPath);
         ProcedureRequest inputProcedureRequest = new FhirParseService().parseResource(inputJson, ProcedureRequest.class);
 
-        var mappedXml = diaryPlanStatementMapper.mapDiaryProcedureRequestToPlanStatement(inputProcedureRequest, false);
+        var mappedXml = diaryPlanStatementMapper.mapProcedureRequestToPlanStatement(inputProcedureRequest, false);
         assertThat(mappedXml).contains(expectedXml);
     }