fix: Handle multiple external portals as siblings (#2217)

api.planx.uk/helpers.test.ts

@@ -1,4 +1,6 @@
-import { getFormattedEnvironment, isLiveEnv } from "./helpers";
+import { ComponentType } from "@opensystemslab/planx-core/types";
+import { dataMerged, getFormattedEnvironment, isLiveEnv } from "./helpers";
+import { queryMock } from "./tests/graphqlQueryMock";
 
 describe("getEnvironment function", () => {
   const OLD_ENV = process.env;
@@ -58,3 +60,111 @@ describe("isLiveEnv() function", () => {
     });
   });
 });
+
+describe("dataMerged() function", () => {
+  beforeEach(() => {
+    const unflattenedParent = {
+      _root: {
+        edges: ["Zj0ZKa0PwT", "Rur8iS88x3"],
+      },
+      "5yElH96W7I": {
+        data: {
+          text: "Option 2",
+        },
+        type: 200,
+        edges: ["aMlxwR7ONH"],
+      },
+      Rur8iS88x3: {
+        data: {
+          color: "#EFEFEF",
+          title: "End of the line",
+          resetButton: false,
+        },
+        type: 8,
+      },
+      SShTHaRo2k: {
+        data: {
+          flowId: "child-id",
+        },
+        type: 310,
+      },
+      Zj0ZKa0PwT: {
+        data: {
+          text: "This is a question with many options",
+        },
+        type: 100,
+        edges: ["c8hZwm0a9c", "5yElH96W7I", "UMsI68BuAy"],
+      },
+      c8hZwm0a9c: {
+        data: {
+          text: "Option 1",
+        },
+        type: 200,
+        edges: ["SShTHaRo2k"],
+      },
+      aMlxwR7ONH: {
+        type: 310,
+        data: {
+          flowId: "child-id",
+        },
+      },
+      UMsI68BuAy: {
+        type: 200,
+        data: {
+          text: "Option 3",
+        },
+      },
+    };
+
+    const unflattenedChild = {
+      _root: {
+        edges: ["sbDyJVsyXg"],
+      },
+      sbDyJVsyXg: {
+        type: 100,
+        data: {
+          description: "<p>Hello there 👋</p>",
+          text: "This is within the portal",
+        },
+      },
+    };
+
+    queryMock.mockQuery({
+      name: "GetFlowData",
+      variables: {
+        id: "child-id",
+      },
+      data: {
+        flows_by_pk: {
+          slug: "child-flow",
+          data: unflattenedChild,
+          team_id: 123,
+        },
+      },
+    });
+
+    queryMock.mockQuery({
+      name: "GetFlowData",
+      variables: {
+        id: "parent-id",
+      },
+      data: {
+        flows_by_pk: {
+          slug: "parent-flow",
+          data: unflattenedParent,
+          team_id: 123,
+        },
+      },
+    });
+  });
+  it("handles multiple external portal nodes", async () => {
+    const result = await dataMerged("parent-id");
+    const nodeTypes = Object.values(result).map((node) => node.type);
+    const areAllPortalsFlattened = !nodeTypes.includes(
+      ComponentType.ExternalPortal,
+    );
+
+    // All external portals have been flattened / replaced
+    expect(areAllPortalsFlattened).toBe(true);
+  });
+});

api.planx.uk/helpers.ts

@@ -2,6 +2,7 @@ import { gql } from "graphql-request";
 import { capitalize } from "lodash";
 import { adminGraphQLClient as adminClient } from "./hasura";
 import { Flow, Node } from "./types";
