Skip to content
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.

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

chore: extend SiteAddress type to account for ranges #3670

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Sep 13, 2024

It came up in Idox feedback that we aren't currently supporting address ranges in the ODP Schema. I looked through the OneApp JSON example we have and don't see any instances of it handling ranges either, but nonetheless it's low effort for us to start more carefully checking for these.

So, this PR adds a few extra fields from the OS Places API "LPI" data source to our internal model which we can later decide how to send along in payloads / the ODP Schema (theopensystemslab/digital-planning-data-schemas#236).

Only additive changes here!

  • saoEnd
  • paoEnd
  • ward
  • parish

Links:

selectedAddress.SAO_TEXT, // populated in cases of building name only, no street number
]
.filter(Boolean)
.join(""),
Copy link
Member Author

Choose a reason for hiding this comment

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

This updates existing sao field to be handled same as pao, whereas previously we only accounted for SAO_TEXT and would miss SAO_START_NUMBER or SAO_START_SUFFIX

street?: string;
town?: string;
postcode?: string;
ward?: string;
parish?: string;
Copy link
Member Author

@jessicamcinchak jessicamcinchak Sep 13, 2024

Choose a reason for hiding this comment

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

I had to refresh my memory here, so writing it down here too.

These are all (unideally) optional type properties for two reasons:

  • This component shares a single setAddress set state action for both pages (selecting an existing address
    & proposing a new one), despite those pages fundamentally requiring/setting different types. There's definitely a future refactor in here, but bigger and more slippery than I'd like to deal with right now!
  • OS Docs are quite confusing to make sense of about what's required versus not sometimes - for example, PARISH_CODE has a "multiplicity" of 1 (not 0..1) which I understand to mean exactly one parish should always be associated with & returned for a given address, yet it's not found in the example response linked above at all

Copy link
Contributor

Choose a reason for hiding this comment

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

Helpful thanks!

Copy link

github-actions bot commented Sep 13, 2024

Removed vultr server and associated DNS entries

@jessicamcinchak jessicamcinchak force-pushed the jess/update-address-model-ranges branch from a392a22 to 3719ed7 Compare September 13, 2024 14:50
@jessicamcinchak jessicamcinchak requested a review from a team September 13, 2024 14:54
@jessicamcinchak jessicamcinchak marked this pull request as ready for review September 13, 2024 14:54
street?: string;
town?: string;
postcode?: string;
ward?: string;
parish?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Helpful thanks!

@jessicamcinchak jessicamcinchak merged commit a43cdc8 into main Sep 16, 2024
22 checks passed
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