Implement validation method for inline editable form

[brynwhyman]

Validation messaging should be handled within the custom form React component as opposed to at a page level.

There are currently two ways to submit content for validation - you can save the page, or you can save an individual block.

Acceptance criteria

• In-line editable content block are validated with form schema as outline in the SPIKE.
• Validation occurs both on page save and on individual block save. (new issue created)
• API for enforcing validation of content block is inline with the one to validate pages.
• Validation works both for inline block and "not inline" block. (new issue created)
• Official elemental doc provides clear guidance on how to validate elemental block.

Note

• Need to create a follow up UX card for handling block that are broken and general display of validation errors.
  • @GuySartorelli has some good questions about what the UI should do with validation errors.
• Can we constrained this card to validating a block that is currently being edited?
• Related card: Field validation is conflicting between block level and page level #1179

SPIKE

There was a spike to investigate this (which includes a POC PR):

• SPIKE Come up with an approach to validate inline blocks #1068

New issues created

• FormBuilder validation message does not show for TextCheckboxGroupField #1154
• Non-inline validation only half works #1155

PRs

• NEW Validation of fields when inline saving #1150
• FIX Enable bypassing entwine onsubmit handler silverstripe-admin#1685
• DOC Add elemental block validation to 5.3.0 changelog developer-docs#466
• DOC Update model validation docs developer-docs#467
• NEW Extensions for elemental silverstripe-frameworktest#169
• API Remove deprecated code #1169

Multi-PR CI run

• https://github.com/emteknetnz/silverstripe-installer/actions/runs/8610397572

[clarkepaul]

We have some initial designs for the notification placement but we haven't gone through each scenario to figure out what the notifications should say in each case. Please refer to the Style Guide for guidance https://projects.invisionapp.com/dsm/silver-stripe/silver-stripe

[ScopeyNZ]

Note at this point we'll ideally have completed silverstripe/silverstripe-admin#327 which implements a custom React form component. With this (and the messages that are a part of FormBuilderLoader) we should be able to expose the messages we get back and ship them up to the parent component.

[chrispenny]

Hi team,

Does this issue also cover adding support for getCMSValidator or (the new) getCMSCompositeValidator with inline edited Elements?

At the moment, it seems like the only way to have validation results for Elements (that are inline edited) is to use the validate() extension point. But, even these are not displayed in the inline editor, and it has a pretty bad side effect, which is already tracked here:
#764

Loosely related:
#840

[brynwhyman]

Sorry this issue is light on detail. It needs a new description to cover how we'd intend on this being implemented, which I'd need some help with.

While looking into silverstripe/silverstripe-admin#840 is was clear that there's a number of loosely related issues, so @Cheddam proposed we step back to look at this holistically: SPIKE Broken form state in EditForms following validation errors

[chrispenny]

Do we have any sort of time frame that we could expect Validation messages to be support by Inline Editing?

Currently we end up needing to disable this feature quite regularly, as our Blocks have required fields. As such, Inline Editing is a bit of underused feature at the moment (for me, at least).

[brynwhyman]

Hi @chrispenny this would be blocked until we get some time to dig deeper in to the broader issues with inline block state management in silverstripe/silverstripe-admin#1131

At this stage I don't have a time frame for when our team might look into this but I'll push it up our backlog and add an update to this issue in a month or so.

[GuySartorelli]

If we add the following to ElementalAreaField, saving the page will also run validation against form validators in each elemental block (i.e. returned from getCMSCompositeValidator() or getCMSValidator())

public function validate($validator)
{
    $valid = parent::validate($validator);

    $elementData = $this->Value();

    if (!$elementData) {
        return $valid;
    }

    // Ensure CMSCompositeValidator validation is run for each inline-editable element.
    $idPrefixLength = strlen(sprintf(ElementalAreaController::FORM_NAME_TEMPLATE ?? '', ''));
    foreach ($elementData as $form => $data) {
        // Extract the ID
        $elementId = (int) substr($form ?? '', $idPrefixLength ?? 0);

        /** @var BaseElement $element */
        $element = $this->getArea()->Elements()->byID($elementId);

        if (!$element) {
            // Ignore broken elements
            continue;
        }

        $form = ElementalAreaController::singleton()->getElementFormForValidation($elementId);

        $form->loadDataFrom(ElementalAreaController::removeNamespacesFromFields($data, $elementId));
        $result = $form->getValidator()->validate();

        if (!$result->isValid()) {
            $valid = false;
            $fields = $this->mapFieldNamesToNamespace($data, $elementId);
            foreach ($result->getMessages() as $metadata) {
                // TODO: Not all messages are field errors
                $validator->validationError($fields[$metadata['fieldName']], $metadata['message']);
            }
        }
    }

    return $valid;
}

