Add getDuplicatedState methods to block classes

Fixes duplication of block ids when duplicating stream field blocks
2022-08-30 14:32:49 +01:00 · 2022-08-30 14:32:49 +01:00 · 6f1fde7cfc
commit 6f1fde7cfc
--- a/.eslintignore
+++ b/.eslintignore
@ -14,3 +14,4 @@ wagtail/snippets/static
 wagtail/users/static
 wagtail/contrib/*/static
 wagtail/contrib/search_promotions/templates/wagtailsearchpromotions/includes/searchpromotions_formset.js
+.mypy_cache
--- a/.prettierignore
+++ b/.prettierignore
@ -12,3 +12,4 @@ _build
 # Files which contain incompatible syntax.
 *.html
 wagtail/contrib/search_promotions/templates/wagtailsearchpromotions/includes/searchpromotions_formset.js
+.mypy_cache
--- a/client/src/components/StreamField/blocks/StreamBlock.js
+++ b/client/src/components/StreamField/blocks/StreamBlock.js
@ -31,6 +31,17 @@ class StreamChild extends BaseSequenceChild {
    };
  }

+  getDuplicatedState() {
+    return {
+      type: this.type,
+      value:
+        this.block.getDuplicatedState === undefined
+          ? this.block.getState()
+          : this.block.getDuplicatedState(),
+      id: uuidv4(),
+    };
+  }
+
  setState({ type, value, id }) {
    this.type = type;
    this.block.setState(value);
@ -377,14 +388,17 @@ export class StreamBlock extends BaseSequenceBlock {

  duplicateBlock(index, opts) {
    const child = this.children[index];
-    const childState = child.getState();
+    const childState = child.getDuplicatedState();
    const animate = opts && opts.animate;
-    childState.id = null;
    this.insert(childState, index + 1, { animate, collapsed: child.collapsed });
    // focus the newly added field if we can do so without obtrusive UI behaviour
    this.children[index + 1].focus({ soft: true });
  }

+  getDuplicatedState() {
+    return this.children.map((streamChild) => streamChild.getDuplicatedState());
+  }
+
  splitBlock(index, valueBefore, valueAfter, shouldMoveCommentFn, opts) {
    const child = this.children[index];
    const animate = opts && opts.animate;
--- a/client/src/components/StreamField/blocks/StreamBlock.test.js
+++ b/client/src/components/StreamField/blocks/StreamBlock.test.js
@ -1,9 +1,16 @@
 import $ from 'jquery';
+import * as uuid from 'uuid';
 import { FieldBlockDefinition } from './FieldBlock';
 import {
  StreamBlockDefinition,
  StreamBlockValidationError,
 } from './StreamBlock';
+import { StructBlockDefinition } from './StructBlock';
+
+// Mock uuid for consistent snapshot results
+jest.mock('uuid');
+const uuidSpy = jest.spyOn(uuid, 'v4');
+uuidSpy.mockReturnValue('fake-uuid-v4-value');

 window.$ = $;

@ -330,6 +337,134 @@ describe('telepath: wagtail.blocks.StreamBlock', () => {
  });
 });

+describe('telepath: wagtail.blocks.StreamBlock with nested stream blocks', () => {
+  let boundBlock;
+
+  beforeEach(() => {
+    // Define a test block
+    const innerStreamDef = new StreamBlockDefinition(
+      'inner_stream',
+      [
+        [
+          '',
+          [
+            new FieldBlockDefinition(
+              'test_block_a',
+              new DummyWidgetDefinition('Block A Widget'),
+              {
+                label: 'Test Block A',
+                required: false,
+                icon: 'pilcrow',
+                classname:
+                  'w-field w-field--char_field w-field--admin_auto_height_text_input',
+              },
+            ),
+          ],
+        ],
+      ],
+      {},
+      {
+        label: 'Inner Stream',
+        required: false,
+        icon: 'placeholder',
+        classname: null,
+        helpText: '',
+        helpIcon: '',
+        maxNum: null,
+        minNum: null,
+        blockCounts: {},
+        strings: {
+          MOVE_UP: 'Move up',
+          MOVE_DOWN: 'Move down',
+          DELETE: 'Delete',
+          DUPLICATE: 'Duplicate',
+          ADD: 'Add',
+        },
+      },
+    );
+
+    const blockDef = new StreamBlockDefinition(
+      '',
+      [
+        [
+          '',
+          [
+            new StructBlockDefinition(
+              'struct_with_inner_stream',
+              [innerStreamDef],
+              {
+                label: 'Struct with inner stream',
+                required: false,
+                icon: 'placeholder',
+                classname: 'struct-block',
+                helpText: '',
+                helpIcon: '',
+              },
+            ),
+          ],
+        ],
+      ],
+      {},
+      {
+        label: '',
+        required: true,
+        icon: 'placeholder',
+        classname: null,
+        helpText: 'use <strong>plenty</strong> of these',
+        helpIcon: '<div class="icon-help">?</div>',
+        maxNum: null,
+        minNum: null,
+        blockCounts: {},
+        strings: {
+          MOVE_UP: 'Move up',
+          MOVE_DOWN: 'Move down',
+          DELETE: 'Delete',
+          DUPLICATE: 'Duplicate',
+          ADD: 'Add',
+        },
+      },
+    );
+
+    // Render it
+    document.body.innerHTML = '<div id="placeholder"></div>';
+    boundBlock = blockDef.render($('#placeholder'), 'the-prefix', []);
+  });
+
+  test('duplicateBlock does not duplicate block ids', () => {
+    // Insert an instance of our struct block
+    boundBlock.insert(
+      {
+        type: 'struct_with_inner_stream',
+        value: { inner_stream: [] },
+        id: 'struct-1',
+      },
+      0,
+    );
+
+    // Insert a block into its nested stream field
+    boundBlock.children[0].block.childBlocks.inner_stream.insert(
+      {
+        type: 'test_block_a',
+        value: 'hello',
+        id: 'inner-id-1',
+      },
+      0,
+    );
+
+    // Duplicate the struct block (outermost) instance
+    boundBlock.children[0].duplicate();
+
+    expect(boundBlock.children[1].id).not.toBeNull();
+    expect(boundBlock.children[1].id).not.toEqual(boundBlock.children[0].id);
+
+    expect(
+      boundBlock.children[1].block.childBlocks.inner_stream.children[0].id,
+    ).not.toEqual(
+      boundBlock.children[0].block.childBlocks.inner_stream.children[0].id,
+    );
+  });
+});
+
 describe('telepath: wagtail.blocks.StreamBlock with labels that need escaping', () => {
  let boundBlock;

--- a/client/src/components/StreamField/blocks/StructBlock.js
+++ b/client/src/components/StreamField/blocks/StructBlock.js
@ -111,6 +111,19 @@ export class StructBlock {
    return state;
  }

+  getDuplicatedState() {
+    const state = {};
+    // eslint-disable-next-line guard-for-in
+    for (const name in this.childBlocks) {
+      const block = this.childBlocks[name];
+      state[name] =
+        block.getDuplicatedState === undefined
+          ? block.getState()
+          : block.getDuplicatedState();
+    }
+    return state;
+  }
+
  getValue() {
    const value = {};
    // eslint-disable-next-line guard-for-in
--- a/client/src/components/StreamField/blocks/snapshots/StreamBlock.test.js.snap
+++ b/client/src/components/StreamField/blocks/snapshots/StreamBlock.test.js.snap
@ -144,11 +144,11 @@ exports[`telepath: wagtail.blocks.StreamBlock blocks can be duplicated 1`] = `
        <div data-streamblock-menu-outer=\\"\\" style=\\"display: none;\\" aria-hidden=\\"true\\">
          <div data-streamblock-menu-inner=\\"\\" class=\\"c-sf-add-panel\\"></div>
        </div>
-      </div><div aria-hidden=\\"false\\" data-contentpath-disabled=\\"\\">
+      </div><div aria-hidden=\\"false\\" data-contentpath=\\"fake-uuid-v4-value\\">
        <input type=\\"hidden\\" name=\\"the-prefix-2-deleted\\" value=\\"\\">
        <input type=\\"hidden\\" name=\\"the-prefix-2-order\\" value=\\"2\\">
        <input type=\\"hidden\\" name=\\"the-prefix-2-type\\" value=\\"test_block_b\\">
-        <input type=\\"hidden\\" name=\\"the-prefix-2-id\\" value=\\"\\">
+        <input type=\\"hidden\\" name=\\"the-prefix-2-id\\" value=\\"fake-uuid-v4-value\\">

        <div>
          <div class=\\"c-sf-container__block-container\\">
--- a/client/src/entrypoints/contrib/typed_table_block/typed_table_block.js
+++ b/client/src/entrypoints/contrib/typed_table_block/typed_table_block.js
@ -466,10 +466,7 @@ export class TypedTableBlock {

  getState() {
    const state = {
-      columns: this.columns.map((column) => ({
-        type: column.blockDef.name,
-        heading: column.headingInput.value,
-      })),
+      columns: this.getColumnStates(),
      rows: this.rows.map((row) => ({
        values: row.blocks.map((block) => block.getState()),
      })),
@ -477,12 +474,22 @@ export class TypedTableBlock {
    return state;
  }

+  getDuplicatedState() {
+    return {
+      columns: this.getColumnStates(),
+      rows: this.rows.map((row) => ({
+        values: row.blocks.map((block) =>
+          block.getDuplicatedState === undefined
+            ? block.getState()
+            : block.getDuplicatedState(),
+        ),
+      })),
+    };
+  }
+
  getValue() {
    const value = {
-      columns: this.columns.map((column) => ({
-        type: column.blockDef.name,
-        heading: column.headingInput.value,
-      })),
+      columns: this.getColumnStates(),
      rows: this.rows.map((row) => ({
        values: row.blocks.map((block) => block.getValue()),
      })),
@ -490,6 +497,13 @@ export class TypedTableBlock {
    return value;
  }

+  getColumnStates() {
+    return this.columns.map((column) => ({
+      type: column.blockDef.name,
+      heading: column.headingInput.value,
+    }));
+  }
+
  getTextLabel(opts) {
    /* Use as many child text labels as we can fit into maxLength */
    const maxLength = opts && opts.maxLength;
