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.

Sign up for GitHub

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: Team Settings Form Fetching Boundary GeoJSON using Boundary URL #3378

Merged
merged 12 commits into from
Jul 8, 2024
Merged

feat: Team Settings Form Fetching Boundary GeoJSON using Boundary URL #3378

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8b6ff65
add boundary form validation and table permissions update
RODO94 Jul 5, 2024
0612245
add boundary form validation and table permissions update
RODO94 Jul 5, 2024
8744aa6
update error message on fail fetch:
RODO94 Jul 5, 2024
b75b3d5
fix regex statement in Yup validation
RODO94 Jul 5, 2024
34e5744
repointing boundarybbox from teams table to team_settings
RODO94 Jul 5, 2024
a3195e5
remove redundant imports
RODO94 Jul 5, 2024
6a1a0e9
adjust one error text
RODO94 Jul 5, 2024
b0cf22a
fix: Remove `@types/geojson` from root project
DafyddLlyr Jul 8, 2024
c4f1e3d
chore: Import turf modules, not full package
DafyddLlyr Jul 8, 2024
21cfa53
feat: Update regex and examples to restrict domain
DafyddLlyr Jul 8, 2024
74e7089
chore: Add types, use helper function for turf
DafyddLlyr Jul 8, 2024
7c1d9fb
fix: Handle AxiosError, use formik helper method
DafyddLlyr Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions editor.planx.uk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
"@tiptap/react": "^2.4.0",
"@tiptap/suggestion": "^2.0.3",
"@turf/area": "^7.0.0",
"@turf/bbox": "^7.0.0",
"@turf/bbox-polygon": "^7.0.0",
"@turf/buffer": "^7.0.0",
"@turf/helpers": "^7.0.0",
"array-move": "^4.0.0",
Expand Down
13 changes: 13 additions & 0 deletions editor.planx.uk/pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion editor.planx.uk/src/@planx/components/FindProperty/Public/Map.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {

const [environment, boundaryBBox] = useStore((state) => [
state.previewEnvironment,
state.boundaryBBox,
state.teamSettings.boundaryBbox,
]);

useEffect(() => {
Expand Down
37 changes: 28 additions & 9 deletions editor.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/BoundaryForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { bbox } from "@turf/bbox";
import { bboxPolygon } from "@turf/bbox-polygon";
import { feature } from "@turf/helpers";
import axios from "axios";
import { useFormik } from "formik";
import { useStore } from "pages/FlowEditor/lib/store";
import React, { ChangeEvent } from "react";
Expand All @@ -9,10 +13,14 @@ import { SettingsForm } from "../shared/SettingsForm";
import { FormProps } from ".";

export default function BoundaryForm({ formikConfig, onSuccess }: FormProps) {
const planningDataURLRegex =
/^https:\/\/www\.planning\.data\.gov\.uk\/entity\/\d{1,7}$/;

const formSchema = Yup.object().shape({
boundaryUrl: Yup.string()
.url(
"Enter a boundary URL in the correct format, https://www.planning.data.gov.uk/",
.matches(
planningDataURLRegex,
"Enter a boundary URL in the correct format, https://www.planning.data.gov.uk/entity/1234567",
)
.required("Enter a boundary URL"),
});
Expand All @@ -21,12 +29,23 @@ export default function BoundaryForm({ formikConfig, onSuccess }: FormProps) {
...formikConfig,
validationSchema: formSchema,
onSubmit: async (values, { resetForm }) => {
const isSuccess = await useStore.getState().updateTeamSettings({
boundaryUrl: values.boundaryUrl,
});
if (isSuccess) {
onSuccess();
resetForm({ values });
try {
const { data } = await axios.get(`${values.boundaryUrl}.geojson`);
const bboxPoly = bboxPolygon(bbox(data));
const bboxFeature = feature(bboxPoly.geometry);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not quite sure if this is the simplest way to achieve this. If we add types and maybe wrap this in a descriptive helper function it may help clear this up.

Copy link
Contributor

@DafyddLlyr DafyddLlyr Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Addressed here - 74e7089

The confusion may have come from the fact that there's a bbox property on the generated GeoJSON feature, which sort of looks wrong if you're not expecting it. This is actually fine and is an optional part of the GeoJSON spec (docs).

image


const isUpdateSuccess = await useStore.getState().updateTeamSettings({
boundaryUrl: values.boundaryUrl,
boundaryBbox: bboxFeature,
});
if (isUpdateSuccess) {
onSuccess();
resetForm({ values });
}
} catch (error) {
formik.errors.boundaryUrl =
"We are unable to retrieve your boundary, check your boundary URL and try again";
console.error(error);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good!

We might want slightly more fine-grained checking of the error type (i.e. using isAxiosError()) or a bit more control over the URL we send the request to.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed here - 7c1d9fb

I've also used the helper method setFieldError() as there may be a side effect of directly modifying the errors object (docs).

}
},
});
Expand All @@ -49,7 +68,7 @@ export default function BoundaryForm({ formikConfig, onSuccess }: FormProps) {
target="_blank"
rel="noopener noreferrer"
>
https://www.planning.data.gov.uk/
https://www.planning.data.gov.uk/entity/1234567
</a>
</p>
</>
Expand Down
4 changes: 2 additions & 2 deletions editor.planx.uk/src/pages/FlowEditor/components/Settings/GeneralSettings/ContactForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ export default function ContactForm({ formikConfig, onSuccess }: FormProps) {
.email(
"Enter an email address in the correct format, like [email protected]",
)
.required("Enter a help email address"),
helpPhone: Yup.string().required("Enter a help phone number"),
.required("Enter a contact email address"),
helpPhone: Yup.string().required("Enter a phone number"),
helpOpeningHours: Yup.string().required("Enter your opening hours"),
homepage: Yup.string()
.url(
Expand Down
5 changes: 0 additions & 5 deletions editor.planx.uk/src/pages/FlowEditor/lib/store/team.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type { StateCreator } from "zustand";
import { SharedStore } from "./shared";

export interface TeamStore {
boundaryBBox?: Team["boundaryBBox"];
teamId: number;
teamIntegrations: TeamIntegrations;
teamName: string;
Expand All @@ -34,7 +33,6 @@ export const teamStore: StateCreator<
[],
TeamStore
> = (set, get) => ({
boundaryBBox: undefined,
teamId: 0,
teamIntegrations: {} as TeamIntegrations,
teamName: "",
Expand All @@ -44,7 +42,6 @@ export const teamStore: StateCreator<

setTeam: (team) => {
set({
boundaryBBox: team.boundaryBBox,
teamId: team.id,
teamIntegrations: team.integrations,
teamName: team.name,
Expand All @@ -60,7 +57,6 @@ export const teamStore: StateCreator<
},

getTeam: () => ({
boundaryBBox: get().boundaryBBox,
id: get().teamId,
integrations: get().teamIntegrations,
name: get().teamName,
Expand Down Expand Up @@ -105,7 +101,6 @@ export const teamStore: StateCreator<

clearTeamStore: () =>
set({
boundaryBBox: undefined,
teamId: 0,
teamIntegrations: undefined,
teamName: "",
Expand Down
2 changes: 1 addition & 1 deletion editor.planx.uk/src/routes/views/draft.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const fetchSettingsForDraftView = async (
name
settings: team_settings {
boundaryUrl: boundary_url
boundaryBBox: boundary_bbox
homepage
helpEmail: help_email
helpPhone: help_phone
Expand All @@ -81,7 +82,6 @@ const fetchSettingsForDraftView = async (
hasPlanningData: has_planning_data
}
slug
boundaryBBox: boundary_bbox
}
settings
slug
Expand Down
3 changes: 1 addition & 2 deletions editor.planx.uk/src/routes/views/published.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export const fetchSettingsForPublishedView = async (
name
settings: team_settings {
boundaryUrl: boundary_url
boundaryBBox: boundary_bbox
homepage
helpEmail: help_email
helpPhone: help_phone
Expand All @@ -107,8 +108,6 @@ export const fetchSettingsForPublishedView = async (
hasPlanningData: has_planning_data
}
slug

boundaryBBox: boundary_bbox
}
settings
status
Expand Down
2 changes: 1 addition & 1 deletion editor.planx.uk/src/routes/views/standalone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const fetchDataForStandaloneView = async (
name
settings: team_settings {
boundaryUrl: boundary_url
boundaryBBox: boundary_bbox
homepage
helpEmail: help_email
helpPhone: help_phone
Expand All @@ -81,7 +82,6 @@ const fetchDataForStandaloneView = async (
hasPlanningData: has_planning_data
}
slug
boundaryBBox: boundary_bbox
}
settings
}
Expand Down
2 changes: 2 additions & 0 deletions hasura.planx.uk/metadata/tables.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1729,6 +1729,7 @@
- role: platformAdmin
permission:
columns:
- boundary_bbox
- boundary_url
- email_reply_to_id
- external_planning_site_name
Expand All @@ -1744,6 +1745,7 @@
- role: teamEditor
permission:
columns:
- boundary_bbox
- boundary_url
- email_reply_to_id
- external_planning_site_name
Expand Down
Loading