Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Field validation is conflicting between block level and page level #1179

Closed
1 of 2 tasks
Jianbinzhu opened this issue Jan 30, 2024 · 10 comments
Closed
1 of 2 tasks

Field validation is conflicting between block level and page level #1179

Jianbinzhu opened this issue Jan 30, 2024 · 10 comments

Comments

@Jianbinzhu
Copy link
Contributor

Jianbinzhu commented Jan 30, 2024

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)
@GuySartorelli
Copy link
Member

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

@GuySartorelli
Copy link
Member

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 GuySartorelli closed this as not planned Won't fix, can't repro, duplicate, stale Feb 11, 2024
@Jianbinzhu
Copy link
Contributor Author

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

@GuySartorelli
Copy link
Member

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?

@maxime-rainville
Copy link
Contributor

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

@GuySartorelli
Copy link
Member

GuySartorelli commented Feb 15, 2024

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)

@maxime-rainville
Copy link
Contributor

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

#329 (comment)

@emteknetnz
Copy link
Member

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

@emteknetnz
Copy link
Member

Will be resolved by #1214

@GuySartorelli
Copy link
Member

Resolved by #1214 - will be released in 5.3.0

@GuySartorelli GuySartorelli closed this as not planned Won't fix, can't repro, duplicate, stale Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants