diff --git a/editor.planx.uk/src/@planx/components/SetValue/Editor.tsx b/editor.planx.uk/src/@planx/components/SetValue/Editor.tsx index 766d094b39..30ffe0264a 100644 --- a/editor.planx.uk/src/@planx/components/SetValue/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/SetValue/Editor.tsx @@ -1,3 +1,4 @@ +import Typography from "@mui/material/Typography"; import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { EditorProps, InternalNotes } from "@planx/components/ui"; import { useFormik } from "formik"; @@ -6,6 +7,7 @@ import ModalSection from "ui/editor/ModalSection"; import ModalSectionContent from "ui/editor/ModalSectionContent"; import Input from "ui/shared/Input"; import InputRow from "ui/shared/InputRow"; +import Radio from "ui/shared/Radio"; import { parseSetValue, SetValue } from "./model"; @@ -13,6 +15,64 @@ type Props = EditorProps; export default SetValueComponent; +interface Option { + value: SetValue["operation"]; + label: string; +} + +const options: Option[] = [ + { + value: "replace", + label: "Replace", + }, + { + value: "append", + label: "Append", + }, + { + value: "removeOne", + label: "Remove single value", + }, + { + value: "removeAll", + label: "Remove all values", + }, +]; + +const DescriptionText: React.FC = ({ fn, val, operation }) => { + if (!fn || !val) return null; + + switch (operation) { + case "replace": + return ( + + Any existing value for {fn} will be replaced by{" "} + {val} + + ); + case "append": + return ( + + Any existing value for {fn} will have{" "} + {val} appended to it + + ); + case "removeOne": + return ( + + Any existing value for {fn} set to{" "} + {val} will be removed + + ); + case "removeAll": + return ( + + All existing values for {fn} will be removed + + ); + } +}; + function SetValueComponent(props: Props) { const formik = useFormik({ initialValues: parseSetValue(props.node?.data), @@ -41,24 +101,26 @@ function SetValueComponent(props: Props) { -
- - {formik.values.fn && formik.values.val && ( -

- any existing value for {formik.values.fn}{" "} - will be replaced by {formik.values.val} -

- )} -
+ + + + { + formik.setFieldValue("operation", newOperation); + }} + /> + ({ fn: data?.fn || "", val: data?.val || "", + operation: data?.operation || "replace", ...parseMoreInformation(data), }); diff --git a/editor.planx.uk/src/@planx/components/SetValue/utils.test.ts b/editor.planx.uk/src/@planx/components/SetValue/utils.test.ts new file mode 100644 index 0000000000..c556459134 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/SetValue/utils.test.ts @@ -0,0 +1,135 @@ +import { SetValue } from "./model"; +import { handleSetValue } from "./utils"; +import { Store } from "pages/FlowEditor/lib/store"; + + +describe("calculateNewValues() helper function", () => { + describe('"replace" operation', () => { + test.each([ + { previous: undefined, expected: ["lion"] }, + { previous: "panda", expected: ["lion"] }, + { previous: "lion", expected: ["lion"] }, + { previous: ["lion"], expected: ["lion"] }, + { previous: ["bear", "dog", "monkey"], expected: ["lion"] }, + { previous: ["bear", "dog", "lion"], expected: ["lion"] }, + ])('input of $previous sets passport value to be $expected', ({ previous, expected }) => { + const mockKey = "myAnimals"; + const mockSetValue: SetValue = { + operation: "replace", + fn: mockKey, + val: "lion", + }; + const mockPassport: Store.passport = { + data: { mockNode: mockSetValue, [mockKey]: previous }, + }; + + const updatedPassport = handleSetValue({ + nodeData: mockSetValue, + previousValues: previous, + passport: mockPassport, + }); + + const actual = updatedPassport.data?.[mockKey]; + expect(actual).toEqual(expected); + }); + }); + + describe('"append" operation', () => { + test.each([ + { previous: undefined, expected: ["lion"] }, + { previous: "panda", expected: ["panda", "lion"] }, + { previous: "lion", expected: ["lion"] }, + { previous: ["lion"], expected: ["lion"] }, + { + previous: ["bear", "dog", "monkey"], + expected: ["bear", "dog", "monkey", "lion"], + }, + { previous: ["bear", "dog", "lion"], expected: ["bear", "dog", "lion"] }, + ])('input of $previous sets passport value to be $expected', ({ previous, expected }) => { + const mockKey = "myAnimals"; + const mockSetValue: SetValue = { + operation: "append", + fn: mockKey, + val: "lion", + }; + const mockPassport: Store.passport = { + data: { mockNode: mockSetValue, [mockKey]: previous }, + }; + + const updatedPassport = handleSetValue({ + nodeData: mockSetValue, + previousValues: previous, + passport: mockPassport, + }); + + const actual = updatedPassport.data?.[mockKey]; + expect(actual).toEqual(expected); + }); + }); + + describe('"removeOne" operation', () => { + test.each([ + { previous: undefined, expected: undefined }, + { previous: "panda", expected: "panda" }, + { previous: "lion", expected: undefined }, + { previous: ["lion"], expected: undefined }, + { + previous: ["bear", "dog", "monkey"], + expected: ["bear", "dog", "monkey"], + }, + { previous: ["bear", "dog", "lion"], expected: ["bear", "dog"] }, + ])('input of $previous sets passport value to be $expected', ({ previous, expected }) => { + const mockKey = "myAnimals"; + const mockSetValue: SetValue = { + operation: "removeOne", + fn: mockKey, + val: "lion", + }; + const mockPassport: Store.passport = { + data: { mockNode: mockSetValue, [mockKey]: previous }, + }; + + const updatedPassport = handleSetValue({ + nodeData: mockSetValue, + previousValues: previous, + passport: mockPassport, + }); + + const actual = updatedPassport.data?.[mockKey]; + expect(actual).toEqual(expected); + }); + }); + + describe('"removeAll" operation', () => { + test.each([ + { previous: undefined, expected: undefined }, + { previous: "panda", expected: undefined }, + { previous: "lion", expected: undefined }, + { previous: ["lion"], expected: undefined }, + { + previous: ["bear", "dog", "monkey"], + expected: undefined, + }, + { previous: ["bear", "dog", "lion"], expected: undefined }, + ])('input of $previous sets passport value to be $expected', ({ previous, expected }) => { + const mockKey = "myAnimals"; + const mockSetValue: SetValue = { + operation: "removeAll", + fn: mockKey, + val: "lion", + }; + const mockPassport: Store.passport = { + data: { mockNode: mockSetValue, [mockKey]: previous }, + }; + + const updatedPassport = handleSetValue({ + nodeData: mockSetValue, + previousValues: previous, + passport: mockPassport, + }); + + const actual = updatedPassport.data?.[mockKey]; + expect(actual).toEqual(expected); + }); + }); +}); diff --git a/editor.planx.uk/src/@planx/components/SetValue/utils.ts b/editor.planx.uk/src/@planx/components/SetValue/utils.ts new file mode 100644 index 0000000000..be6756dff3 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/SetValue/utils.ts @@ -0,0 +1,89 @@ +import { Store } from "pages/FlowEditor/lib/store"; +import { SetValue } from "./model"; + +type PreviousValues = string | string[] | undefined; + +type HandleSetValue = (params: { + nodeData: SetValue; + previousValues: PreviousValues; + passport: Store.passport; +}) => Store.passport; + +/** + * Handle modifying passport values when passing through a SetValue component + * Called by computePassport() + */ +export const handleSetValue: HandleSetValue = ({ + nodeData: { operation, fn, val: current }, + previousValues, + passport, +}) => { + // We do not amend values set at objects + // These are internal exceptions we do not want to allow users to edit + // e.g. property.boundary.title + const isObject = + typeof previousValues === "object" && + !Array.isArray(previousValues) && + previousValues !== null; + if (isObject) return passport; + + const previous = formatPreviousValues(previousValues); + + const newValues = calculateNewValues({ + operation, + previous, + current, + }); + + if (newValues) { + passport.data![fn] = newValues; + + // Operation has cleared passport value + if (!newValues.length) delete passport.data![fn]; + } + + return passport; +}; + +type CalculateNewValues = (params: { + operation: SetValue["operation"]; + previous: string[]; + current: string; +}) => string | string[] | undefined; + +const calculateNewValues: CalculateNewValues = ({ + operation, + previous, + current, +}) => { + switch (operation) { + case "replace": + return [current]; + + case "removeOne": { + // Do not convert output from string to string[] if operation not possible + if (previous.length === 1 && previous[0] !== current) { + return previous[0]; + } + + const removeCurrent = (val: string) => val !== current; + const filtered = previous.filter(removeCurrent); + return filtered; + } + + case "removeAll": + return []; + + case "append": { + const combined = [...previous, current]; + const unique = [...new Set(combined)]; + return unique; + } + } +}; + +const formatPreviousValues = (values: PreviousValues): string[] => { + if (!values) return []; + if (Array.isArray(values)) return values; + return [values]; +}; diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Node.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Node.tsx index 271fe92ffd..40b0dceb75 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Node.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Node.tsx @@ -2,7 +2,7 @@ import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import React from "react"; import { ErrorBoundary } from "react-error-boundary"; -import { useStore } from "../../../lib/store"; +import { Store, useStore } from "../../../lib/store"; import { stripTagsAndLimitLength } from "../lib/utils"; import Breadcrumb from "./Breadcrumb"; import Checklist from "./Checklist"; @@ -102,12 +102,7 @@ const Node: React.FC = (props) => { case TYPES.Send: return ; case TYPES.SetValue: - return ( - - ); + return ; case TYPES.TaskList: return ( = (props) => { } }; +const getSetValueText = ({ operation, fn, val }: Store.node["data"]) => { + switch (operation) { + case "append": + return `Append ${val} to ${fn}`; + case "removeOne": + return `Remove ${val} from ${fn}`; + case "removeAll": + return `Remove ${fn}`; + default: + return `Replace ${fn} with ${val}`; + } +}; + function exhaustiveCheck(type: never): never { throw new Error(`Missing type ${type}`); } diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/record.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/record.test.ts index 1d5ac7782d..e8d825104f 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/record.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/record.test.ts @@ -26,7 +26,7 @@ describe("error handling", () => { }, }); - expect(() => record("x", {})).toThrow("id not found"); + expect(() => record("x", {})).toThrow('id "x" not found'); }); }); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts new file mode 100644 index 0000000000..09052ced51 --- /dev/null +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts @@ -0,0 +1,353 @@ +import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; +import { cloneDeep, merge } from "lodash"; + +import { Store, vanillaStore } from "../store"; + +const { getState, setState } = vanillaStore; +const { resetPreview, record, computePassport, currentCard } = getState(); + +const baseFlow: Store.flow = { + _root: { + edges: ["setValue1", "middleOfService", "setValue2", "endOfService"], + }, + setValue1: { + data: { + fn: "myKey", + val: "myFirstValue", + operation: null, + }, + type: TYPES.SetValue, + }, + middleOfService: { + type: TYPES.Notice, + data: { + title: "Middle of service", + color: "#EFEFEF", + resetButton: false, + }, + }, + setValue2: { + data: { + fn: "myKey", + val: "mySecondValue", + operation: null, + }, + type: TYPES.SetValue, + }, + endOfService: { + type: TYPES.Notice, + data: { + title: "End of service", + color: "#EFEFEF", + resetButton: true, + }, + }, +}; + +describe("SetValue component", () => { + describe("replace operation", () => { + const replaceFlow = merge(cloneDeep(baseFlow), { + setValue1: { + data: { + operation: "replace", + }, + }, + setValue2: { + data: { + operation: "replace", + }, + }, + }); + + beforeEach(() => { + resetPreview(); + setState({ flow: replaceFlow }); + }); + + it("sets a value if not previously set", () => { + // Value not set + expect(computePassport()?.data?.myKey).not.toBeDefined(); + + // Step through first SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + + // SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue1"); + + // Middle of flow reached + expect(currentCard()?.id).toEqual("middleOfService"); + + // Passport correctly populated + expect(computePassport()?.data?.myKey).toHaveLength(1); + expect(computePassport()?.data?.myKey).toContain("myFirstValue"); + }); + + it("replaces an existing value", () => { + // Step through second SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + record("middleOfService", {}); + record("setValue2", { data: { myKey: ["mySecondValue"] } }); + + // Second SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue2"); + + // End of flow reached + expect(currentCard()?.id).toEqual("endOfService"); + + // Passport correctly populated + expect(computePassport()?.data?.myKey).toHaveLength(1); + expect(computePassport()?.data?.myKey).toContain("mySecondValue"); + }); + }); + + describe("append operation", () => { + const appendFlow = merge(cloneDeep(baseFlow), { + setValue1: { + data: { + operation: "append", + }, + }, + setValue2: { + data: { + operation: "append", + }, + }, + }); + + beforeEach(() => { + resetPreview(); + setState({ flow: appendFlow }); + }); + + it("sets a value if not previously set", () => { + // Value not set + expect(computePassport()?.data?.myKey).not.toBeDefined(); + + // Step through first SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + + // SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue1"); + + // Middle of flow reached + expect(currentCard()?.id).toEqual("middleOfService"); + + // Passport correctly populated + expect(computePassport()?.data?.myKey).toHaveLength(1); + expect(computePassport()?.data?.myKey).toContain("myFirstValue"); + }); + + it("appends to an existing value", () => { + // Step through second SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + record("middleOfService", {}); + + expect(computePassport()?.data?.myKey).toHaveLength(1); + expect(computePassport()?.data?.myKey).toContain("myFirstValue"); + + record("setValue2", { data: { myKey: ["mySecondValue"] } }); + + // Second SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue2"); + + // End of flow reached + expect(currentCard()?.id).toEqual("endOfService"); + + // Passport correctly populated + expect(computePassport()?.data?.myKey).toHaveLength(2); + expect(computePassport()?.data?.myKey).toContain("myFirstValue"); + expect(computePassport()?.data?.myKey).toContain("mySecondValue"); + }); + }); + + describe("removeOne operation", () => { + const removeOneFlow = merge(cloneDeep(baseFlow), { + _root: { + edges: [ + "setValue1", + "middleOfService", + "setValue2", + "setValue3", + "endOfService", + ], + }, + setValue1: { + data: { + operation: "removeOne", + }, + }, + setValue2: { + data: { + operation: "replace", + }, + }, + setValue3: { + data: { + fn: "myKey", + val: "mySecondValue", + operation: "removeOne", + }, + type: TYPES.SetValue, + }, + }); + + beforeEach(() => { + resetPreview(); + setState({ flow: removeOneFlow }); + }); + + it("does nothing if a value is not previously set", () => { + // Value not set + expect(computePassport()?.data?.myKey).not.toBeDefined(); + + // Step through first SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + + // SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue1"); + + // Middle of flow reached + expect(currentCard()?.id).toEqual("middleOfService"); + + // Passport correctly populated - value not present + expect(computePassport()?.data?.myKey).toBeUndefined(); + }); + + it("removes a passport variable when the value matches", () => { + // Step through second SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + record("middleOfService", {}); + record("setValue2", { data: { myKey: ["mySecondValue"] } }); + + // Second SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue2"); + + // Value is set by "replace" operation + expect(computePassport()?.data?.myKey).toHaveLength(1); + expect(computePassport()?.data?.myKey).toContain("mySecondValue"); + + // Step through final SetValue component + record("setValue3", { data: { myKey: ["mySecondValue"] } }); + + // End of flow reached + expect(currentCard()?.id).toEqual("endOfService"); + + // Passport correctly populated - value no longer set + expect(computePassport()?.data?.myKey).toBeUndefined(); + }); + + it("does not remove a passport variable when the value does not match", () => { + const flowWithMismatchedValue = merge(cloneDeep(removeOneFlow), { + setValue3: { + data: { + fn: "myKey", + // Value not present, will not be removed + val: "myUnsetValue", + operation: "removeOne", + }, + type: TYPES.SetValue, + }, + }); + + resetPreview(); + setState({ flow: flowWithMismatchedValue }); + + // Step through flow + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + record("middleOfService", {}); + record("setValue2", { data: { myKey: ["mySecondValue"] } }); + record("setValue3", { data: { myKey: ["myUnsetValue"] } }); + + // End of flow reached + expect(currentCard()?.id).toEqual("endOfService"); + + // Passport correctly populated - passport variable not removed as values do not match + expect(computePassport()?.data?.myKey).toEqual("mySecondValue"); + }); + }); + + describe("removeAll operation", () => { + const removeAllFlow = merge(cloneDeep(baseFlow), { + _root: { + edges: [ + "setValue1", + "middleOfService", + "setValue2", + "setValue3", + "endOfService", + ], + }, + setValue1: { + data: { + operation: "removeAll", + }, + }, + setValue2: { + data: { + operation: "replace", + }, + }, + setValue3: { + data: { + fn: "myKey", + val: "mySecondValue", + operation: "removeAll", + }, + type: TYPES.SetValue, + }, + }); + + beforeEach(() => { + resetPreview(); + setState({ flow: removeAllFlow }); + }); + + it("does nothing if a value is not previously set", () => { + // Value not set + expect(computePassport()?.data?.myKey).not.toBeDefined(); + + // Step through first SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + + // SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue1"); + + // Middle of flow reached + expect(currentCard()?.id).toEqual("middleOfService"); + + // Passport correctly populated - value not present + expect(computePassport()?.data?.myKey).toBeUndefined(); + }); + + it("removes a passport variable", () => { + // Step through second SetValue + record("setValue1", { data: { myKey: ["myFirstValue"] } }); + record("middleOfService", {}); + record("setValue2", { data: { myKey: ["mySecondValue"] } }); + + // Second SetValue is visited + const breadcrumbKeys = Object.keys(getState().breadcrumbs); + expect(breadcrumbKeys).toContain("setValue2"); + + // Value is set by "replace" operation + expect(computePassport()?.data?.myKey).toHaveLength(1); + expect(computePassport()?.data?.myKey).toContain("mySecondValue"); + + // Step through final SetValue component + record("setValue3", { data: { myKey: ["mySecondValue"] } }); + + // End of flow reached + expect(currentCard()?.id).toEqual("endOfService"); + + // Passport correctly populated - key:value pair removed + expect(computePassport()?.data).not.toHaveProperty("myKey"); + }); + }); +}); 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 73dd288e26..97f437e533 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts @@ -10,6 +10,7 @@ import { } from "@opensystemslab/planx-core/types"; import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { FileList } from "@planx/components/FileUploadAndLabel/model"; +import { SetValue } from "@planx/components/SetValue/model"; import { sortIdsDepthFirst } from "@planx/graph"; import { logger } from "airbrake"; import { objectWithoutNullishValues } from "lib/objectHelpers"; @@ -28,6 +29,7 @@ import { ApplicationPath } from "./../../../../types"; import type { Store } from "."; import { NavigationStore } from "./navigation"; import type { SharedStore } from "./shared"; +import { handleSetValue } from "@planx/components/SetValue/utils"; const SUPPORTED_DECISION_TYPES = [TYPES.Checklist, TYPES.Question]; let memoizedPreviousCardId: string | undefined = undefined; @@ -246,7 +248,7 @@ export const previewStore: StateCreator< {} as Store.passport["data"], ); - return { + let passport: Store.passport = { ...acc, data: { ...acc.data, @@ -254,6 +256,17 @@ export const previewStore: StateCreator< ...passportData, }, }; + + const isSetValue = flow[id].type === TYPES.SetValue; + if (isSetValue) { + passport = handleSetValue({ + nodeData: flow[id].data as SetValue, + previousValues: acc.data?.[key], + passport, + }); + } + + return passport; }, { data: {}, @@ -276,7 +289,7 @@ export const previewStore: StateCreator< updateSectionData, } = get(); - if (!flow[id]) throw new Error("id not found"); + if (!flow[id]) throw new Error(`id "${id}" not found`); if (userData) { // add breadcrumb @@ -889,4 +902,4 @@ export const removeNodesDependentOnPassport = ( return acc; }, [] as string[]); return { removedNodeIds, breadcrumbsWithoutPassportData: newBreadcrumbs }; -}; +}; \ No newline at end of file