feat: undo operations to a flow (#3056)

Co-authored-by: Ian Jones <[email protected]>

editor.planx.uk/package.json

@@ -9,6 +9,7 @@
     "@emotion/react": "^11.11.1",
     "@emotion/styled": "^11.11.0",
     "@mui/icons-material": "^5.15.2",
+    "@mui/lab": "5.0.0-alpha.170",
     "@mui/material": "^5.15.2",
     "@mui/utils": "^5.15.2",
     "@opensystemslab/map": "^0.8.2",
@@ -57,6 +58,7 @@
     "nanoid-good": "^3.1.0",
     "natsort": "^2.0.3",
     "navi": "^0.15.0",
+    "ot-json0": "^1.1.0",
     "postcode": "^5.1.0",
     "ramda": "^0.29.1",
     "react": "^18.2.0",

editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx

@@ -14,7 +14,7 @@ import { PrivateFileUpload } from "@planx/components/shared/PrivateFileUpload/Pr
 import { squareMetresToHectares } from "@planx/components/shared/utils";
 import type { PublicProps } from "@planx/components/ui";
 import buffer from "@turf/buffer";
-import { type Feature,point } from "@turf/helpers";
+import { type Feature, point } from "@turf/helpers";
 import { Store, useStore } from "pages/FlowEditor/lib/store";
 import React, { useEffect, useRef, useState } from "react";
 import { FONT_WEIGHT_SEMI_BOLD } from "theme";

editor.planx.uk/src/@planx/components/Question/Public/Question.tsx

@@ -65,10 +65,12 @@ const Question: React.FC<IQuestion> = (props) => {
     validationSchema: object({
       selected: object({
         id: string().required("Select your answer before continuing"),
-        a: mixed().required().test(value => 
-          typeof value === "number" || 
-          typeof value === "string")
-      })
+        a: mixed()
+          .required()
+          .test(
+            (value) => typeof value === "number" || typeof value === "string",
+          ),
+      }),
     }),
   });
 