+import { ComponentType } from "@opensystemslab/planx-core/types";
 
 // Get a flow's data (unflattened, without external portal nodes)
 const getFlowData = async (id: string): Promise<Flow> => {
@@ -138,24 +139,39 @@ const dataMerged = async (id: string, ob: Record<string, any> = {}) => {
   // get the primary flow data
   const { slug, data } = await getFlowData(id);
 
-  // recursively get and flatten internal portals (type 300) & external portals (type 310)
+  // recursively get and flatten internal portals & external portals
   for (const [nodeId, node] of Object.entries(data)) {
-    if (nodeId === "_root" && Object.keys(ob).length > 0) {
+    const isExternalPortalRoot =
+      nodeId === "_root" && Object.keys(ob).length > 0;
+    const isExternalPortal = node.type === ComponentType.ExternalPortal;
+    const isMerged = ob[node.data?.flowId];
+
+    // Merge portal root as a new node in the graph
+    if (isExternalPortalRoot) {
       ob[id] = {
         ...node,
-        type: 300,
+        type: ComponentType.InternalPortal,
         data: { text: slug },
       };
-    } else if (node.type === 310 && !ob[node.data?.flowId]) {
-      await dataMerged(node.data?.flowId, ob);
+    }
+
+    // Merge as internal portal, with reference to flowId
+    else if (isExternalPortal) {
       ob[nodeId] = {
-        type: 300,
+        type: ComponentType.InternalPortal,
         edges: [node.data?.flowId],
       };
-    } else {
-      ob[nodeId] = node;
+
+      // Recursively merge flow
+      if (!isMerged) {
+        await dataMerged(node.data?.flowId, ob);
+      }
     }
+
+    // Merge all other nodes
+    else ob[nodeId] = node;
   }
+
   return ob;
 };
 

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

@@ -20,28 +20,44 @@ const getFlowData = async (id: string) => {
   return data.flows_by_pk;
 };
 
-export const dataMerged = async (
-  id: string,
-  ob: Record<string, any> = {},
-): Promise<Record<string, any>> => {
-  const { slug, data }: { slug: string; data: Record<string, any> } =
-    await getFlowData(id);
+// Flatten a flow's data to include main content & portals in a single JSON representation
+// XXX: getFlowData & dataMerged are currently repeated in api.planx.uk/helpers.ts
+//        in order to load frontend /preview routes for flows that are not published
+export const dataMerged = async (id: string, ob: Record<string, any> = {}) => {
+  // get the primary flow data
+  const { slug, data }: { slug: string; data: Record<string, any> } = await getFlowData(id);
+
+  // recursively get and flatten internal portals & external portals
   for (const [nodeId, node] of Object.entries(data)) {
-    if (nodeId === "_root" && Object.keys(ob).length > 0) {
+    const isExternalPortalRoot = nodeId === "_root" && Object.keys(ob).length > 0;
+    const isExternalPortal = node.type === TYPES.ExternalPortal;
+    const isMerged = ob[node.data?.flowId];
+
+    // Merge portal root as a new node in the graph
+    if (isExternalPortalRoot) {
       ob[id] = {
         ...node,
         type: TYPES.InternalPortal,
         data: { text: slug },
       };
-    } else if (node.type === TYPES.ExternalPortal && !ob[node.data.flowId]) {
-      await dataMerged(node.data.flowId, ob);
+    }
+
+    // Merge as internal portal, with reference to flowId
+    else if (isExternalPortal) {
       ob[nodeId] = {
         type: TYPES.InternalPortal,
-        edges: [node.data.flowId],
+        edges: [node.data?.flowId],
       };
-    } else {
-      ob[nodeId] = node;
+
+      // Recursively merge flow
+      if (!isMerged) {
+        await dataMerged(node.data?.flowId, ob);
+      }
     }
+
+    // Merge all other nodes
+    else ob[nodeId] = node;
   }
+
   return ob;
 };

editor.planx.uk/src/pages/FlowEditor/lib/__tests__/externalPortals.test.ts

@@ -0,0 +1,63 @@
+import { FullStore, vanillaStore } from "../store";
+import multipleExternalPortals from "./mocks/multipleExternalPortals.json";
+import singleExternalPortal from "./mocks/singleExternalPortal.json";
+
+const { getState, setState } = vanillaStore;
+const { upcomingCardIds, record } = getState();
+
+let initialState: FullStore;
+
+beforeEach(() => {
+  initialState = getState();
+});
+
+describe("A flow with a single external portal can be navigated as expected", () => {
+  beforeEach(() => setState({ flow: singleExternalPortal }));
+  afterEach(() => setState(initialState));
+
+  it("without entering the portal", () => {
+    expect(upcomingCardIds()[0]).toEqual("firstNode");
+    // Navigate down branch avoiding external portal
+    record("firstNode", { answers: ["option2"] });
+    expect(upcomingCardIds()[0]).toEqual("finalNode");
+  });
+
+  it("via the portal", () => {
+    expect(upcomingCardIds()[0]).toEqual("firstNode");
+    // Navigate down branch via external portal
+    record("firstNode", { answers: ["option1"] });
+    expect(upcomingCardIds()[0]).toEqual("withinExternalPortal");
+    record("withinExternalPortal", { answers: [] });
+    expect(upcomingCardIds()[0]).toEqual("finalNode");
+  });
+});
+
+describe("A flow with repeated external portals can be navigated as expected", () => {
+  beforeEach(() => setState({ flow: multipleExternalPortals }));
+  afterEach(() => setState(initialState));
+
+  it("without entering the portal", () => {
+    expect(upcomingCardIds()[0]).toEqual("firstNode");
+    // Navigate down branch avoiding external portal
+    record("firstNode", { answers: ["option3"] });
+    expect(upcomingCardIds()[0]).toEqual("finalNode");
+  });
+
+  it("via the first portal", () => {
+    expect(upcomingCardIds()[0]).toEqual("firstNode");
+    // Navigate down branch via first external portal
+    record("firstNode", { answers: ["option1"] });
+    expect(upcomingCardIds()[0]).toEqual("withinExternalPortal");
+    record("withinExternalPortal", { answers: [] });
+    expect(upcomingCardIds()[0]).toEqual("finalNode");
+  });
+
+  it("via the second portal", () => {
+    expect(upcomingCardIds()[0]).toEqual("firstNode");
+    // Navigate down branch via second external portal
+    record("firstNode", { answers: ["option2"] });
+    expect(upcomingCardIds()[0]).toEqual("withinExternalPortal");
+    record("withinExternalPortal", { answers: [] });
+    expect(upcomingCardIds()[0]).toEqual("finalNode");
+  });
+});

editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/multipleExternalPortals.json

@@ -0,0 +1,81 @@
+{
+  "_root": {
+    "edges": [
+      "firstNode",
+      "finalNode"
+    ]
+  },
+  "externalPortal1": {
+    "type": 300,
+    "edges": [
+      "externalFlowId"
+    ]
+  },
+  "option3": {
+    "data": {
+      "text": "Option 3"
+    },
+    "type": 200
+  },
+  "externalPortal2": {
+    "type": 300,
+    "edges": [
+      "externalFlowId"
+    ]
+  },
+  "firstNode": {
+    "data": {
+      "text": "This is a question"
+    },
+    "type": 100,
+    "edges": [
+      "option1",
+      "option2",
+      "option3"
+    ]
+  },
+  "option2": {
+    "data": {
+      "text": "Option 2"
+    },
+    "type": 200,
+    "edges": [
+      "externalPortal2"
+    ]
+  },
+  "finalNode": {
+    "data": {
+      "color": "#EFEFEF",
+      "title": "This is the end",
+      "resetButton": false
+    },
+    "type": 8
+  },
+  "withinExternalPortal": {
+    "data": {
+      "color": "#EFEFEF",
+      "title": "This is inside the portal",
+      "description": "<p>Hello there 👋</p>",
+      "resetButton": false
+    },
+    "type": 8
+  },
+  "option1": {
+    "data": {
+      "text": "Option 1"
+    },
+    "type": 200,
+    "edges": [
+      "externalPortal1"
+    ]
+  },
+  "externalFlowId": {
+    "data": {
+      "text": "daf-external-portal-test"
+    },
+    "type": 300,
+    "edges": [
+      "withinExternalPortal"
+    ]
+  }
+}