--- a/client/src/entrypoints/contrib/typed_table_block/typed_table_block.test.js
+++ b/client/src/entrypoints/contrib/typed_table_block/typed_table_block.test.js
@ -2,6 +2,7 @@ import '../../admin/telepath/telepath';
 import $ from 'jquery';
 import { TypedTableBlockDefinition } from './typed_table_block';
 import { FieldBlockDefinition } from '../../../components/StreamField/blocks/FieldBlock';
+import { StreamBlockDefinition } from '../../../components/StreamField/blocks/StreamBlock';

 window.$ = $;

@ -191,3 +192,150 @@ describe('wagtail.contrib.typed_table_block.blocks.TypedTableBlock', () => {
    expect(document.getElementsByName('mytable-row-count')[0].value).toBe('3');
  });
 });
+
+describe('wagtail.contrib.typed_table_block.blocks.TypedTableBlock in StreamBlock', () => {
+  let boundBlock;
+  let innerStreamDef;
+
+  beforeEach(() => {
+    innerStreamDef = new StreamBlockDefinition(
+      'inner_stream',
+      [
+        [
+          '',
+          [
+            new FieldBlockDefinition(
+              'test_block_a',
+              new DummyWidgetDefinition('Block A Widget'),
+              {
+                label: 'Test Block A',
+                required: false,
+                icon: 'pilcrow',
+                classname:
+                  'w-field w-field--char_field w-field--admin_auto_height_text_input',
+              },
+            ),
+          ],
+        ],
+      ],
+      {},
+      {
+        label: 'Inner Stream',
+        required: false,
+        icon: 'placeholder',
+        classname: null,
+        helpText: '',
+        helpIcon: '',
+        maxNum: null,
+        minNum: null,
+        blockCounts: {},
+        strings: {
+          MOVE_UP: 'Move up',
+          MOVE_DOWN: 'Move down',
+          DELETE: 'Delete',
+          DUPLICATE: 'Duplicate',
+          ADD: 'Add',
+        },
+      },
+    );
+
+    const streamBlockDef = new StreamBlockDefinition(
+      '',
+      [
+        [
+          '',
+          [
+            new TypedTableBlockDefinition(
+              'table',
+              [innerStreamDef],
+              {},
+              {
+                label: '',
+                required: true,
+                icon: 'placeholder',
+                classname: null,
+                helpText: 'use <strong>plenty</strong> of these',
+                helpIcon: '<div class="icon-help">?</div>',
+                strings: {
+                  ADD_COLUMN: 'Add column',
+                  ADD_ROW: 'Add row',
+                  COLUMN_HEADING: 'Column heading',
+                  INSERT_COLUMN: 'Insert column',
+                  DELETE_COLUMN: 'Delete column',
+                  INSERT_ROW: 'Insert row',
+                  DELETE_ROW: 'Delete row',
+                },
+              },
+            ),
+          ],
+        ],
+      ],
+      {},
+      {
+        label: '',
+        required: true,
+        icon: 'placeholder',
+        classname: null,
+        helpText: 'use <strong>plenty</strong> of these',
+        helpIcon: '<div class="icon-help">?</div>',
+        maxNum: null,
+        minNum: null,
+        blockCounts: {},
+        strings: {
+          MOVE_UP: 'Move up',
+          MOVE_DOWN: 'Move down',
+          DELETE: 'Delete',
+          DUPLICATE: 'Duplicate',
+          ADD: 'Add',
+        },
+      },
+    );
+
+    // Render it
+    document.body.innerHTML = '<div id="placeholder"></div>';
+    boundBlock = streamBlockDef.render($('#placeholder'), 'the-prefix', []);
+  });
+
+  test('duplicateBlock does not duplicate stream block ids in typed table blocks', () => {
+    // Insert a typed table block at the top level
+    boundBlock.insert(
+      {
+        type: 'table',
+        value: { rows: [], columns: [] },
+        id: 'old-id-1',
+      },
+      0,
+    );
+
+    const tableBlock = boundBlock.children[0].block;
+
+    // Add a column and row for the nested stream block
+    tableBlock.insertColumn(0, innerStreamDef);
+    tableBlock.insertRow(0);
+    const innerStreamBlock = tableBlock.rows[0].blocks[0];
+
+    // Insert a block into the inner stream
+    innerStreamBlock.insert(
+      { type: 'test_block_a', value: 'foobar', id: 'old-inner-stream-id-1' },
+      0,
+    );
+
+    // Duplicate the outermost block (typed table block)
+    boundBlock.duplicateBlock(0);
+
+    // Check the ids on the top level blocks
+    expect(boundBlock.children[1].id).not.toBeNull();
+    expect(boundBlock.children[1].id).not.toEqual(boundBlock.children[0].id);
+
+    // Check the ids on the nested blocks
+    expect(
+      boundBlock.children[1].block.rows[0].blocks[0].children[0].id,
+    ).not.toBeNull();
+
+    expect(
+      boundBlock.children[1].block.rows[0].blocks[0].children[0].id,
+    ).not.toEqual(
+      boundBlock.children[0].block.rows[0].blocks[0].children[0].id,
+    );
+  });
+});