VED-898: Refactor RecordAttributes so it handles Immunization FHIR object rather than a dictionary#1259
Merged
Thomas-Boyle merged 8 commits intomasterfrom Mar 4, 2026
Merged
Conversation
…ed methods in FhirService and tests. This change enhances type safety and improves the handling of immunization data throughout the repository and service layers.
…hir-api into 898-refactor-recordattributes
EmptyCommit:
Thomas-Boyle
had a problem deploying
to
internal-dev-sandbox
GitHub Actions
Failure
Thomas-Boyle
had a problem deploying
to
internal-dev
GitHub Actions
Failure
Thomas-Boyle
had a problem deploying
to
internal-dev-sandbox
GitHub Actions
Failure
Thomas-Boyle
had a problem deploying
to
internal-dev
GitHub Actions
Failure
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
edhall-nhs
reviewed
Mar 3, 2026
Contributor
edhall-nhs
left a comment
edhall-nhs
left a comment
There was a problem hiding this comment.
Looks good. Only one comment about naming which I think needs addressing. See other optional comments about further refactoring.
| """Build DynamoDB attributes from a FHIR Immunization resource.""" | ||
| imms_dict = immunization.dict() | ||
| patient_resolved = patient if patient is not None else get_contained_patient(imms_dict) | ||
| nhs_number = get_nhs_number(imms_dict) |
Contributor
There was a problem hiding this comment.
Like mentioned in this comment, could probably be a separate ticket to refactor all these helpers so they use the Immunization object
…y and enhance get_vaccine_type to accept Immunization model. Update references accordingly.
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Akol125
approved these changes
Mar 3, 2026
Contributor
Akol125
left a comment
Akol125
left a comment
There was a problem hiding this comment.
Looks good to me, also like the change from self. to cls() in the from_immunization function.
dlzhry2nhs
approved these changes
Mar 4, 2026
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
|
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev-sandbox
GitHub Actions
Inactive
Thomas-Boyle
temporarily deployed
to
internal-dev
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RecordAttributesin the FHIR repository now works with FHIRImmunizationentities instead of plain dicts; key derivation (PK, PatientPK, PatientSK, IdentifierPK) is unchanged.ImmunizationRepository.create_immunizationandupdate_immunizationnow takeImmunizationobjects and persist them viaimmunization.json(use_decimal=True).FhirService.update_immunizationparses both the incoming payload and the existing stored resource intoImmunizationentities and passes the new entity to the repo.External API, validation, authorisation, versioning, and DynamoDB schema are unchanged.