From 45edb4ccf72c74aa2c57cd0fdc911b06f8ed01d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 15 Sep 2023 13:05:16 +0100 Subject: [PATCH] fix: Handle multiple external portals as siblings --- api.planx.uk/helpers.test.ts | 112 +++++++++++++++++- api.planx.uk/helpers.ts | 32 +++-- editor.planx.uk/src/lib/dataMergedHotfix.ts | 40 +++++-- .../lib/__tests__/externalPortals.test.ts | 63 ++++++++++ .../mocks/multipleExternalPortals.json | 81 +++++++++++++ .../__tests__/mocks/singleExternalPortal.json | 65 ++++++++++ 6 files changed, 372 insertions(+), 21 deletions(-) create mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/externalPortals.test.ts create mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/multipleExternalPortals.json create mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/singleExternalPortal.json diff --git a/api.planx.uk/helpers.test.ts b/api.planx.uk/helpers.test.ts index 45e5966865..653cec6185 100644 --- a/api.planx.uk/helpers.test.ts +++ b/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: "

Hello there 👋

", + 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); + }); +}); diff --git a/api.planx.uk/helpers.ts b/api.planx.uk/helpers.ts index 4e0d9771db..f0894a1098 100644 --- a/api.planx.uk/helpers.ts +++ b/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 => { @@ -138,24 +139,39 @@ const dataMerged = async (id: string, ob: Record = {}) => { // 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; }; diff --git a/editor.planx.uk/src/lib/dataMergedHotfix.ts b/editor.planx.uk/src/lib/dataMergedHotfix.ts index daa41fa042..2e1d40c309 100644 --- a/editor.planx.uk/src/lib/dataMergedHotfix.ts +++ b/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 = {}, -): Promise> => { - const { slug, data }: { slug: string; data: Record } = - 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 = {}) => { + // get the primary flow data + const { slug, data }: { slug: string; data: Record } = 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; }; diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/externalPortals.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/externalPortals.test.ts new file mode 100644 index 0000000000..564469d48e --- /dev/null +++ b/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"); + }); +}); \ No newline at end of file diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/multipleExternalPortals.json b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/multipleExternalPortals.json new file mode 100644 index 0000000000..b2693fa5e4 --- /dev/null +++ b/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": "

Hello there 👋

", + "resetButton": false + }, + "type": 8 + }, + "option1": { + "data": { + "text": "Option 1" + }, + "type": 200, + "edges": [ + "externalPortal1" + ] + }, + "externalFlowId": { + "data": { + "text": "daf-external-portal-test" + }, + "type": 300, + "edges": [ + "withinExternalPortal" + ] + } +} \ No newline at end of file diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/singleExternalPortal.json b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/singleExternalPortal.json new file mode 100644 index 0000000000..dea585a6dc --- /dev/null +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/mocks/singleExternalPortal.json @@ -0,0 +1,65 @@ +{ + "_root": { + "edges": [ + "firstNode", + "finalNode" + ] + }, + "option2": { + "data": { + "text": "Option 2" + }, + "type": 200 + }, + "finalNode": { + "data": { + "color": "#EFEFEF", + "title": "End of the line", + "resetButton": false + }, + "type": 8 + }, + "externalPortal": { + "type": 300, + "edges": [ + "externalFlowId" + ] + }, + "firstNode": { + "data": { + "text": "This is a question with many options" + }, + "type": 100, + "edges": [ + "option1", + "option2" + ] + }, + "option1": { + "data": { + "text": "Option 1" + }, + "type": 200, + "edges": [ + "externalPortal" + ] + }, + "withinExternalPortal": { + "data": { + "color": "#EFEFEF", + "title": "This is an external portal", + "description": "

Hello! 👋

", + "resetButton": false + }, + "type": 8 + }, + "externalFlowId": { + "data": { + "text": "test-external-portal" + }, + "type": 300, + "edges": [ + "withinExternalPortal" + ] + } +} \ No newline at end of file