From 26e779d607ef6a7b31c04ca54046d13d74262b3d Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 20 Feb 2024 15:47:57 +0100 Subject: [PATCH] dataMerged() tests should share same mock data as flattenFlows() --- api.planx.uk/helpers.test.ts | 218 +++++------------- api.planx.uk/helpers.ts | 19 +- .../FlowEditor/components/PreviewBrowser.tsx | 1 - 3 files changed, 62 insertions(+), 176 deletions(-) diff --git a/api.planx.uk/helpers.test.ts b/api.planx.uk/helpers.test.ts index 076016a27d..7bd7374c45 100644 --- a/api.planx.uk/helpers.test.ts +++ b/api.planx.uk/helpers.test.ts @@ -6,10 +6,13 @@ import { isLiveEnv, } from "./helpers"; import { queryMock } from "./tests/graphqlQueryMock"; -import { userContext } from "./modules/auth/middleware"; -import { getJWT } from "./tests/mockJWT"; +import { + childFlow, + draftParentFlow, + flattenedParentFlow, +} from "./tests/mocks/validateAndPublishMocks"; -describe("getEnvironment function", () => { +describe("getFormattedEnvironment() function", () => { const OLD_ENV = process.env; beforeEach(() => { @@ -69,183 +72,67 @@ describe("isLiveEnv() function", () => { }); describe("dataMerged() function", () => { - const getStoreMock = jest.spyOn(userContext, "getStore"); - getStoreMock.mockReturnValue({ - user: { - sub: "123", - jwt: getJWT({ role: "teamEditor" }), - }, - }); - 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 invalidUnflattenedParent = { - _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: "unpublished-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: "unpublished-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", + matchOnVariables: true, variables: { - id: "child-id", + id: "parent-flow-with-external-portal", }, data: { flow: { - slug: "child-flow", - data: unflattenedChild, - team_id: 123, + data: draftParentFlow, + slug: "parent-flow", + team_id: 1, team: { slug: "testing", }, - publishedFlows: [ - { - data: unflattenedChild, - }, - ], + publishedFlows: [{ data: flattenedParentFlow }], }, }, }); + }); + it("flattens published external portal nodes by overwriting their type", async () => { queryMock.mockQuery({ name: "GetFlowData", + matchOnVariables: true, variables: { - id: "parent-id", + id: "child-flow-id", }, data: { flow: { - slug: "parent-flow", - data: unflattenedParent, - team_id: 123, + data: childFlow, + slug: "child-flow", + team_id: 1, team: { slug: "testing", }, + publishedFlows: [{ data: childFlow }], }, }, }); + const result = await dataMerged("parent-flow-with-external-portal"); + const nodeTypes = Object.values(result).map((node) => + "type" in node ? node.type : undefined, + ); + + expect(nodeTypes.includes(ComponentType.ExternalPortal)).toBe(false); + }); + + it("throws an error when an external portal is not published", async () => { queryMock.mockQuery({ name: "GetFlowData", + matchOnVariables: true, variables: { - id: "unpublished-child-id", + id: "child-flow-id", }, data: { flow: { - slug: "unpublished-child-flow", - data: unflattenedChild, - team_id: 123, + data: childFlow, + slug: "child-flow", + team_id: 1, team: { slug: "testing", }, @@ -254,39 +141,38 @@ describe("dataMerged() function", () => { }, }); + await expect( + dataMerged("parent-flow-with-external-portal"), + ).rejects.toThrow(); + }); + + it("flattens any published or unpublished external portal nodes when isDraftData only is set to true", async () => { queryMock.mockQuery({ name: "GetFlowData", + matchOnVariables: true, variables: { - id: "invalid-parent-id", + id: "child-flow-id", }, data: { flow: { - slug: "invalid-parent-flow", - data: invalidUnflattenedParent, - team_id: 123, + data: childFlow, + slug: "child-flow", + team_id: 1, team: { slug: "testing", }, + publishedFlows: [], }, }, }); - }); - it("handles multiple external portal nodes", async () => { - const result = await dataMerged("parent-id"); - const nodeTypes = Object.values(result).map((node) => - "type" in node ? node.type : undefined, - ); - const areAllPortalsFlattened = !nodeTypes.includes( - ComponentType.ExternalPortal, + const result = await dataMerged( + "parent-flow-with-external-portal", + {}, + false, + true, ); - - // All external portals have been flattened / replaced - expect(areAllPortalsFlattened).toBe(true); - }); - - it("throws an error when an external portal is not published", async () => { - await expect(dataMerged("invalid-parent-id")).rejects.toThrow(); + expect(result).toEqual(flattenedParentFlow); }); }); diff --git a/api.planx.uk/helpers.ts b/api.planx.uk/helpers.ts index c2c23c5523..3122695d5c 100644 --- a/api.planx.uk/helpers.ts +++ b/api.planx.uk/helpers.ts @@ -153,21 +153,23 @@ const getMostRecentPublishedFlow = async ( return mostRecent; }; -// Flatten a flow's data to include main content & portals in a single JSON representation -// XXX: getFlowData & dataMerged are currently repeated in ../editor.planx.uk/src/lib/dataMergedHotfix.ts -// in order to load frontend /preview routes for flows that are not published +/** + * Flatten a flow to create a single JSON representation of the main flow data and any external portals + * By default, requires that any external portals are published and flattens their latest published version + * When draftDataOnly = true, flattens the draft data of the main flow and the draft data of any external portals published or otherwise + */ const dataMerged = async ( id: string, ob: { [key: string]: Node } = {}, isPortal = false, draftDataOnly = false, ): Promise => { - // get the primary draft flow data, checking for the latest published version of external portals + // get the primary draft flow data, including its' latest published version const response = await getFlowData(id); const { slug, team, publishedFlows } = response; let { data } = response; - // only flatten portals that are published, unless we're loading all draft data on an /unpublished route + // only flatten external portals that are published, unless we're loading draftDataOnly on an /unpublished route if (isPortal && !draftDataOnly) { if (publishedFlows?.[0]?.data) { data = publishedFlows[0].data; @@ -185,7 +187,7 @@ const dataMerged = async ( const isExternalPortal = node.type === ComponentType.ExternalPortal; const isMerged = ob[node.data?.flowId]; - // Merge portal root as a new node in the graph + // merge external portal _root as a new node in the graph if (isExternalPortalRoot) { ob[id] = { ...node, @@ -194,7 +196,7 @@ const dataMerged = async ( }; } - // Merge as internal portal, with reference to flowId + // merge external portal as an internal portal type node, with reference to flowId else if (isExternalPortal) { ob[nodeId] = { type: ComponentType.InternalPortal, @@ -207,11 +209,10 @@ const dataMerged = async ( } } - // Merge all other nodes + // merge all other nodes else ob[nodeId] = node; } - // TODO: Don't cast here once types updated across API return ob as FlowGraph; }; diff --git a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx index 064b859ceb..570a42d3ff 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx @@ -235,7 +235,6 @@ const PreviewBrowser: React.FC<{ try { setLastPublishedTitle("Checking for changes..."); const alteredFlow = await validateAndDiffFlow(flowId); - console.log("here", alteredFlow); setAlteredNodes( alteredFlow?.data.alteredNodes ? alteredFlow.data.alteredNodes