From 8b2c2ee488860420ce03339209b96f964bce0061 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Tue, 23 Jun 2026 16:06:50 +0100 Subject: [PATCH 1/6] NIAD-3471 Support multiple given names in practitioner XML - Changed EN.given field from String to List to support multiple given name elements - Updated AgentDirectoryMapper to iterate over all given names and add each to FHIR HumanName - Added helper method getGivenNamesAsString() to concatenate multiple given names for text field - Updated hasNoName() to correctly check for given names in List structure - Updated JAXB schema test to work with List given names - Added convenience method getFirstGiven() for backward compatibility - Added unit tests for multiple given names scenarios (with and without family name). --- .../mapper/AgentDirectoryMapper.java | 27 ++++++++++--- .../util/ParticipantReferenceUtil.java | 11 ++---- .../mapper/AgentDirectoryMapperTest.java | 39 ++++++++++++++++++- schema/src/main/java/org/hl7/v3/EN.java | 38 +++++++++++++++--- schema/src/test/java/JaxbTest.java | 3 +- 5 files changed, 98 insertions(+), 20 deletions(-) diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java index 3db0f0f93..fc3142378 100644 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java @@ -130,17 +130,23 @@ private List getPractitionerName(PN name) { } private static void setTextFromAvailableNameFields(PN name, HumanName humanName) { - var requiresSeparator = StringUtils.isNotBlank(name.getPrefix()) && StringUtils.isNotBlank(name.getGiven()); - var text = StringUtils.join(name.getPrefix(), (requiresSeparator ? " " : ""), name.getGiven()); + var givenNames = getGivenNamesAsString(name); + var requiresSeparator = StringUtils.isNotBlank(name.getPrefix()) && StringUtils.isNotBlank(givenNames); + var text = StringUtils.join(name.getPrefix(), (requiresSeparator ? " " : ""), givenNames); humanName.setText(text); } private void setHumanNameValuesFromName(PN name, HumanName humanName) { humanName.setFamily(name.getFamily()); - var given = getPractitionerGiven(name.getGiven()); - if (given != null) { - humanName.getGiven().add(given); + var givenList = name.getGiven(); + if (givenList != null && !givenList.isEmpty()) { + for (String givenName : givenList) { + var given = getPractitionerGiven(givenName); + if (given != null) { + humanName.getGiven().add(given); + } + } } var prefix = getPractitionerPrefix(name.getPrefix()); @@ -149,10 +155,19 @@ private void setHumanNameValuesFromName(PN name, HumanName humanName) { } } + private static String getGivenNamesAsString(PN name) { + var givenList = name.getGiven(); + if (givenList != null && !givenList.isEmpty()) { + return StringUtils.join(givenList, " "); + } + return null; + } + private static boolean hasNoName(PN name) { + var hasGiven = name != null && name.getGiven() != null && !name.getGiven().isEmpty(); return name == null || (StringUtils.isBlank(name.getFamily()) - && StringUtils.isBlank(name.getGiven()) + && !hasGiven && StringUtils.isBlank(name.getPrefix())); } diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/util/ParticipantReferenceUtil.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/util/ParticipantReferenceUtil.java index d4cc10b06..37b5112d2 100644 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/util/ParticipantReferenceUtil.java +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/util/ParticipantReferenceUtil.java @@ -71,19 +71,16 @@ public static Map> fetchRecorderAndAsserter(RCMRMT03 public static Reference getParticipant2Reference(RCMRMT030101UKEhrComposition ehrComposition, String typeCode) { - var participant2Reference = ehrComposition.getParticipant2().stream() + return ehrComposition.getParticipant2().stream() .filter(participant2 -> participant2.getNullFlavor() == null) .filter(participant2 -> typeCode.equals(participant2.getTypeCode().getFirst())) .map(RCMRMT030101UKParticipant2::getAgentRef) .map(RCMRMT030101UKAgentRef::getId) .filter(II::hasRoot) .map(II::getRoot) - .findFirst(); - - if (participant2Reference.isPresent()) { - return new Reference(PRACTITIONER_REFERENCE_PREFIX.formatted(participant2Reference.get())); - } - return null; + .findFirst() + .map(ref -> new Reference(PRACTITIONER_REFERENCE_PREFIX.formatted(ref))) + .orElse(null); } private static Optional getParticipant2Reference(RCMRMT030101UKEhrComposition ehrComposition) { diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java index f506f7d3e..9a1b51d60 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java @@ -248,7 +248,44 @@ public void When_MapAgentDirectoryOnlyAgentPersonWithNameElementWithOnlyGiven_Ex () -> assertThat(practitioner.getNameFirstRep().getFamily()).isNull(), () -> assertThat(practitioner.getNameFirstRep().getGiven()).isEmpty(), () -> assertThat(practitioner.getNameFirstRep().getPrefix()).isEmpty(), - () -> assertThat(practitioner.getNameFirstRep().getText()).isEqualTo("NHS") + () -> assertThat(practitioner.getNameFirstRep().getText()).isEqualTo("John Paul") + ); + } + + @Test + public void When_MapAgentWithMultipleGivenAndFamilyName_Expect_AllGivenNamesInArray() { + var agentDirectoryXml = """ + + + + + + + + Minire + E + Clarkson + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + assertAll( + () -> assertThat(practitioner.getId()).isEqualTo("CD8E40B3-6A3C-11F1-AE7C-00155D75C807"), + () -> assertThat(practitioner.getNameFirstRep().getFamily()).isEqualTo("Clarkson"), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).hasSize(2), + () -> assertThat(practitioner.getNameFirstRep().getGiven().getFirst().getValue()).isEqualTo("Minire"), + () -> assertThat(practitioner.getNameFirstRep().getGiven().get(1).getValue()).isEqualTo("E"), + () -> assertThat(practitioner.getNameFirstRep().getText()).isNull(), + () -> assertThat(practitioner.getMeta().getProfile().getFirst().getValue()).isEqualTo(PRACT_META_PROFILE) ); } diff --git a/schema/src/main/java/org/hl7/v3/EN.java b/schema/src/main/java/org/hl7/v3/EN.java index c391bd42b..adead270e 100644 --- a/schema/src/main/java/org/hl7/v3/EN.java +++ b/schema/src/main/java/org/hl7/v3/EN.java @@ -69,7 +69,7 @@ public class EN { protected List use; protected String delimiter; protected String family; - protected String given; + protected List given; protected String prefix; protected String suffix; protected IVLTS validTime; @@ -120,12 +120,40 @@ public void setFamily(String family) { this.family = family; } - public String getGiven() { - return given; + /** + * Gets the value of the given property. + * + *

+ * This accessor method returns a reference to the live list, + * not a snapshot. Therefore any modification you make to the + * returned list will be present inside the JAXB object. + * This is why there is not a set method for the given property. + * + *

+ * For example, to add a new item, do as follows: + * 

+     *    getGiven().add(newItem);
+     *
+ * + *

+ * Objects of the following type(s) are allowed in the list + * {@link String } + */ + public List getGiven() { + if (given == null) { + given = new ArrayList<>(); + } + return this.given; } - public void setGiven(String given) { - this.given = given; + /** + * Convenience method for backward compatibility - returns the first given name as a string. + */ + public String getFirstGiven() { + if (given != null && !given.isEmpty()) { + return given.get(0); + } + return null; } public String getPrefix() { diff --git a/schema/src/test/java/JaxbTest.java b/schema/src/test/java/JaxbTest.java index 759873637..fd625500a 100644 --- a/schema/src/test/java/JaxbTest.java +++ b/schema/src/test/java/JaxbTest.java @@ -40,7 +40,8 @@ void When_RCMRIN030000UK06MessageIsUnmarshalled_Expect_FieldsToBeParsable() thro // then assertThat(person.getName().getFamily()).isEqualTo("Whitcombe"); - assertThat(person.getName().getGiven()).isEqualTo("Peter"); + assertThat(person.getName().getGiven()).hasSize(1); + assertThat(person.getName().getGiven().getFirst()).isEqualTo("Peter"); assertThat(person.getName().getPrefix()).isEqualTo("Dr"); assertThat(person.getName().getValidTime().getCenter().getValue()).isEqualTo("20100114"); assertThat(place.getName()).isEqualTo("EMIS Test Practice Location"); From 4fb0f4692b0d12abda4e95468e8f0a513fc2edfd Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Tue, 23 Jun 2026 16:14:55 +0100 Subject: [PATCH 2/6] * Update CHANGELOG.md --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4d1c5c86..ac311748f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added * Added support for mapping different EhrSupplyType (e.g. NHS prescription, OTC sale) into the Medication Statement Prescribing Agency extension -* Added fallback for Condition.asserter to use the EHRComposition / author / agent field when EHRComposition / participant2 is absent. -* +* Added fallback for Condition.asserter to use the EHRComposition / author / agent field when EHRComposition / participant2 is absent. + ### Fixed +* Fixed handling of multiple `` name fields in practitioner XML to JSON mapping - now correctly captures all given names as an array in FHIR HumanName. * Improved error handling in SkeletonProcessingService to throw a meaningful IllegalArgumentException when a payload node cannot be matched to a skeleton document ID, replacing an uninformative NullPointerException. From 933075c9d75bd5779bc5b66feb3753332d263416 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Tue, 23 Jun 2026 16:31:05 +0100 Subject: [PATCH 3/6] * Update failing test with correct asserts. --- .../pss/translator/mapper/AgentDirectoryMapperTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java index 9a1b51d60..f53ac9920 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java @@ -248,7 +248,7 @@ public void When_MapAgentDirectoryOnlyAgentPersonWithNameElementWithOnlyGiven_Ex () -> assertThat(practitioner.getNameFirstRep().getFamily()).isNull(), () -> assertThat(practitioner.getNameFirstRep().getGiven()).isEmpty(), () -> assertThat(practitioner.getNameFirstRep().getPrefix()).isEmpty(), - () -> assertThat(practitioner.getNameFirstRep().getText()).isEqualTo("John Paul") + () -> assertThat(practitioner.getNameFirstRep().getText()).isEqualTo("NHS") ); } From b9bf6455541063938d2932e6ccdf5790c49c2a75 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Tue, 23 Jun 2026 16:41:10 +0100 Subject: [PATCH 4/6] * Add additional tests to address surviving mutations. --- .../mapper/AgentDirectoryMapperTest.java | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java index f53ac9920..b5b2c3ca8 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java @@ -289,6 +289,113 @@ public void When_MapAgentWithMultipleGivenAndFamilyName_Expect_AllGivenNamesInAr ); } + @Test + public void When_MapAgentWithThreeGivenNames_Expect_AllThreeGivenNamesInArray() { + var agentDirectoryXml = """ + + + + + + + John + Paul + George + Smith + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + assertAll( + () -> assertThat(practitioner.getNameFirstRep().getFamily()).isEqualTo("Smith"), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).hasSize(3), + () -> assertThat(practitioner.getNameFirstRep().getGiven().getFirst().getValue()).isEqualTo("John"), + () -> assertThat(practitioner.getNameFirstRep().getGiven().get(1).getValue()).isEqualTo("Paul"), + () -> assertThat(practitioner.getNameFirstRep().getGiven().get(2).getValue()).isEqualTo("George"), + () -> assertThat(practitioner.getNameFirstRep().getText()).isNull() + ); + } + + @Test + public void When_MapAgentWithEmptyGivenNameInList_Expect_OnlyNonEmptyGivenNamesAdded() { + var agentDirectoryXml = """ + + + + + + + John + + Paul + Smith + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + // Should only have 2 given names since one is empty + assertAll( + () -> assertThat(practitioner.getNameFirstRep().getFamily()).isEqualTo("Smith"), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).hasSize(2), + () -> assertThat(practitioner.getNameFirstRep().getGiven().getFirst().getValue()).isEqualTo("John"), + () -> assertThat(practitioner.getNameFirstRep().getGiven().get(1).getValue()).isEqualTo("Paul") + ); + } + + @Test + public void When_MapAgentWithMultipleGivenNamesAndPrefix_Expect_AllNamesAndPrefix() { + var agentDirectoryXml = """ + + + + + + + Dr + John + Robert + Smith + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + assertAll( + () -> assertThat(practitioner.getNameFirstRep().getPrefix()).hasSize(1), + () -> assertThat(practitioner.getNameFirstRep().getPrefix().getFirst().getValue()).isEqualTo("Dr"), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).hasSize(2), + () -> assertThat(practitioner.getNameFirstRep().getGiven().getFirst().getValue()).isEqualTo("John"), + () -> assertThat(practitioner.getNameFirstRep().getGiven().get(1).getValue()).isEqualTo("Robert"), + () -> assertThat(practitioner.getNameFirstRep().getFamily()).isEqualTo("Smith") + ); + } + @Test public void When_MapAgentDirectoryOnlyAgentPersonWithNameElementWithOnlyPrefix_Expect_TextSetToPrefix() { var agentDirectoryXml = """ From cb5203087aca4fed20123467a8bcb58556482652 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT <88717465+MartinWheelerMT@users.noreply.github.com> Date: Tue, 23 Jun 2026 17:09:46 +0100 Subject: [PATCH 5/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../pss/translator/mapper/AgentDirectoryMapper.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java index fc3142378..72d43e450 100644 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java @@ -156,11 +156,15 @@ private void setHumanNameValuesFromName(PN name, HumanName humanName) { } private static String getGivenNamesAsString(PN name) { - var givenList = name.getGiven(); - if (givenList != null && !givenList.isEmpty()) { - return StringUtils.join(givenList, " "); + if (name == null) { + return null; } - return null; + + var givenList = name.getGiven().stream() + .filter(StringUtils::isNotBlank) + .toList(); + + return givenList.isEmpty() ? null : StringUtils.join(givenList, " "); } private static boolean hasNoName(PN name) { From 36187c1b1550a092783a607f7b6d034dec00e916 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Tue, 23 Jun 2026 17:14:56 +0100 Subject: [PATCH 6/6] * Add additional tests to address surviving mutations. --- .../mapper/AgentDirectoryMapperTest.java | 97 ++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java index b5b2c3ca8..e5171ef8e 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java @@ -291,6 +291,7 @@ public void When_MapAgentWithMultipleGivenAndFamilyName_Expect_AllGivenNamesInAr @Test public void When_MapAgentWithThreeGivenNames_Expect_AllThreeGivenNamesInArray() { + final var expectedGivenCount = 3; var agentDirectoryXml = """ @@ -317,7 +318,7 @@ public void When_MapAgentWithThreeGivenNames_Expect_AllThreeGivenNamesInArray() assertAll( () -> assertThat(practitioner.getNameFirstRep().getFamily()).isEqualTo("Smith"), - () -> assertThat(practitioner.getNameFirstRep().getGiven()).hasSize(3), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).hasSize(expectedGivenCount), () -> assertThat(practitioner.getNameFirstRep().getGiven().getFirst().getValue()).isEqualTo("John"), () -> assertThat(practitioner.getNameFirstRep().getGiven().get(1).getValue()).isEqualTo("Paul"), () -> assertThat(practitioner.getNameFirstRep().getGiven().get(2).getValue()).isEqualTo("George"), @@ -396,6 +397,100 @@ public void When_MapAgentWithMultipleGivenNamesAndPrefix_Expect_AllNamesAndPrefi ); } + @Test + public void When_MapAgentWithPrefixButNullGivenNames_Expect_TextSetToPrefix() { + var agentDirectoryXml = """ + + + + + + + Prof + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + assertAll( + () -> assertThat(practitioner.getNameFirstRep().getText()).isEqualTo("Prof"), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).isEmpty(), + () -> assertThat(practitioner.getNameFirstRep().getFamily()).isNull(), + () -> assertThat(practitioner.getNameFirstRep().getPrefix()).isEmpty() + ); + } + + @Test + public void When_MapAgentWithAllGivenNamesEmpty_Expect_TextSetToEmpty() { + // This test ensures that if getGiven() returns an empty list (all items filtered out), + // hasNoName() correctly identifies it as having NO name + var agentDirectoryXml = """ + + + + + + + + + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + assertThat(practitioner.getNameFirstRep().getGiven()).isEmpty(); + } + + @Test + public void When_MapAgentWithPrefixAndEmptyGivenNames_Expect_TextSetOnlyToPrefix() { + var agentDirectoryXml = """ + + + + + + + Dr + + Smith + + + + + """; + var agentDirectory = unmarshallAgentDirectoryFromXmlString(agentDirectoryXml); + + var mappedAgents = agentDirectoryMapper.mapAgentDirectory(agentDirectory); + + assertThat(mappedAgents).hasSize(1); + + var practitioner = (Practitioner) mappedAgents.getFirst(); + + assertAll( + () -> assertThat(practitioner.getNameFirstRep().getFamily()).isEqualTo("Smith"), + () -> assertThat(practitioner.getNameFirstRep().getPrefix()).hasSize(1), + () -> assertThat(practitioner.getNameFirstRep().getPrefix().getFirst().getValue()).isEqualTo("Dr"), + () -> assertThat(practitioner.getNameFirstRep().getGiven()).isEmpty(), + () -> assertThat(practitioner.getNameFirstRep().getText()).isNull() + ); + } + @Test public void When_MapAgentDirectoryOnlyAgentPersonWithNameElementWithOnlyPrefix_Expect_TextSetToPrefix() { var agentDirectoryXml = """