silverstripe/silverstripe-elemental

Field validation is conflicting between block level and page level

Closed this issue · 10 comments

Module version(s) affected

5.1.1

Description

Here's an edge case that would cause the validation message to be confusing, hopefully this is the right place to raise this issue.

I have a block that has a required Title field, and when I hit publish on the page level, the validation message is attached to the page Title.

How to reproduce

  • Create a new page
  • Fill in all required fields on the page level
  • Without publishing the page
  • Add a new block with the validation below
  • Publish the page

This validation is from the block (BaseElement)

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

        if ($this->isInDB() && trim($this->Title ?? '') === '') {
            $validator->addFieldError('Title', 'Title is required (this message is coming from block level)');
        }

        return $validator;
    }

You would see that the validation message attached to the page field is coming from the block.

Screenshot 2024-01-30 at 9 01 11 AM

Possible Solution

No response

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

Validation of inline-editable blocks doesn't work at all right now (see #699 and #329) - does this only happen with inline-editable blocks?

Closing under the assumption that this does indeed only happen with inline-editable blocks.
If that's not the case, let me know and I'll reopen the issue.

@GuySartorelli This is also happening to non inline-editable blocks.

Thanks for that extra context. I've reopened and labelled as a bug.
Would you like to take a crack at a PR to fix this?

For my dumb monkey brain, the $this->isInDB() needs to be there otherwise you can't initialise the a new block

I'm tempted to say that maybe we fold this back in this other card. #329

For my dumb monkey brain, the $this->isInDB() needs to be there otherwise you can't initialise the a new block

That's entirely irrelevant to the bug, which is that a field error for the block is being displayed against the page.

This bug should be fixable (i.e. don't display the error against the page's field because it has nothing to do with that field) without implementing validation for the block itself (which is out of scope, and that part will explicitly be handled in the card you've linked to)

@GuySartorelli had a lot of great question on this related card

#329 (comment)

Related - contains a little research on the current state of validation on elemental - #329 (comment)

Will be resolved by #1214

Resolved by #1214 - will be released in 5.3.0