From fa648116b22612648674ae6af1e27611bfa7d392 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 28 Jun 2021 18:55:07 +0100 Subject: [PATCH] Correctly handle nulls in ListBlock validation errors Fixes @cnk's test case from #7248. blockErrors within a ListBlockValidationError is an array (with nulls for the items with no errors), but setError was treating it as a dict keyed by block index, which meant it was erroneously passing nulls to the child setError method. FieldBlock handles this gracefully, but other blocks such as StructBlock don't. --- .../StreamField/blocks/ListBlock.js | 11 +++++---- .../StreamField/blocks/ListBlock.test.js | 24 +++++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/client/src/components/StreamField/blocks/ListBlock.js b/client/src/components/StreamField/blocks/ListBlock.js index 4d13e97a25..cb52aa1629 100644 --- a/client/src/components/StreamField/blocks/ListBlock.js +++ b/client/src/components/StreamField/blocks/ListBlock.js @@ -130,12 +130,13 @@ export class ListBlock extends BaseSequenceBlock { } const error = errorList[0]; - // eslint-disable-next-line no-restricted-syntax - for (const blockIndex in error.blockErrors) { - if (error.blockErrors.hasOwnProperty(blockIndex)) { - this.children[blockIndex].setError(error.blockErrors[blockIndex]); + // error.blockErrors = a list with the same length as the data, + // with nulls for items without errors + error.blockErrors.forEach((blockError, blockIndex) => { + if (blockError) { + this.children[blockIndex].setError(blockError); } - } + }); } } diff --git a/client/src/components/StreamField/blocks/ListBlock.test.js b/client/src/components/StreamField/blocks/ListBlock.test.js index 823adb9644..b895038eee 100644 --- a/client/src/components/StreamField/blocks/ListBlock.test.js +++ b/client/src/components/StreamField/blocks/ListBlock.test.js @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ -import { FieldBlockDefinition } from './FieldBlock'; +import { FieldBlock, FieldBlockDefinition } from './FieldBlock'; import { ListBlockDefinition, ListBlockValidationError } from './ListBlock'; import $ from 'jquery'; @@ -43,6 +43,26 @@ class ValidationError { } } + +/* ListBlock should not call setError on its children with a null value; FieldBlock handles this +gracefully, so define a custom one that doesn't +*/ + +class ParanoidFieldBlock extends FieldBlock { + setError(errorList) { + if (!errorList) { + throw new Error('ParanoidFieldBlock.setError was passed a null errorList'); + } + return super.setError(errorList); + } +} + +class ParanoidFieldBlockDefinition extends FieldBlockDefinition { + render(placeholder, prefix, initialState, initialError) { + return new ParanoidFieldBlock(this, placeholder, prefix, initialState, initialError); + } +} + describe('telepath: wagtail.blocks.ListBlock', () => { let boundBlock; @@ -57,7 +77,7 @@ describe('telepath: wagtail.blocks.ListBlock', () => { // Define a test block const blockDef = new ListBlockDefinition( 'test_listblock', - new FieldBlockDefinition( + new ParanoidFieldBlockDefinition( '', new DummyWidgetDefinition('The widget'), {