Skip to content

VED-872 Tighten given name validation#915

Merged
dlzhry2nhs merged 3 commits intomasterfrom
bugfix/VED-872-improve-given-name-validation
Oct 20, 2025
Merged

VED-872 Tighten given name validation#915
dlzhry2nhs merged 3 commits intomasterfrom
bugfix/VED-872-improve-given-name-validation

Conversation

@dlzhry2nhs
Copy link
Collaborator

@dlzhry2nhs dlzhry2nhs commented Oct 17, 2025

Summary

  • Routine Change

Patient given name validation has been tightened up to align with PDS FHIR API validation. https://digital.nhs.uk/developer/api-catalogue/personal-demographics-service-fhir#post-/Patient

Namely:

  • Array of given names has max length 5
  • Each item must be a non-empty string of max length 35

By extension, the Practitioner given name also now is checked to ensure it is not a whitespace string. However, no further validation to do with the length is applied.

Check with testers: this change makes the given name validation error more specific by flagging which item in the array had an issue. Hopefully this is acceptable but needs checking and e2e tests may need updating accordingly.

Reviews Required

  • Dev

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • I have ensured the changelog has been updated by the submitter, if necessary.

@github-actions
Copy link
Contributor

This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket:

VED-872

if len(field_value) == 0:
raise ValueError(f"{field_location} must be a non-empty array")

if max_length:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This needs to be used carefully by us. Can add a docstring if you think useful, but when using this method you need to ensure you either use defined_length OR max_length. Though the code won't blow up, it does not really make sense to use both together, but nothing prevents you doing so.

([], "contained[?(@.resourceType=='Patient')].name[0].given must be a non-empty array"),
([""], "contained[?(@.resourceType=='Patient')].name[0].given[0] must be a non-empty string"),
([" \n"], "contained[?(@.resourceType=='Patient')].name[0].given[0] must be a non-empty string"),
(["Test", " "], "contained[?(@.resourceType=='Patient')].name[0].given[1] must be a non-empty string"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need to run past Krupa & Fatima. Have made a minor adjustment to the error message so that, when the error is to do with the element within the list, then we flag the index of the item with the problem.

@sonarqubecloud
Copy link

@dlzhry2nhs dlzhry2nhs temporarily deployed to internal-dev-sandbox October 20, 2025 11:15 — with GitHub Actions Inactive
@dlzhry2nhs dlzhry2nhs merged commit f703f91 into master Oct 20, 2025
21 of 25 checks passed
@dlzhry2nhs dlzhry2nhs deleted the bugfix/VED-872-improve-given-name-validation branch October 20, 2025 11:22
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