-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: send BOPS enhanced constraints data and update PlanningConstraint breadcrumb shape #1827
Conversation
Removed vultr server and associated DNS entries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small question!
(useStore( | ||
(state) => state.computePassport().data?._address | ||
) as SiteAddress) || {}; | ||
const showGraphError = !x && !y && !longitude && !latitude; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We show the graph error if all of these are missing - should it be if any of them are missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair question - I think you're right that ||
makes more sense than &&
here even though these fields are all required on our minimum address type. Either case should still only happen when: 1) planning constraints isn't positioned after find property, or 2) there's been an error in OS source data 👌
Relies on theopensystemslab/planx-core#46
Planning constraints breadcrumb data before:
fn: [...intersectingPassportValues]
_nots: { fn: [...nonIntersectingPassportValues] }
digitalLandRequest: https://planning.data.gov.uk...
- never used afaik, just available as referenceAfter:
fn: [...intersectingPassportValues]
_nots: { fn: [...nonIntersectingPassportValues] }
upcomingCardIds()
still and I was about to easily lose an afternoon untangling. So I'll pick it up in another PR as it's no longer used to format what we send to BOPS (seeplanx-core
change), but only referenced internally for automations_constraints: [{...gisResponse}, {...roadsResponse}]
prop.fn
was set to, but I don't think that matters for BOPS or other end consumers?Testing with BOPS:
constraints
key as normalconstraints_proposed
key with_constraints
for comparison / review - I think this can be a good first trial of testing the usefulness of our raw internal response with an end consumer in the spirit of them pulling "full fat" data one dayInternal testing: