fix: ensure PB elements have stable element IDs (#3977)

packages/api-page-builder-import-export/__tests__/importExportTask/ElementIdsProcessor.test.ts

@@ -0,0 +1,9 @@
+import { ElementIdsProcessor } from "~/import/process/blocks/ElementIdsProcessor";
+import { inputBlock, processedBlock } from "./block.mock";
+
+describe("ElementIdsProcessor", () => {
+    it("should generate stable block element IDs", () => {
+        const processor = new ElementIdsProcessor();
+        expect(processor.process(inputBlock)).toEqual(processedBlock);
+    });
+});

packages/api-page-builder-import-export/src/import/process/blocks/ElementIdsProcessor.ts

@@ -0,0 +1,22 @@
+import { generateAlphaNumericId } from "@webiny/utils";
+import { PageContentElement } from "@webiny/api-page-builder/types";
+import { ExportedBlockData } from "~/export/utils";
+
+export class ElementIdsProcessor {
+    process(block: ExportedBlockData["block"]): ExportedBlockData["block"] {
+        return {
+            ...block,
+            content: this.ensureElementId(block.content)
+        };
+    }
+
+    private ensureElementId(element: PageContentElement): PageContentElement {
+        const id = element.id || element.data.variableId || generateAlphaNumericId(10);
+
+        return {
+            ...element,
+            id,
+            elements: element.elements.map(element => this.ensureElementId(element))
+        };
+    }
+}

packages/api-page-builder-import-export/src/import/process/blocks/importBlock.ts

@@ -12,6 +12,7 @@ import { deleteS3Folder } from "~/import/utils/deleteS3Folder";
 import { updateFilesInData } from "~/import/utils/updateFilesInData";
 import { INSTALL_EXTRACT_DIR } from "~/import/constants";
 import { ExportedBlockData } from "~/export/process/exporters/BlockExporter";
+import { ElementIdsProcessor } from "~/import/process/blocks/ElementIdsProcessor";
 
 interface ImportBlockParams {
     key: string;
@@ -45,7 +46,15 @@ export async function importBlock({
 
     // Load the block data file from disk.
     log(`Load file ${blockDataFileKey}`);
-    const { block, category, files } = await loadJson<ExportedBlockData>(BLOCK_DATA_FILE_PATH);
+
+    const {
+        category,
+        files,
+        block: rawBlock
+    } = await loadJson<ExportedBlockData>(BLOCK_DATA_FILE_PATH);
+
+    const blockProcessor = new ElementIdsProcessor();
+    const block = blockProcessor.process(rawBlock);
 
     // Only update block data if there are files.
     if (files && Array.isArray(files) && files.length > 0) {

packages/api-page-builder/__tests__/graphql/mocks/pageTemplates/simplePageBlockContent.ts

@@ -1,8 +1,13 @@
+import { PageElementId } from "~/graphql";
+
+const blockId = PageElementId.create().getValue();
+
 /**
  * Contains a grid > cell with a heading and a paragraph.
  * The heading and paragraph are both editable (linked elements).
  */
 export const simplePageBlockContent = {
+    id: blockId,
     type: "block",
     data: {
         settings: {
@@ -49,6 +54,7 @@ export const simplePageBlockContent = {
     },
     elements: [
         {
+            id: PageElementId.create().getValue(),
             type: "grid",
             data: {
                 settings: {
@@ -92,6 +98,7 @@ export const simplePageBlockContent = {
             },
             elements: [
                 {
+                    id: PageElementId.create().getValue(),
                     type: "cell",
                     data: {
                         settings: {
@@ -116,6 +123,7 @@ export const simplePageBlockContent = {
                     },
                     elements: [
                         {
+                            id: PageElementId.create().getValue(),
                             type: "heading",
                             data: {
                                 text: {
@@ -146,6 +154,7 @@ export const simplePageBlockContent = {
                             path: ["UTaSFnVtkV", "uFzaV9SB6q", "k77Fdcod55", "BOMdKQBt23"]
                         },
                         {
+                            id: PageElementId.create().getValue(),
                             type: "paragraph",
                             data: {
                                 text: {

packages/api-page-builder/__tests__/graphql/pageTemplates.test.ts

@@ -266,13 +266,13 @@ describe("Page Templates Test", () => {
                                     blockId: "DABBrS43HC",
                                     variables: [
                                         {
-                                            id: "FDEezrJ8NH",
+                                            id: "DABBrS43HC#FDEezrJ8NH",
                                             label: "Heading text",
                                             type: "heading",
                                             value: '{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"UPDATED-HEADING","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"heading-element","version":1,"tag":"h1","styles":[{"styleId":"heading1","type":"typography"}]}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'
                                         },
                                         {
-                                            id: "SezNLOdXw3",
+                                            id: "DABBrS43HC#SezNLOdXw3",
                                             label: "Paragraph text",
                                             type: "paragraph",
                                             value: '{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"UPDATED-PARAGRAPH","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph-element","version":1,"styles":[{"styleId":"paragraph1","type":"typography"}]}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'
@@ -300,16 +300,18 @@ describe("Page Templates Test", () => {
             data: {},
             elements: [
                 {
+                    id: "DABBrS43HC",
+                    type: "block",
                     data: {
                         variables: [
                             {
-                                id: "FDEezrJ8NH",
+                                id: "DABBrS43HC#FDEezrJ8NH",
                                 type: "heading",
                                 label: "Heading text",
                                 value: '{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"UPDATED-HEADING","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"heading-element","version":1,"tag":"h1","styles":[{"styleId":"heading1","type":"typography"}]}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'
                             },
                             {
-                                id: "SezNLOdXw3",
+                                id: "DABBrS43HC#SezNLOdXw3",
                                 type: "paragraph",
                                 label: "Paragraph text",
                                 value: '{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"UPDATED-PARAGRAPH","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph-element","version":1,"styles":[{"styleId":"paragraph1","type":"typography"}]}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'
@@ -337,7 +339,7 @@ describe("Page Templates Test", () => {
                                                         text: '{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"UPDATED-HEADING","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"heading-element","version":1,"tag":"h1","styles":[{"styleId":"heading1","type":"typography"}]}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'
                                                     }
                                                 },
-                                                variableId: "FDEezrJ8NH"
+                                                variableId: "DABBrS43HC#FDEezrJ8NH"
                                             }
                                         },
                                         {
@@ -353,7 +355,7 @@ describe("Page Templates Test", () => {
                                                         text: '{"root":{"children":[{"children":[{"detail":0,"format":0,"mode":"normal","style":"","text":"UPDATED-PARAGRAPH","type":"text","version":1}],"direction":null,"format":"","indent":0,"type":"paragraph-element","version":1,"styles":[{"styleId":"paragraph1","type":"typography"}]}],"direction":null,"format":"","indent":0,"type":"root","version":1}}'
                                                     }
                                                 },
-                                                variableId: "SezNLOdXw3"
+                                                variableId: "DABBrS43HC#SezNLOdXw3"
                                             }
                                         }
                                     ]

packages/api-page-builder/__tests__/graphql/pages.test.ts

@@ -1,10 +1,10 @@
+import bytes from "bytes";
 import useGqlHandler from "./useGqlHandler";
-
 import { defaultIdentity } from "../tenancySecurity";
 import { expectCompressed } from "~tests/graphql/utils/expectCompressed";
 import { decompress } from "./utils/compression";
 import { calculateSize, createPageContent } from "~tests/graphql/mocks/pageContent";
-import bytes from "bytes";
+import { PageElementId } from "~/graphql/crud/pages/PageElementId";
 
 jest.setTimeout(100000);
 
@@ -418,7 +418,12 @@ describe("CRUD Test", () => {
             data: {
                 name: "block-name",
                 blockCategory: "block-category",
-                content: { data: {}, elements: [], type: "block" }
+                content: {
+                    id: PageElementId.create().getValue(),
+                    data: {},
+                    elements: [],
+                    type: "block"
+                }
             }
         });
 
@@ -428,7 +433,12 @@ describe("CRUD Test", () => {
             data: {
                 name: "element-name",
                 type: "element",
-                content: { some: "element-content" }
+                content: {
+                    id: PageElementId.create().getValue(),
+                    type: "paragraph",
+                    data: {},
+                    elements: []
+                }
             }
         });
 
@@ -438,7 +448,7 @@ describe("CRUD Test", () => {
 
         const updatedContent = {
             ...uncompressedBlock,
-            elements: [...uncompressedBlock.content.elements, pageElementData]
+            elements: [...uncompressedBlock.content.elements, pageElementData.content]
         };
         const [updatePageBlockResult] = await updatePageBlock({
             id: blockData.id,
@@ -474,19 +484,26 @@ describe("CRUD Test", () => {
             category: "category-slug"
         });
 
-        const pageId = createPageResponse.data.pageBuilder.createPage.data.id;
+        const { id: pageId, content } = createPageResponse.data.pageBuilder.createPage.data;
 
         // Add block to the page as reference (without elements)
+        const pageBlockElementId = PageElementId.create().getValue();
+
         await updatePage({
             id: pageId,
             data: {
                 content: {
-                    data: {},
+                    ...content,
                     elements: [
-                        { data: { blockId: blockData.id }, elements: [], path: [], type: "block" }
-                    ],
-                    path: [],
-                    type: "document"
+                        ...content.elements,
+                        {
+                            id: pageBlockElementId,
+                            data: { blockId: blockData.id },
+                            elements: [],
+                            path: [],
+                            type: "block"
+                        }
+                    ]
                 }
             }
         });
@@ -501,12 +518,8 @@ describe("CRUD Test", () => {
             data: { blockId: blockData.id },
             elements: [
                 {
-                    id: pageElementData.id,
-                    name: "element-name",
-                    content: { some: "element-content" },
-                    type: "element",
-                    createdOn: expect.stringMatching(/^20/),
-                    createdBy: defaultIdentity
+                    ...pageElementData.content,
+                    id: `${pageBlockElementId}#${pageElementData.content.id}`
                 }
             ],
             path: [],

packages/api-page-builder/src/graphql/crud/pageBlocks.crud.ts

@@ -11,7 +11,9 @@ import {
     PageBlock,
     PageBlocksCrud,
     PageBlockStorageOperationsListParams,
-    PbContext
+    PbContext,
+    PageContentElement,
+    PageBlockVariable
 } from "~/types";
 import { NotFoundError } from "@webiny/handler-graphql";
 import { createTopic } from "@webiny/pubsub";
@@ -21,6 +23,7 @@ import {
 } from "~/graphql/crud/pageBlocks/validation";
 import { createZodError, mdbid, removeUndefinedValues } from "@webiny/utils";
 import { PageBlocksPermissions } from "./permissions/PageBlocksPermissions";
+import { PageElementId } from "~/graphql/crud/pages/PageElementId";
 
 export interface CreatePageBlocksCrudParams {
     context: PbContext;
@@ -287,11 +290,18 @@ export const createPageBlocksCrud = (params: CreatePageBlocksCrudParams): PageBl
         ) {
             const blocks = [];
 
-            for (const pageBlock of content?.elements) {
-                const blockId = pageBlock.data?.blockId;
-                // If block has blockId, then it is a reference block, and we need to get elements for it.
+            /**
+             * If the content block has a `blockId`, then it is a referenced block.
+             * For referenced blocks, we need to load the actual block definition, and copy the elements to the current content block.
+             * We also need to determine the appropriate block variable value to use:
+             * - if there's already a block variable value on the content block, use it.
+             * - if not, fall back to the default block variable value, which was defined during the block creation, in the Block Editor.
+             */
+            for (const contentBlock of content?.elements) {
+                const blockId = contentBlock.data?.blockId;
+
                 if (!blockId) {
-                    blocks.push(pageBlock);
+                    blocks.push(contentBlock);
                     continue;
                 }
 
@@ -303,14 +313,24 @@ export const createPageBlocksCrud = (params: CreatePageBlocksCrudParams): PageBl
                     }
                 });
 
-                // We check if the block has variable values set on the page/template, and use them
-                // in priority over the ones set inline in the block editor.
-                const blockDataVariables = blockData?.content?.data?.variables || [];
-                const variables = blockDataVariables.map((blockDataVariable: any) => {
-                    const value =
-                        pageBlock.data?.variables?.find(
-                            (variable: any) => variable.id === blockDataVariable.id
-                        )?.value || blockDataVariable.value;
+                const blockDataVariables: PageBlockVariable[] =
+                    blockData?.content?.data?.variables || [];
+
+                const contentBlockVariables: PageBlockVariable[] =
+                    contentBlock.data?.variables || [];
+
+                const variables = blockDataVariables.map(blockDataVariable => {
+                    // Check if content block has a value for the given block variable.
+                    const contentBlockVariable = contentBlockVariables.find(variable => {
+                        // We must ignore the prefix before the `#` character, as it will vary between block instances.
+                        const baseVariableId = variable.id.split("#").pop();
+                        return baseVariableId === blockDataVariable.id;
+                    });
+
+                    // Use the content block variable value, or fall back to the default block variable value.
+                    const value = contentBlockVariable
+                        ? contentBlockVariable.value
+                        : blockDataVariable.value;
 
                     return {
                         ...blockDataVariable,
@@ -320,13 +340,16 @@ export const createPageBlocksCrud = (params: CreatePageBlocksCrudParams): PageBl
 
                 blocks.push(
                     structuredClone({
-                        ...pageBlock,
+                        ...contentBlock,
                         data: {
-                            ...pageBlock?.data,
+                            ...contentBlock?.data,
                             ...blockData?.content?.data,
-                            variables
+                            variables: generateBlockVariableIds(variables, contentBlock.id)
                         },
-                        elements: blockData?.content?.elements || []
+                        elements: generateElementIds(
+                            blockData?.content?.elements || [],
+                            contentBlock.id
+                        )
                     })
                 );
             }
@@ -335,3 +358,37 @@ export const createPageBlocksCrud = (params: CreatePageBlocksCrudParams): PageBl
         }
     };
 };
+
+function generateElementIds(elements: PageContentElement[], id: string): PageContentElement[] {
+    return elements.map(element => {
+        return {
+            ...element,
+            id: `${id}#${PageElementId.create(element.id)}`,
+            elements: generateElementIds(element.elements || [], id),
+            data: prefixElementVariableId(element.data, id)
+        };
+    });
+}
+
+function generateBlockVariableIds(variables: PageBlockVariable[], blockId: string) {
+    return variables.map(variable => {
+        const variableId = variable.id.split("#").pop();
+        const newId = [blockId, variableId].join("#");
+
+        return { ...variable, id: newId };
+    });
+}
+
+function prefixElementVariableId(
+    data: PageContentElement["data"],
+    id: string
+): PageContentElement["data"] {
+    if (data?.variableId) {
+        const variableId = data.variableId.split("#").pop();
+        const newId = [id, variableId].join("#");
+
+        return { ...data, variableId: newId };
+    }
+
+    return data;
+}