private function mapFieldNamesToNamespace(array $data, $elementID): array
{
    $output = [];
    $template = sprintf(EditFormFactory::FIELD_NAMESPACE_TEMPLATE, $elementID, '');
    foreach (array_keys($data) as $namespacedField) {
        // Only look at fields which belong to this element
        if (substr($namespacedField, 0, strlen($template)) !== $template) {
            continue;
        }

        $fieldName = substr($namespacedField, strlen($template));
        $output[$fieldName] = $namespacedField;
    }
    return $output;
}

[GuySartorelli]

There are some UX considerations we need to consider before implementing this, such as:

• Do invalid blocks get expanded automatically?
• Do invalid blocks get highlighted e.g. with a red border?
• How do we display validation errors that apply for a block but aren't for a given field?
• How do we deal with validation errors in a separate tab on of the block (e.g. settings tab on the block)?
• What about validation errors for the ElementalArea field itself? Is that in scope?
  • Should those be displayed above (non-standard but means those messages are easier to see and relate to the elemental area) or below (more standard but content authors won't notice them until they scroll past the elemental area)?
• Should we display a summary of all validation errors in some central place? IIRC that's what you're meant to do for accessibility - though that should probably be for the form (or at least the current tab of the form) as a whole, not just for the elemental area?)
• Presumably when saving a block from the ... menu on the block itself, its validation error status will change independently from all other blocks (i.e. other blocks which already had visible validation errors will still have those visible validation errors)

[maxime-rainville]

Following up from our refinement session yesterday, I wanted to validate how the form for each inline block was being fetch.

When you start editing a block, it fires a request to /admin/elemental-area/schema/elementForm/2, where 2 is the ID of the block you are editing. This returns a form schema response.

The schema is being generated at

silverstripe-elemental/src/Controllers/ElementalAreaController.php

Lines 84 to 107 in 5dfe7e2

	public function getElementForm($elementID)
	{
	$scaffolder = Injector::inst()->get(EditFormFactory::class);
	$element = BaseElement::get()->byID($elementID);
	if (!$element) {
	return null;
	}
	/** @var Form $form */
	$form = $scaffolder->getForm(
	$this,
	sprintf(static::FORM_NAME_TEMPLATE ?? '', $elementID),
	['Record' => $element]
	);
	if (!$element->canEdit()) {
	$form->makeReadonly();
	}
	$form->addExtraClass('element-editor-editform__form');
	return $form;
	}

There's a separate endpoint for saving the inline form.

I'm thinking maybe the way to approach this is to come up with a convention for deciding how to validate data entered in a form schema and/or formalise how form schema should be used.

[GuySartorelli]

How the form is provided to be rendered is almost irrelevant, unless that's being provided completely fresh after a form submission and after saving a block, and there's a clean way to correctly pass validation messages through that pipe.

What matters more IMO is what happens at submission time, i.e.

• How is the data submitted?
• How is that submission handled?
• What sort of response is given to that submission, and how do we send validation details down that line?

The first two questions there are already answered in this issue's description - and part of the problem is that there's two different types of form submission depending on whether a whole page or just one block is being saved.

Yes, standardisation around how forms are handled across the board would be awesome. I don't know if that's something we can do within the lifetime of CMS 5 though. Do we want to hold this back until CMS 6?

[maxime-rainville]

We've been talking about this for a bit and can't come up with an obvious way to do this upfront. So we've schedule a SPIKE for it #1068

[GuySartorelli]

Merged green PRs - reassigning to Steve to do whatever needs doing with the draft PR

[GuySartorelli]

PRs merged