Skip to content

VED-320: BugFixes-Delta-postcode#539

Merged
Akol125 merged 2 commits intomasterfrom
VED-320-Delta-Fix
May 29, 2025
Merged

VED-320: BugFixes-Delta-postcode#539
Akol125 merged 2 commits intomasterfrom
VED-320-Delta-Fix

Conversation

@Akol125
Copy link
Contributor

@Akol125 Akol125 commented May 28, 2025

Summary

  • Routine Change
  • ❗ Breaking Change
  • 🤖 Operational or Infrastructure Change
  • ✨ New Feature
  • ⚠️ Potential issues that might be caused by this change

Add any other relevant notes or explanations here. Remove this line if you have nothing to add.

Reviews Required

  • Dev
  • Test
  • Tech Author
  • Product Owner

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.

@Akol125 Akol125 changed the title fix address VED-320: Delta postcode field May 29, 2025
@sonarqubecloud
Copy link

@Akol125 Akol125 requested review from mfjarvis and nhsdevws May 29, 2025 00:32
@Akol125 Akol125 changed the title VED-320: Delta postcode field VED-320: BugFixes-Delta-postcode May 29, 2025
"- 'YYYY-MM-DDThh:mm:ss%z' — Full date and time with timezone (e.g. +00:00 or +01:00)\n"
"- 'YYYY-MM-DDThh:mm:ss.f%z' — Full date and time with milliseconds and timezone\n\n"
"Only '+00:00' and '+01:00' are accepted as valid timezone offsets.\n"
f"{field_location} must be a valid datetime in one of the following formats:"
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps format with JSON

"- 'YYYY-MM-DDThh:mm:ss.f%z' — Full date and time with milliseconds and timezone\n\n"
"Only '+00:00' and '+01:00' are accepted as valid timezone offsets.\n"
f"{field_location} must be a valid datetime in one of the following formats:"
"- 'YYYY-MM-DD' — Full date only"
Copy link
Contributor

Choose a reason for hiding this comment

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

Format with JSON


if not (valid_addresses := [
addr for addr in addresses
if addr.get("postalCode") and self._is_current_period(addr, occurrence_time)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto enum

@mfjarvis
Copy link
Contributor

Not sure I agree with use, type and postalCode being moved to an enum. By extension, wouldn't we end up with every field name we read from the FHIR message being added to some awful giant enum? 😅

@mfjarvis
Copy link
Contributor

The possible values of use and type, by all means, but not the field names themselves

@mfjarvis
Copy link
Contributor

If we wanted to enforce that we were only ever reading from valid FHIR fields, then I'd suggest using model classes like the ones provided by the fhir.resources package. But doing that would also add some avoidable overhead to something which needs to run fairly fast

@Akol125 Akol125 merged commit f90c55f into master May 29, 2025
8 checks passed
@Akol125 Akol125 deleted the VED-320-Delta-Fix branch May 29, 2025 11:34
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.

3 participants