Skip to content

NIAD-3471 Support multiple given names in practitioner XML #1292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MartinWheelerMT wants to merge 6 commits into
base: main
Choose a base branch
from
Open

NIAD-3471 Support multiple given names in practitioner XML #1292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6749d03
NIAD-3471 Support multiple given names in practitioner XML
MartinWheelerMT Jun 23, 2026
787dcd1
* Update CHANGELOG.md
MartinWheelerMT Jun 23, 2026
2300d92
* Update failing test with correct asserts.
MartinWheelerMT Jun 23, 2026
e7e885d
* Add additional tests to address surviving mutations.
MartinWheelerMT Jun 23, 2026
9f13500
Potential fix for pull request finding
MartinWheelerMT Jun 23, 2026
f6828a8
* Add additional tests to address surviving mutations.
MartinWheelerMT Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<given>` 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.

Expand Down
31 changes: 25 additions & 6 deletions ...-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,23 @@
}

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());
Expand All @@ -149,10 +155,23 @@
}
}

private static String getGivenNamesAsString(PN name) {
if (name == null) {

Check warning on line 159 in gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 159 without causing a test to fail 

removed conditional - replaced equality check with false (covered by 5 tests RemoveConditionalMutator_EQUAL_ELSE)
return null;

Check warning on line 160 in gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 160 without causing a test to fail 

replaced return value with "" for getGivenNamesAsString (no tests cover this line EmptyObjectReturnValsMutator)
}

var givenList = name.getGiven().stream()
.filter(StringUtils::isNotBlank)

Check warning on line 164 in gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 164 without causing a test to fail 

removed call to filter (covered by 5 tests RemoveFilterMutator)
.toList();

return givenList.isEmpty() ? null : StringUtils.join(givenList, " ");

Check warning on line 167 in gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 167 without causing a test to fail 

removed conditional - replaced equality check with false (covered by 5 tests RemoveConditionalMutator_EQUAL_ELSE)
}
Comment thread
Copilot marked this conversation as resolved.

private static boolean hasNoName(PN name) {
var hasGiven = name != null && name.getGiven() != null && !name.getGiven().isEmpty();

Check warning on line 171 in gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 171 without causing a test to fail 

removed conditional - replaced equality check with true (covered by 21 tests RemoveConditionalMutator_EQUAL_IF)
return name == null
|| (StringUtils.isBlank(name.getFamily())
&& StringUtils.isBlank(name.getGiven())
&& !hasGiven
&& StringUtils.isBlank(name.getPrefix()));
}
Comment thread
MartinWheelerMT marked this conversation as resolved.

Expand Down
11 changes: 4 additions & 7 deletions ...ranslator/src/main/java/uk/nhs/adaptors/pss/translator/util/ParticipantReferenceUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,16 @@ public static Map<String, Optional<Reference>> 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<String> getParticipant2Reference(RCMRMT030101UKEhrComposition ehrComposition) {
Expand Down
239 changes: 239 additions & 0 deletions ...nslator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,245 @@ public void When_MapAgentDirectoryOnlyAgentPersonWithNameElementWithOnlyGiven_Ex
);
}

@Test
public void When_MapAgentWithMultipleGivenAndFamilyName_Expect_AllGivenNamesInArray() {
var agentDirectoryXml = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
Comment thread
MartinWheelerMT marked this conversation as resolved.
<Agent classCode="AGNT">
<id root="CD8E40B3-6A3C-11F1-AE7C-00155D75C807"/>
<code code="704951000000106" codeSystem="2.16.840.1.113883.2.1.3.2.4.15" displayName="Other person"/>
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name use="L">
<given>Minire</given>
<given>E</given>
<family>Clarkson</family>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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)
);
}

@Test
public void When_MapAgentWithThreeGivenNames_Expect_AllThreeGivenNamesInArray() {
final var expectedGivenCount = 3;
var agentDirectoryXml = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
<Agent classCode="AGNT">
<id root="95D00D99-0601-4A8E-AD1D-1B564307B0A6" />
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name>
<given>John</given>
<given>Paul</given>
<given>George</given>
<family>Smith</family>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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(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"),
() -> assertThat(practitioner.getNameFirstRep().getText()).isNull()
);
}

@Test
public void When_MapAgentWithEmptyGivenNameInList_Expect_OnlyNonEmptyGivenNamesAdded() {
var agentDirectoryXml = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
<Agent classCode="AGNT">
<id root="95D00D99-0601-4A8E-AD1D-1B564307B0A6" />
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name>
<given>John</given>
<given></given>
<given>Paul</given>
<family>Smith</family>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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 = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
<Agent classCode="AGNT">
<id root="95D00D99-0601-4A8E-AD1D-1B564307B0A6" />
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name>
<prefix>Dr</prefix>
<given>John</given>
<given>Robert</given>
<family>Smith</family>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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_MapAgentWithPrefixButNullGivenNames_Expect_TextSetToPrefix() {
var agentDirectoryXml = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
<Agent classCode="AGNT">
<id root="95D00D99-0601-4A8E-AD1D-1B564307B0A6" />
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name>
<prefix>Prof</prefix>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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 = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
<Agent classCode="AGNT">
<id root="95D00D99-0601-4A8E-AD1D-1B564307B0A6" />
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name>
<given></given>
<given></given>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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 = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
<Agent classCode="AGNT">
<id root="95D00D99-0601-4A8E-AD1D-1B564307B0A6" />
<agentPerson classCode="PSN" determinerCode="INSTANCE">
<name>
<prefix>Dr</prefix>
<given></given>
<family>Smith</family>
</name>
</agentPerson>
</Agent>
</part>
</agentDirectory>""";
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 = """
Expand Down
38 changes: 33 additions & 5 deletions schema/src/main/java/org/hl7/v3/EN.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public class EN {
protected List<CsEntityNameUse> use;
protected String delimiter;
protected String family;
protected String given;
protected List<String> given;
protected String prefix;
protected String suffix;
protected IVLTS validTime;
Expand Down Expand Up @@ -120,12 +120,40 @@ public void setFamily(String family) {
this.family = family;
}

public String getGiven() {
return given;
/**
* Gets the value of the given property.
*
* <p>
* 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 <CODE>set</CODE> method for the given property.
*
* <p>
* For example, to add a new item, do as follows:
* <pre>
* getGiven().add(newItem);
* </pre>
*
* <p>
* Objects of the following type(s) are allowed in the list
* {@link String }
*/
public List<String> 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() {
Expand Down
Loading
Loading