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: optimise planning constraints requests #3740

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Sep 30, 2024

Joann at Gloucester is still reporting partial GIS queries: https://opendigitalplanning.slack.com/archives/C05NQJK5S9L/p1726495149134779

Double-checking a few of our configs here to see if there's anything we've missed:

  • Can we filter Planning Data metadata response fields similar to entity?
  • Can we optimise /roads OGC query?
    • Confirmed this is working as expected, there's not much way around a slow string-matching USRN query! Shared details with Steve at Planning Data about if/how roads might be a candidate for adding to the Planning Data platform in the future which could simplify a bit (eg consistent performance/caching/uptime, but still need to handle non-geospatial query method!)
  • Is loading logic correct throughout? Are we properly handling scenarios where one of two requests errors (ui feedback & passport vars)?
    • Again, properly looked this over & we're doing exactly what we want! If one of the two requests fails, you only see results from the other and any "failed" variables are not added to passport at all, therefore prompting users to manually see questions

Only changes:

  • Split up components in Public.tsx & renamed PlanningConstraintsContentPresentational as this is a more established pattern elsewhere among frontend components

Copy link

github-actions bot commented Sep 30, 2024

Removed vultr server and associated DNS entries

@jessicamcinchak jessicamcinchak marked this pull request as ready for review October 2, 2024 08:30
@jessicamcinchak jessicamcinchak requested a review from a team October 2, 2024 08:30
Copy link
Contributor

@RODO94 RODO94 left a comment

Choose a reason for hiding this comment

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

@jessicamcinchak trying to track and follow the code here. Are most changes on moving the jsx content to Presentational.tsx file from the Public.tsx file?

I see big diff around the handleSubmit() but it looks mostly like it has only changed position in the code rather than any logical changes?

Should I be testing any logic?

@jessicamcinchak
Copy link
Member Author

@RODO94 sorry for confusion - this ended up being a pure quick refactor for readability/smaller component files, no logic changes

Copy link
Contributor

@RODO94 RODO94 left a comment

Choose a reason for hiding this comment

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

@jessicamcinchak thanks for clarifying, looks good to me. Thanks for picking up all the comms on this with Planning Data

@jessicamcinchak jessicamcinchak merged commit 55dc78b into main Oct 2, 2024
12 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/optimise-constraints-requests branch October 2, 2024 15:46
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