@@ -82,9 +84,7 @@ const Question: React.FC<IQuestion> = (props) => {
   }
 
   return (
-    <Card
-      handleSubmit={formik.handleSubmit}
-    >
+    <Card handleSubmit={formik.handleSubmit}>
       <QuestionHeader
         title={props.text}
         description={props.description}

editor.planx.uk/src/@planx/components/shared/Preview/SimpleExpand.tsx

@@ -6,11 +6,13 @@ import React, { PropsWithChildren } from "react";
 import { FONT_WEIGHT_SEMI_BOLD } from "theme";
 import Caret from "ui/icons/Caret";
 
-const StyledButton = styled(Button)(() => ({
+const StyledButton = styled(Button, {
+  shouldForwardProp: (prop) => prop !== "lightFontStyle",
+})<{ lightFontStyle: boolean }>(({ theme, lightFontStyle }) => ({
   boxShadow: "none",
-  color: "black",
-  fontSize: "1.125rem",
-  fontWeight: FONT_WEIGHT_SEMI_BOLD,
+  color: lightFontStyle ? theme.palette.grey[600] : "black",
+  fontSize: lightFontStyle ? "1rem" : "1.125rem",
+  fontWeight: lightFontStyle ? "inherit" : FONT_WEIGHT_SEMI_BOLD,
   width: "100%",
 }));
 
@@ -20,12 +22,14 @@ interface Props {
     closed: string;
   };
   id: string;
+  lightFontStyle?: boolean;
 }
 
 const SimpleExpand: React.FC<PropsWithChildren<Props>> = ({
   children,
   buttonText,
   id,
+  lightFontStyle,
 }) => {
   const [show, setShow] = React.useState(false);
   return (
@@ -35,6 +39,7 @@ const SimpleExpand: React.FC<PropsWithChildren<Props>> = ({
           onClick={() => setShow(!show)}
           aria-expanded={show}
           aria-controls={id}
+          lightFontStyle={lightFontStyle || false}
         >
           {show ? buttonText.closed : buttonText.open}
           <Caret

editor.planx.uk/src/@planx/graph/__tests__/formatOps.test.ts

@@ -1,7 +1,7 @@
-import { formatOps,Graph } from "../index";
+import { formatOps, Graph } from "../index";
 
 describe("Update operations", () => {
-  test("Updating a single property of a node", () => {
+  test("Updating a single property of a node that is in `allowProps`", () => {
     const ops = [
       {
         p: ["FW5G3EMBI3", "data", "text"],
@@ -11,7 +11,21 @@ describe("Update operations", () => {
     ];
 
     expect(formatOps(flowWithChecklist, ops)).toEqual([
-      'Updated Checklist text from "Which fruits?" to "Which vegetables?"',
+      'Updated Checklist text from "Which fruits?" to "Which vegetables?"', // shows prop name "fn" and content "from x to y"
+    ]);
+  });
+
+  test("Updating a single property of a node that is not in `allowProps`", () => {
+    const ops = [
+      {
+        p: ["FW5G3EMBI3", "data", "info"],
+        oi: "New help text",
+        od: "Old help text",
+      },
+    ];
+
+    expect(formatOps(flowWithChecklist, ops)).toEqual([
+      `Updated Checklist help text ("Why it matters")`, // shows prop name "info", without content
     ]);
   });
 
@@ -32,9 +46,9 @@ describe("Update operations", () => {
     ];
 
     expect(formatOps(flowWithChecklist, ops)).toEqual([
-      'Added Checklist fn "fruit"',
-      'Added Answer val "berry.blue"',
-      'Added Answer val "banana"',
+      'Added Checklist data field "fruit"',
+      'Added Answer data field "berry.blue"',
+      'Added Answer data field "banana"',
     ]);
   });
 });
@@ -118,12 +132,8 @@ describe("Insert operations", () => {
     ]);
   });
 
-  test("Adding a new child and data property to an existing node", () => {
+  test("Adding a new child to an existing node", () => {
     const ops = [
-      {
-        p: ["FW5G3EMBI3", "data", "description"],
-        oi: "<p>Fruits contain seeds and come from the flower of a plant</p>",
-      },
       {
         p: ["FW5G3EMBI3", "edges"],
         oi: ["WDwUTbF7Gq", "SO5XbLwSYp", "xTBfSd1Tjy", "zzQAMXexRj"],
@@ -141,11 +151,36 @@ describe("Insert operations", () => {
     ];
 
     expect(formatOps(flowWithChecklist, ops)).toEqual([
-      'Added Checklist description "<p>Fruits contain seeds and come from the flower of a plant</p>"',
       "Updated order of Checklist edges",
       'Added Answer "Strawberry"',
     ]);
   });
+
+  test("Adding a data property to an existing node that is in `allowProps`", () => {
+    const ops = [
+      {
+        p: ["FW5G3EMBI3", "data", "fn"],
+        oi: "food.fruit",
+      },
+    ];
+
+    expect(formatOps(flowWithChecklist, ops)).toEqual([
+      `Added Checklist data field "food.fruit"`, // shows prop name "fn" and content
+    ]);
+  });
+
+  test("Adding a data property to an existing node that is not in `allowProps`", () => {
+    const ops = [
+      {
+        p: ["FW5G3EMBI3", "data", "description"],
+        oi: "<p>Fruits contain seeds and come from the flower of a plant</p>",
+      },
+    ];
+
+    expect(formatOps(flowWithChecklist, ops)).toEqual([
+      "Added Checklist description", // only shows prop name "description", not content
+    ]);
+  });
 });
 
 describe("Remove operations", () => {

editor.planx.uk/src/@planx/graph/index.ts

@@ -507,58 +507,135 @@ export const sortIdsDepthFirst =
 export const formatOps = (graph: Graph, ops: Array<OT.Op>): string[] => {
   const output: string[] = [];
 
+  // Only show full change description for simple props, omit complex or long ones like `moreInfo`, `fileTypes`, etc
+  const allowProps: string[] = ["title", "text", "fn", "val"];
+
+  // Create a simple lookup to overwrite most common component props to a more human-readable name
+  const propsMap: Record<string, string> = {
+    fn: "data field",
+    val: "data field",
+    info: `help text ("Why it matters")`,
+    howMeasured: `help text ("How is it defined?")`,
+    policyRef: `help text source links`,
+    definitionImg: `help text image`,
+  };
+
   // Updating a node or its properties (update = delete + insert)
   const handleUpdate = (node: Node, op: OT.Object.Replace) => {
-    if (op.od.type && op.oi.type) {
+    if (op.od?.type && op.oi?.type) {
       output.push(
-        `Replaced ${TYPES[op.od.type]} "${op.od.data?.title || op.od.data?.text
-        }" with ${TYPES[op.oi.type]} "${op.oi.data?.title || op.oi.data?.text
+        `Replaced ${TYPES[op.od.type]} "${
+          op.od.data?.title ||
+          op.od.data?.text ||
+          op.od.data?.content ||
+          op.od.data?.fn ||
+          op.od.data?.val ||
+          op.od.data?.flowId
+        }" with ${TYPES[op.oi.type]} "${
+          op.oi.data?.title ||
+          op.oi.data?.text ||
+          op.oi.data?.content ||
+          op.oi.data?.fn ||
+          op.oi.data?.val ||
+          op.od.data?.flowId
         }"`,
       );
     } else if (op.p.includes("data")) {
-      output.push(
-        `Updated ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} from "${op.od}" to "${op.oi
-        }"`,
-      );
+      const prop = op.p?.[2] as string;
+      if (allowProps.includes(prop)) {
+        output.push(
+          `Updated ${node?.type ? TYPES[node.type] : "node"} ${
+            propsMap[prop] || prop
+          } from "${op.od}" to "${op.oi}"`,
+        );
+      } else {
+        output.push(
+          `Updated ${node?.type ? TYPES[node.type] : "node"} ${
+            propsMap[prop] || prop
+          }`,
+        );
+      }
     } else if (op.p.includes("edges")) {
       output.push(
-        `Updated order of ${node.type ? TYPES[node.type] : "graph"} edges`,
+        `Updated order of ${node?.type ? TYPES[node.type] : "graph"} edges`,
       );
     }
   };
 
   // Updating the _root list (update = list insert or list delete)
   const handleRootUpdate = (op: OT.Array.Replace) => {
     if (op.p.includes("edges") && op.p.includes("_root")) {
-      output.push(`Re-ordered the graph`);
+      output.push(`Re-ordered the root graph`);
+    } else if (op.p.includes("edges")) {
+      output.push(`Moved node`);
     }
   };
 
   // Adding (inserting) a node or its properties
   const handleAdd = (node: Node, op: OT.Object.Add) => {
-    if (op.oi.type) {
+    if (op.oi?.type) {
       output.push(
-        `Added ${TYPES[op.oi.type]} "${op.oi.data?.title || op.oi.data?.text
+        `Added ${TYPES[op.oi.type]} "${
+          op.oi.data?.title ||
+          op.oi.data?.text ||
+          op.oi.data?.content ||
+          op.oi.data?.fn ||
+          op.oi.data?.flowId
         }"`,
       );
     } else if (op.p.includes("data")) {
-      output.push(`Added ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} "${op.oi}"`);
+      const prop = op.p?.[2] as string;
+      if (allowProps.includes(prop)) {
+        output.push(
+          `Added ${node?.type ? TYPES[node?.type] : "node"} ${
+            propsMap[prop] || prop
+          } "${op.oi}"`,
+        );
+      } else {
+        output.push(
+          `Added ${node?.type ? TYPES[node?.type] : "node"} ${
+            propsMap[prop] || prop
+          }`,
+        );
+      }
     } else if (op.p.includes("edges")) {
-      output.push(`Added ${node.type ? TYPES[node.type] : "node"} to branch`);
+      const node = graph[op.oi?.[0]];
+      output.push(`Added ${node?.type ? TYPES[node.type] : "node"} to branch`);
     }
   };
 
   // Removing (deleting) a node or its properties
   const handleRemove = (node: Node, op: OT.Object.Remove) => {
-    if (op.od.type) {
+    if (op.od?.type) {
       output.push(
-        `Removed ${TYPES[op.od.type]} "${op.od.data?.title || op.od.data?.text
+        `Removed ${TYPES[op.od.type]} "${
+          op.od.data?.title ||
+          op.od.data?.text ||
+          op.od.data?.content ||
+          op.od.data?.fn ||
+          op.od.data?.flowId
         }"`,
       );
     } else if (op.p.includes("data")) {
-      output.push(`Removed ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} "${op.od}"`);
+      const prop = op.p?.[2] as string;
+      if (allowProps.includes(prop)) {
+        output.push(
+          `Removed ${node?.type ? TYPES[node.type] : "node"} ${
+            propsMap[prop] || prop
+          } "${op.od}"`,
+        );
+      } else {
+        output.push(
+          `Removed ${node?.type ? TYPES[node.type] : "node"} ${
+            propsMap[prop] || prop
+          }`,
+        );
+      }
     } else if (op.p.includes("edges")) {
-      output.push(`Removed ${node.type ? TYPES[node.type] : "node"} from branch`);
+      const node = graph[op.od?.[0]];
+      output.push(
+        `Removed ${node?.type ? TYPES[node.type] : "node"} from branch`,
+      );
     }
   };
 

editor.planx.uk/src/lib/featureFlags.ts

@@ -1,5 +1,5 @@
 // add/edit/remove feature flags in array below
-const AVAILABLE_FEATURE_FLAGS = ["UNDO", "EDITOR_NAVIGATION"] as const;
+const AVAILABLE_FEATURE_FLAGS = ["EDITOR_NAVIGATION"] as const;
 
 type FeatureFlag = (typeof AVAILABLE_FEATURE_FLAGS)[number];