diff --git a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts index 1b66e90a88..4b9829dd69 100644 --- a/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts +++ b/api.planx.uk/modules/analytics/metabase/collection/collection.test.ts @@ -1 +1,92 @@ -test.todo("should test collection check and creation"); +import { $api } from "../../../../client/index.js"; +import { updateMetabaseId } from "./updateMetabaseId.js"; +import { getTeamIdAndMetabaseId } from "./getTeamIdAndMetabaseId.js"; + +describe("getTeamIdAndMetabaseId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("successfully gets team and existing metabase id", async () => { + vi.spyOn($api.client, "request").mockResolvedValue({ + teams: [ + { + id: 26, + slug: "barnet", + metabaseId: 20, + }, + ], + }); + + const teamAndMetabaseId = await getTeamIdAndMetabaseId("barnet"); + + expect(teamAndMetabaseId.id).toEqual(26); + expect(teamAndMetabaseId.metabaseId).toEqual(20); + }); + + test("handles team with null metabase id", async () => { + vi.spyOn($api.client, "request").mockResolvedValue({ + teams: [ + { + id: 26, + slug: "barnet", + metabaseId: null, + }, + ], + }); + + const teamAndMetabaseId = await getTeamIdAndMetabaseId("Barnet"); + + expect(teamAndMetabaseId.id).toEqual(26); + expect(teamAndMetabaseId.metabaseId).toBeNull(); + }); +}); + +describe("updateMetabaseId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("successfully updates metabase ID", async () => { + // Mock the GraphQL request + vi.spyOn($api.client, "request").mockResolvedValue({ + update_teams: { + returning: [ + { + id: 1, + slug: "testteam", + metabase_id: 123, + }, + ], + }, + }); + + const result = await updateMetabaseId(1, 123); + + expect(result).toEqual({ + update_teams: { + returning: [ + { + id: 1, + slug: "testteam", + metabase_id: 123, + }, + ], + }, + }); + + expect($api.client.request).toHaveBeenCalledWith(expect.any(String), { + id: 1, + metabaseId: 123, + }); + }); + + test("handles GraphQL error", async () => { + // Mock a failed GraphQL request + vi.spyOn($api.client, "request").mockRejectedValue( + new Error("GraphQL error"), + ); + + await expect(updateMetabaseId(1, 123)).rejects.toThrow("GraphQL error"); + }); +}); diff --git a/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts new file mode 100644 index 0000000000..4007bfef64 --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/getTeamIdAndMetabaseId.ts @@ -0,0 +1,38 @@ +import { gql } from "graphql-request"; +import { $api } from "../../../../client/index.js"; + +interface GetMetabaseId { + teams: { + id: number; + slug: string; + metabaseId: number | null; + }[]; +} + +export const getTeamIdAndMetabaseId = async (slug: string) => { + try { + const response = await $api.client.request( + gql` + query GetTeamAndMetabaseId($slug: String!) { + teams(where: { slug: { _eq: $slug } }) { + id + slug + metabaseId: metabase_id + } + } + `, + { + slug: slug, + }, + ); + + const result = response.teams[0]; + return result; + } catch (e) { + console.error( + "Error fetching team's ID / Metabase ID from PlanX DB:", + (e as Error).stack, + ); + throw e; + } +}; diff --git a/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts new file mode 100644 index 0000000000..93ffe1042d --- /dev/null +++ b/api.planx.uk/modules/analytics/metabase/collection/updateMetabaseId.ts @@ -0,0 +1,43 @@ +import { gql } from "graphql-request"; +import { $api } from "../../../../client/index.js"; + +interface UpdateMetabaseId { + teams: { + id: number; + slug: string; + metabaseId: number; + }; +} + +/** Updates column `metabase_id` in the Planx DB `teams` table */ +export const updateMetabaseId = async (teamId: number, metabaseId: number) => { + try { + const response = await $api.client.request( + gql` + mutation UpdateTeamMetabaseId($id: Int!, $metabaseId: Int!) { + update_teams( + where: { id: { _eq: $id } } + _set: { metabase_id: $metabaseId } + ) { + returning { + id + slug + metabase_id + } + } + } + `, + { + id: teamId, + metabaseId: metabaseId, + }, + ); + return response; + } catch (e) { + console.error( + "There's been an error while updating the Metabase ID for this team", + (e as Error).stack, + ); + throw e; + } +}; diff --git a/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts b/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts index 8bc6fe181b..02ecfbc29e 100644 --- a/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts +++ b/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts @@ -19,11 +19,24 @@ import { } from "./helpers/navigateAndPublish"; import { TestContext } from "./helpers/types"; import { serviceProps } from "./helpers/serviceData"; -import { checkGeoJsonContent } from "./helpers/geospatialChecks"; import { - mockMapGeoJson, + alterDrawGeoJson, + checkGeoJsonContent, + checkUploadFileAltRoute, + getMapProperties, + resetMapBoundary, + waitForMapComponent, +} from "./helpers/geospatialChecks"; +import { + GeoJsonChangeHandler, + mockChangedMapGeoJson, mockPropertyTypeOptions, + mockTitleBoundaryGeoJson, } from "./mocks/geospatialMocks"; +import { + setupOSMapsStyles, + setupOSMapsVectorTiles, +} from "./mocks/osMapsResponse"; test.describe("Flow creation, publish and preview", () => { let context: TestContext = { @@ -72,8 +85,7 @@ test.describe("Flow creation, publish and preview", () => { await editor.createInternalPortal(); await editor.populateInternalPortal(); await page.getByRole("link", { name: "start" }).click(); // return to main flow - await editor.createUploadAndLabel(); - // TODO: editor.createPropertyInfo() + // await editor.createUploadAndLabel(); await editor.createDrawBoundary(); await editor.createPlanningConstraints(); // await editor.createFileUpload(); @@ -81,7 +93,6 @@ test.describe("Flow creation, publish and preview", () => { await expect(editor.nodeList).toContainText([ "Find property", "an internal portalEdit Portal", - "Upload and label", "Confirm your location plan", "Planning constraints", // "File upload", @@ -119,6 +130,9 @@ test.describe("Flow creation, publish and preview", () => { `/${context.team.slug}/${serviceProps.slug}/published?analytics=false`, ); + setupOSMapsStyles(page); + setupOSMapsVectorTiles(page); + await expect( page.locator("h1", { hasText: "Find the property" }), ).toBeVisible(); @@ -130,7 +144,7 @@ test.describe("Flow creation, publish and preview", () => { ).toBeVisible(); // Check map component has geoJson content - await checkGeoJsonContent(page, mockMapGeoJson); + await checkGeoJsonContent(page, "geojsondata", mockTitleBoundaryGeoJson); // Check property info is being shown await expect(page.getByText("Test Street, Testville")).toBeVisible(); @@ -169,7 +183,49 @@ test.describe("Flow creation, publish and preview", () => { ).toBeVisible(); await clickContinue({ page }); + const drawBoundaryTitle = page.getByRole("heading", { + name: "Confirm your location plan", + }); + await expect(drawBoundaryTitle).toBeVisible(); + + await checkGeoJsonContent( + page, + "drawgeojsondata", + mockTitleBoundaryGeoJson, + ); + + const area = "The property boundary you have drawn is 490.37"; + + await expect(page.getByText(area)).toBeVisible(); + + // navigate to upload file page and back + await checkUploadFileAltRoute(page); + + await expect( + drawBoundaryTitle, + "We have navigated back to the map component", + ).toBeVisible(); + + // ensure map has loaded correctly + await waitForMapComponent(page); + + await resetMapBoundary(page); + + await alterDrawGeoJson(page); + + // extract new GeoJSON data + const newGeoJson = await getMapProperties(page, "drawgeojsondata"); + const parsedJson: GeoJsonChangeHandler = JSON.parse(newGeoJson!); + + // check it matches our static mock + await checkGeoJsonContent(page, "drawgeojsondata", mockChangedMapGeoJson); + + await expect( + page.getByText(`${parsedJson.properties!["area.squareMetres"]}`), + "The correct value for area comes from the map properties ", + ).toBeVisible(); + // TODO: answer uploadAndLabel - // TODO: answerPropertyInfo, answerDrawBoundary, answerPlanningConstraints + // TODO: answerPropertyInfo, answerPlanningConstraints }); }); diff --git a/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts b/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts index 7b55e01997..ea13894975 100644 --- a/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts +++ b/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts @@ -1,12 +1,87 @@ import { expect, Page } from "@playwright/test"; import { Feature } from "geojson"; -export const checkGeoJsonContent = async (page: Page, geoJson: Feature) => { +export const waitForMapComponent = async (page: Page) => { + await page.waitForFunction(() => { + const map = document.getElementById("draw-boundary-map"); + return map; + }); +}; + +export const getMapProperties = async ( + page: Page, + attribute: "geojsondata" | "drawgeojsondata", +) => { + const mapComponent = await page.waitForSelector("my-map"); + return await mapComponent.getAttribute(attribute); +}; + +export const alterDrawGeoJson = async (page: Page) => { + const map = page.getByTestId("map-test-id"); + + await map.click({ button: "left", position: { x: 100, y: 200 } }); + await map.click({ button: "left", position: { x: 150, y: 250 } }); + await map.click({ button: "left", position: { x: 200, y: 250 } }); + await map.click({ button: "left", position: { x: 100, y: 200 } }); +}; + +export const resetMapBoundary = async (page: Page) => { + const resetButton = page.getByLabel("Reset map view"); + await resetButton.click(); + + const resetGeoJson = await getMapProperties(page, "drawgeojsondata"); + + expect(resetGeoJson, "drawGeoJsonData should be reset").toEqual(null); +}; + +export const checkGeoJsonContent = async ( + page: Page, + attribute: "geojsondata" | "drawgeojsondata", + geoJson: Feature, +) => { // Wait for the map component to be present const mapComponent = await page.waitForSelector("my-map"); + await page.waitForFunction(() => customElements.get("my-map")); + await expect( + page.getByTestId("map-test-id"), + "Check we can see the map", + ).toBeVisible(); + // Get the geojsonData attribute - const geojsonData = await mapComponent.getAttribute("geojsondata"); + const geojsonData = await mapComponent.getAttribute(attribute); + + expect( + JSON.parse(geojsonData!), + "map attribute matches expected mock attribute", + ).toEqual(geoJson); +}; + +export const checkUploadFileAltRoute = async (page: Page) => { + const uploadButton = page.getByTestId("upload-file-button"); + + await expect( + uploadButton, + "We can see a button to upload a file instead", + ).toBeVisible(); + + await uploadButton.click(); + + await expect( + page.getByRole("heading", { name: "Upload a location plan" }), + "Should be in a page for uploading a file", + ).toBeVisible(); + + await expect( + page.getByRole("button", { name: "Drop file here or choose" }), + "A button for uploading files is visible", + ).toBeVisible(); + + await page.getByTestId("continue-button").click(); + + await page.getByTestId("error-message-upload-location-plan").isVisible(); + + const useMapButton = page.getByTestId("use-map-button"); - expect(JSON.parse(geojsonData!)).toEqual(geoJson); + await useMapButton.click(); }; diff --git a/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts b/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts index fb26308b1d..e923545f46 100644 --- a/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts +++ b/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts @@ -1,13 +1,21 @@ import { OptionWithDataValues } from "../helpers/types"; +import { Feature, Polygon } from "geojson"; + +type ChangeHandlerProperties = { + label: string; + "area.squareMetres": number; + "area.hectares": number; +}; + +export type GeoJsonChangeHandler = Feature; export const mockPropertyTypeOptions: OptionWithDataValues[] = [ { optionText: "Residential", dataValue: "residential" }, { optionText: "Commercial", dataValue: "commercial" }, ]; -import { Feature } from "geojson"; - -export const mockMapGeoJson: Feature = { +export const mockTitleBoundaryGeoJson: Feature = { + type: "Feature", geometry: { type: "MultiPolygon", coordinates: [ @@ -23,7 +31,6 @@ export const mockMapGeoJson: Feature = { ], ], }, - type: "Feature", properties: { "entry-date": "2024-05-06", "start-date": "2010-05-12", @@ -37,3 +44,23 @@ export const mockMapGeoJson: Feature = { "organisation-entity": "13", }, }; + +export const mockChangedMapGeoJson: GeoJsonChangeHandler = { + type: "Feature", + geometry: { + type: "Polygon", + coordinates: [ + [ + [-0.6341888375038146, 51.60562241658701], + [-0.6341217822784424, 51.605580770520504], + [-0.63405472705307, 51.605580770520504], + [-0.6341888375038146, 51.60562241658701], + ], + ], + }, + properties: { + label: "1", + "area.squareMetres": 10.72, + "area.hectares": 0.001072, + }, +}; diff --git a/e2e/tests/ui-driven/src/mocks/osMapsMockData.ts b/e2e/tests/ui-driven/src/mocks/osMapsMockData.ts new file mode 100644 index 0000000000..b99f14f4d3 --- /dev/null +++ b/e2e/tests/ui-driven/src/mocks/osMapsMockData.ts @@ -0,0 +1,22 @@ +export const osMapsStylesResponse = { + version: 8, + sprite: + "https://api.os.uk/maps/vector/v1/vts/resources/sprites/sprite?key=YOUR_KEY&srs=3857", + glyphs: + "https://api.os.uk/maps/vector/v1/vts/resources/fonts/{fontstack}/{range}.pbf?key=YOUR_KEY&srs=3857", + sources: { + esri: { + type: "vector", + url: "https://api.os.uk/maps/vector/v1/vts?key=YOUR_KEY&srs=3857", + }, + }, + layers: [ + { + id: "background", + type: "background", + paint: { + "background-color": "#0437F2", + }, + }, + ], +}; diff --git a/e2e/tests/ui-driven/src/mocks/osMapsResponse.ts b/e2e/tests/ui-driven/src/mocks/osMapsResponse.ts new file mode 100644 index 0000000000..ccd6f6de6f --- /dev/null +++ b/e2e/tests/ui-driven/src/mocks/osMapsResponse.ts @@ -0,0 +1,27 @@ +import { Page } from "@playwright/test"; +import { osMapsStylesResponse } from "./osMapsMockData"; + +export async function setupOSMapsStyles(page: Page) { + const ordnanceSurveyMapsStyles = new RegExp( + /\/proxy\/ordnance-survey\/maps\/vector\/v1\/vts\/resources\/styles.*/, + ); + await page.route(ordnanceSurveyMapsStyles, async (route) => { + await route.fulfill({ + status: 200, + body: JSON.stringify(osMapsStylesResponse), + }); + }); +} + +export async function setupOSMapsVectorTiles(page: Page) { + const ordnanceSurveyVectorTiles = new RegExp( + /\/proxy\/ordnance-survey\/maps\/vector\/v1\/vts\/tile/, + ); + + await page.route(ordnanceSurveyVectorTiles, async (route) => { + await route.fulfill({ + status: 200, + body: Buffer.from([]), + }); + }); +} diff --git a/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx b/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx index 4034072d8f..662f2a25b4 100644 --- a/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx @@ -7,20 +7,21 @@ import { ModalFooter } from "ui/editor/ModalFooter"; import ModalSection from "ui/editor/ModalSection"; import ModalSectionContent from "ui/editor/ModalSectionContent"; import RichTextInput from "ui/editor/RichTextInput/RichTextInput"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; import Input from "ui/shared/Input/Input"; import InputRow from "ui/shared/InputRow"; import { Switch } from "ui/shared/Switch"; import { DataFieldAutocomplete } from "@planx/components/shared/DataFieldAutocomplete"; import { useStore } from "pages/FlowEditor/lib/store"; -import { parseBaseNodeData } from "../../shared"; +import { Option, parseBaseNodeData } from "../../shared"; import { ICONS } from "../../shared/icons"; import type { Checklist } from "../model"; import { toggleExpandableChecklist } from "../model"; import { ChecklistProps } from "../types"; import { Options } from "./Options"; -export const ChecklistComponent: React.FC = (props) => { +export const ChecklistEditor: React.FC = (props) => { const type = TYPES.Checklist; const formik = useFormik({ @@ -41,7 +42,7 @@ export const ChecklistComponent: React.FC = (props) => { : groupedOptions?.flatMap((group) => group.children); const filteredOptions = (sourceOptions || []).filter( - (option) => option.data.text, + (option) => option.data.text ); const processedOptions = filteredOptions.map((option) => ({ @@ -68,14 +69,22 @@ export const ChecklistComponent: React.FC = (props) => { }), }, }, - processedOptions, + processedOptions ); } else { alert(JSON.stringify({ type, ...values, options }, null, 2)); } }, - validate: ({ options, groupedOptions, ...values }) => { + validate: ({ options, groupedOptions, allRequired, ...values }) => { const errors: FormikErrors = {}; + + const exclusiveOptions: Option[] | undefined = options?.filter( + (option) => option.data.exclusive + ); + if (allRequired && exclusiveOptions && exclusiveOptions.length > 0) { + errors.allRequired = + 'Cannot configure exclusive "or" option alongside "all required" setting'; + } // Account for flat or expandable Checklist options options = options || groupedOptions?.map((group) => group.children)?.flat(); @@ -83,6 +92,10 @@ export const ChecklistComponent: React.FC = (props) => { errors.fn = "At least one option must set a data value when the checklist has a data field"; } + if (exclusiveOptions && exclusiveOptions.length > 1) { + errors.options = + "There should be a maximum of one exclusive option configured"; + } return errors; }, }); @@ -155,19 +168,20 @@ export const ChecklistComponent: React.FC = (props) => { onChange={() => formik.setFieldValue( "allRequired", - !formik.values.allRequired, + !formik.values.allRequired ) } label="All required" /> + formik.setFieldValue( "neverAutoAnswer", - !formik.values.neverAutoAnswer, + !formik.values.neverAutoAnswer ) } label="Always put to user (forgo automation)" @@ -175,11 +189,13 @@ export const ChecklistComponent: React.FC = (props) => { - + + + ); }; -export default ChecklistComponent; +export default ChecklistEditor; diff --git a/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx b/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx index e3202d6331..13d2afa804 100644 --- a/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx +++ b/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx @@ -2,6 +2,9 @@ import Delete from "@mui/icons-material/Delete"; import Box from "@mui/material/Box"; import Button from "@mui/material/Button"; import IconButton from "@mui/material/IconButton"; +import { BaseOptionsEditor } from "@planx/components/shared/BaseOptionsEditor"; +import { hasFeatureFlag } from "lib/featureFlags"; +import { partition } from "lodash"; import adjust from "ramda/src/adjust"; import compose from "ramda/src/compose"; import remove from "ramda/src/remove"; @@ -9,16 +12,25 @@ import React from "react"; import { FormikHookReturn } from "types"; import ListManager from "ui/editor/ListManager/ListManager"; import ModalSectionContent from "ui/editor/ModalSectionContent"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; import Input from "ui/shared/Input/Input"; import InputRow from "ui/shared/InputRow"; +import { getOptionsSchemaByFn } from "@planx/components/shared/utils"; import { useStore } from "pages/FlowEditor/lib/store"; import { Option } from "../../shared"; import type { Group } from "../model"; import ChecklistOptionsEditor from "./OptionsEditor"; -import { getOptionsSchemaByFn } from "@planx/components/shared/utils"; export const Options: React.FC<{ formik: FormikHookReturn }> = ({ formik }) => { + const [exclusiveOptions, nonExclusiveOptions]: Option[][] = partition( + formik.values.options, + (option) => option.data.exclusive + ); + + const exclusiveOrOptionManagerShouldRender = + hasFeatureFlag("EXCLUSIVE_OR") && nonExclusiveOptions.length; + const schema = useStore().getFlowSchema()?.options; const initialOptions: Option[] | undefined = formik.initialValues.options || formik.initialValues.groupedOptions?.map((group: Group