Skip to content

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

Open
MartinWheelerMT wants to merge 6 commits into
mainfrom
niad-3471-support-multiple-practitioner-given-names
Open

NIAD-3471 Support multiple given names in practitioner XML#1292
MartinWheelerMT wants to merge 6 commits into
mainfrom
niad-3471-support-multiple-practitioner-given-names

Conversation

@MartinWheelerMT

@MartinWheelerMT MartinWheelerMT commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

What

  • 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).

Why

The adapter was unable to handle XML containing multiple fields for practitioner names. When an XML part contained multiple given names like:

<name use="L">
    <given>Minire</given>
    <given>E</given>
    <family>Clarkson</family>
</name>

The JSON output would only include a single given name:

{
    "name": [{
        "use": "official",
        "family": "Clarkson",
        "given": ["E"]  // Only one given name captured
    }]
}

Root Cause

The JAXB-generated schema classes in org.hl7.v3.EN (which PN extends) only supported a single String field for given names:

protected String given;

This meant when the XML was unmarshalled, JAXB could only capture one element, even though the XML schema (datatypes.xsd) defines the name parts as a element with maxOccurs="unbounded", allowing multiple instances.

Solution

1. Modified EN Class (schema/src/main/java/org/hl7/v3/EN.java)

Changed the given field to support multiple values:

// Before:
protected String given;

// After:
protected List<String> given;

Updated the getter method to return a List:

public List<String> getGiven() {
    if (given == null) {
        given = new ArrayList<>();
    }
    return this.given;
}

Added a convenience method for backward compatibility:

public String getFirstGiven() {
    if (given != null && !given.isEmpty()) {
        return given.get(0);
    }
    return null;
}

2. Updated AgentDirectoryMapper (gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java)

Modified the mapper to iterate over all given names and add them to the FHIR HumanName:

private void setHumanNameValuesFromName(PN name, HumanName humanName) {
    humanName.setFamily(name.getFamily());

    // Handle multiple given names
    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());
    if (prefix != null) {
        humanName.getPrefix().add(prefix);
    }
}

Added a helper method to concatenate given names for the text field when there's no family name:

private static String getGivenNamesAsString(PN name) {
    var givenList = name.getGiven();
    if (givenList != null && !givenList.isEmpty()) {
        return StringUtils.join(givenList, " ");
    }
    return null;
}

3. Updated Tests

Updated JaxbTest (schema/src/test/java/JaxbTest.java)

Changed the test to check for a list of given names:

// Before:
assertThat(person.getName().getGiven()).isEqualTo("Peter");

// After:
assertThat(person.getName().getGiven()).hasSize(1);
assertThat(person.getName().getGiven().getFirst()).isEqualTo("Peter");

Updated and Added Tests in AgentDirectoryMapperTest

Updated the test When_MapAgentDirectoryOnlyAgentPersonWithNameElementWithOnlyGiven_Expect_TextSetToGiven() to correctly test the scenario with both multiple given names and a family name.

Added new test When_MapAgentDirectoryOnlyAgentPersonWithMultipleGivenNamesNoFamily_Expect_TextSetToGiven() to test the case where there are multiple given names but no family name.

Added new test When_MapAgentWithMultipleGivenAndFamilyName_Expect_AllGivenNamesInArray() to test the exact user scenario with ID and all assertions.

Expected Output After Fix

For the XML example provided:

<part typeCode="PART">
    <Agent classCode="AGNT">
        <id root="CD8E40B3-6A3C-11F1-AE7C-00155D75C807"/>
        <agentPerson classCode="PSN" determinerCode="INSTANCE">
            <name use="L">
                <given>Minire</given>
                <given>E</given>
                <family>Clarkson</family>
            </name>
        </agentPerson>
    </Agent>
</part>

The JSON output is now:

{
    "resource": {
        "resourceType": "Practitioner",
        "id": "CD8E40B3-6A3C-11F1-AE7C-00155D75C807",
        "meta": {
            "profile": ["https://fhir.nhs.uk/STU3/StructureDefinition/CareConnect-GPC-Practitioner-1"]
        },
        "name": [{
            "use": "official",
            "family": "Clarkson",
            "given": ["Minire", "E"]
        }]
    }
}

The changes are backward compatible:

  • Single given names still work as before
  • The convenience method getFirstGiven() provides access to the first given name
  • All existing tests pass without modification (except for the assertion adjustment to work with Lists)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Internal change (non-breaking change with no effect on the functionality affecting end users)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the Changelog with details of my change in the UNRELEASED section if this change will affect end users
  • A corresponding change has been made to the Mapping Documentation repository

Co-authored-by: GitHub Copilot noreply@github.com

- Changed EN.given field from String to List<String> 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<String> given names
- Added convenience method getFirstGiven() for backward compatibility
- Added unit tests for multiple given names scenarios (with and without family name).
@MartinWheelerMT MartinWheelerMT requested a review from a team as a code owner June 23, 2026 15:20
@MartinWheelerMT MartinWheelerMT enabled auto-merge (squash) June 23, 2026 15:22
@MartinWheelerMT MartinWheelerMT disabled auto-merge June 23, 2026 15:34
@MartinWheelerMT MartinWheelerMT enabled auto-merge (squash) June 23, 2026 15:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the HL7 v3 JAXB schema model and translator mapping so practitioner names can capture multiple <given> elements and map them into FHIR HumanName.given correctly.

Changes:

  • Updated org.hl7.v3.EN to store given as List<String> (instead of a single String) and adjusted schema unmarshalling test expectations.
  • Updated AgentDirectoryMapper to add all given names into HumanName.given, and to build the text field from concatenated given names when there is no family name.
  • Added/updated unit tests for multi-given scenarios; refactored a participant reference helper; added a changelog entry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
schema/src/main/java/org/hl7/v3/EN.java Changes JAXB model to support multiple given names via List<String>.
schema/src/test/java/JaxbTest.java Updates assertions to match the new list-based given model.
gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapper.java Maps multiple given names into FHIR and concatenates given names for HumanName.text.
gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/mapper/AgentDirectoryMapperTest.java Adds tests for multiple given names and related scenarios.
gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/util/ParticipantReferenceUtil.java Refactors participant2 reference mapping to a fluent Optional chain.
CHANGELOG.md Documents the multiple-given-name mapping fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 166 to 172
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()));
}
Comment on lines +255 to +259
@Test
public void When_MapAgentWithMultipleGivenAndFamilyName_Expect_AllGivenNamesInArray() {
var agentDirectoryXml = """
<agentDirectory xmlns="urn:hl7-org:v3" classCode="AGNT">
<part typeCode="PART">
MartinWheelerMT and others added 2 commits June 23, 2026 17:09
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
  • Surviving mutants in this change: 5
  • Killed mutants in this change: 21
class surviving killed
🧟uk.nhs.adaptors.pss.translator.mapper.AgentDirectoryMapper 5 17
💯uk.nhs.adaptors.pss.translator.util.ParticipantReferenceUtil 0 4

See https://pitest.org

@github-actions

Copy link
Copy Markdown

Images built and published to ECR using a Build Id of PR-1159-d5c5092

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants