From 6a2e9bea972bcda52dffd3991e79a66b7a2549bb Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Wed, 2 Oct 2024 11:09:57 +0200 Subject: [PATCH] clear MapAndLabel breadcrumbs if FindProperty or DrawBoundary have changed --- .../mocks/breadcrumbsDependentOnPassport.json | 24 ++++++ .../mocks/flowWithPassportComponents.json | 13 +++ .../removeNodesDependentOnPassport.test.ts | 81 ++++++++++++++----- .../src/pages/FlowEditor/lib/store/preview.ts | 7 +- 4 files changed, 103 insertions(+), 22 deletions(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/breadcrumbsDependentOnPassport.json b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/breadcrumbsDependentOnPassport.json index 6afd65a03f..09e770cf6e 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/breadcrumbsDependentOnPassport.json +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/breadcrumbsDependentOnPassport.json @@ -141,5 +141,29 @@ "data": { "text": "Test2" } + }, + "mapAndLabel": { + "auto": false, + "data": { + "trees": { + "type": "Feature", + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [-0.07626448954420499, 51.48571252157308], + [-0.0762916416717913, 51.48561932090584], + [-0.07614058275089933, 51.485617225458554], + [-0.07611118911905082, 51.4857099488319], + [-0.07626448954420499, 51.48571252157308] + ] + ] + }, + "properties": { + "label": 1, + "species": "Elm" + } + } + } } } diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/flowWithPassportComponents.json b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/flowWithPassportComponents.json index 81f5d2dc12..f621b4065a 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/flowWithPassportComponents.json +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/flowWithPassportComponents.json @@ -5,10 +5,23 @@ "text", "drawBoundary", "planningConstraints", + "mapAndLabel", "text2", "notice" ] }, + "mapAndLabel": { + "data": { + "fn": "trees", + "title": "Map and label", + "basemap": "OSM", + "drawColor": "#00ff00", + "drawType": "Polygon", + "schemaName": "MockTrees", + "schema": "MockTrees" + }, + "type": 155 + }, "planningConstraints": { "data": { "fn": "property.constraints.planning", diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts index deff110ec2..fb0e5d0511 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts @@ -1,3 +1,4 @@ +import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import cloneDeep from "lodash/cloneDeep"; import { Store, useStore } from "../../store"; @@ -47,7 +48,11 @@ describe("removeNodesDependentOnPassport", () => { text2: breadcrumbs["text2"], }; - const expectRemovedNots = ["planningConstraints", "drawBoundary"]; + const expectRemovedNots = [ + "mapAndLabel", + "planningConstraints", + "drawBoundary", + ]; expect(breadcrumbsWithoutPassportData).toEqual(expectedBreadcrumbs); expect(removedNodeIds.sort()).toEqual(expectRemovedNots.sort()); @@ -67,7 +72,7 @@ describe("removeNodesDependentOnPassport", () => { }); describe("nodesDependentOnPassport with record", () => { - test("should remove Draw Boundary and Planning constraints from cachedBreadcrumbs", () => { + test("should remove DrawBoundary, PlanningConstraints, and MapAndLabel from cachedBreadcrumbs after FindProperty changes", () => { const cachedBreadcrumbs = { ...breadcrumbsDependentOnPassport, } as Store.CachedBreadcrumbs; @@ -115,7 +120,7 @@ describe("nodesDependentOnPassport with record", () => { expect(getState().cachedBreadcrumbs).toEqual(expectedCachedBreadcrumbs); }); - test("should remove Planning constraints from cachedBreadcrumbs", () => { + test("should remove PlanningConstraints and MapAndLabel from cachedBreadcrumbs after DrawBoundary changes and retain FindProperty breadcrumbs", () => { const cachedBreadcrumbs = { ...breadcrumbsDependentOnPassport, } as Store.CachedBreadcrumbs; @@ -168,7 +173,11 @@ describe("nodesDependentOnPassport with record", () => { text: breadcrumbsDependentOnPassport.text, }; const flow = { ...flowWithPassportComponents }; - const _nodesPendingEdit = ["drawBoundary", "planningConstraints"]; + const _nodesPendingEdit = [ + "drawBoundary", + "planningConstraints", + "mapAndLabel", + ]; setState({ flow, @@ -191,14 +200,18 @@ describe("nodesDependentOnPassport with record", () => { expect(getCurrentCard()?.id).toEqual("drawBoundary"); }); - test("should clear _nodesPendingEdit after edition", () => { + test("should clear _nodesPendingEdit after continuing through final dependent component type", () => { const breadcrumbs = { findProperty: breadcrumbsDependentOnPassport.findProperty, text: breadcrumbsDependentOnPassport.text, drawBoundary: breadcrumbsDependentOnPassport.drawBoundary, }; const flow = { ...flowWithPassportComponents }; - const _nodesPendingEdit = ["drawBoundary", "planningConstraints"]; + const _nodesPendingEdit = [ + "drawBoundary", + "planningConstraints", + "mapAndLabel", + ]; setState({ flow, @@ -216,6 +229,32 @@ describe("nodesDependentOnPassport with record", () => { }, }); + record("mapAndLabel", { + answers: [], + auto: false, + data: { + trees: { + type: "Feature", + geometry: { + type: "Polygon", + coordinates: [ + [ + [-0.07626448954420499, 51.48571252157308], + [-0.0762916416717913, 51.48561932090584], + [-0.07614058275089933, 51.485617225458554], + [-0.07611118911905082, 51.4857099488319], + [-0.07626448954420499, 51.48571252157308], + ], + ], + }, + properties: { + label: 1, + species: "Elm", + }, + }, + }, + }); + expect(getState()._nodesPendingEdit).toHaveLength(0); }); }); @@ -248,7 +287,11 @@ describe("nodesDependentOnPassport with previousCard", () => { findProperty: breadcrumbsDependentOnPassport.findProperty, }; const flow = { ...flowWithPassportComponents }; - const _nodesPendingEdit = ["drawBoundary", "planningConstraints"]; + const _nodesPendingEdit = [ + "drawBoundary", + "planningConstraints", + "mapAndLabel", + ]; setState({ breadcrumbs, @@ -346,7 +389,7 @@ const mockFlowData = { data: { text: "Question", }, - type: 100, + type: TYPES.Question, edges: ["Om0CWNHoDs", "GxcDrNTW26"], }, "4FRZMfNlXf": { @@ -354,33 +397,33 @@ const mockFlowData = { flag: "PP-NOT_DEVELOPMENT", text: "Not development", }, - type: 200, + type: TYPES.Answer, edges: ["OjcsvOxVum"], }, AHOdMRaRGK: { data: { title: "Question text", }, - type: 110, + type: TYPES, }, DTXNs02JmU: { data: { text: "A2", }, - type: 200, + type: TYPES.Answer, edges: ["AHOdMRaRGK"], }, GM8yVE4Fgm: { data: { title: "Prior approval ", }, - type: 110, + type: TYPES.TextInput, }, GxcDrNTW26: { data: { text: "path2", }, - type: 200, + type: TYPES.Answer, edges: ["J5SvQgzuK0"], }, IzT93uCmyF: { @@ -388,41 +431,41 @@ const mockFlowData = { flag: "PRIOR_APPROVAL", text: "Prior", }, - type: 200, + type: TYPES.Answer, edges: ["1eJjMmhGBU"], }, J5SvQgzuK0: { data: { text: "Question 2", }, - type: 100, + type: TYPES.Question, edges: ["fSN4QxmM2w", "DTXNs02JmU"], }, OjcsvOxVum: { data: { title: "Non development text", }, - type: 110, + type: TYPES.TextInput, }, Om0CWNHoDs: { data: { text: "path1", }, - type: 200, + type: TYPES.Answer, edges: ["GM8yVE4Fgm"], }, fSN4QxmM2w: { data: { text: "A1", }, - type: 200, + type: TYPES.Answer, }, mBFPszBssY: { data: { text: "Checklist for review", allRequired: false, }, - type: 105, + type: TYPES.Checklist, edges: ["IzT93uCmyF", "4FRZMfNlXf"], }, }, diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts b/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts index 6bdff0f25a..44e4a90ae3 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts @@ -284,7 +284,7 @@ export const previewStore: StateCreator< }, // record() notably handles removing cachedBreadcrumbs for dependent component types - // ie if you 'go back' to change your address, DrawBoundary and PlanningConstraints shouldn't be retained because they reference the property site passport, but answers to other questions can be retained + // ie if you 'go back' to change your address, `DEPENDENT_TYPES` shouldn't be retained because they reference the property site passport, but answers to other questions can be retained record(id, userData) { const { breadcrumbs, @@ -594,7 +594,6 @@ export const previewStore: StateCreator< // Temporarily always returns false until upcomingCardIds is optimised // OSL Slack explanation: https://bit.ly/3x38IRY return false; - }, restore: false, @@ -862,6 +861,7 @@ function handleNodesWithPassport({ POPULATE_PASSPORT.includes(flow[id].type!) && newBreadcrumbs?.[id] && !isEqual(userData, newBreadcrumbs[id]); + // Check if component populates passport so that nodes dependent on passport values // do not have inconsistent data on them after changing answer in Review. if (breadcrumbPopulatesPassport) { @@ -897,8 +897,9 @@ export const removeNodesDependentOnPassport = ( removedNodeIds: string[]; } => { const DEPENDENT_TYPES = [ - TYPES.PlanningConstraints, TYPES.DrawBoundary, + TYPES.MapAndLabel, + TYPES.PlanningConstraints, TYPES.PropertyInformation, ]; const newBreadcrumbs = { ...breadcrumbs };