Validation not compatible with adding blocks inline in 4.0

[thats4shaw]

In the past i've relied on some light validation for elements to ensure some particular content is present.

For example:

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

    if (!$this->ImageID) {
        $validation->addError('Image is required');
    }

    return $validation;
}

When adding an element inline with validation like above above in 4.0, it throws a console error - Uncaught (in promise) Error: GraphQL error: Image is required. This does makes sense as you are in fact saving the element.

I've also tried leveraging getCMSValidator:

public function getCMSValidator()
{
    return RequiredFields::create([
        'ImageID',
    ]);
}

However this doesn't trigger on save/publish. It does however looks like it gets initialised when adding a block.

A scenario such as above could easily be swapped out for template logic but I'm sure there will be a requirement where validation may actually be desired.

[ScopeyNZ]

Thanks for reporting this issue.

To be clear the blocks are not saved (which is the correct behaviour)?

[thats4shaw]

The new block is never actually able to be added or appear due to graphql error mentioned above being triggered by the validation.

[ScopeyNZ]

I see... Okay - interesting conundrum. Elemental 4 has an approach a bit like SiteTree where the block is saved as soon as you press the add button. I suppose that means we should skip the validation only once for creating blocks?

[thats4shaw]

I think that would make sense and would be inline with the behaviour of SiteTree.

[robbieaverill]

Just a thought, you could wrap your validate() logic in an $this->isInDB() check

[thats4shaw]

Ah, yes! That'll do nicely.

[robbieaverill]

Ok I don't think there's anything we can fix in this module. User code needs to ensure that validation is conditional on the element existing first. Feel free to reopen if you think we can fix something here though!

[thats4shaw]

Finally revisited this and turns out the $this->isInDB() workaround doesn't work - validation must triggering after the fact. I'm just going to turn off inline editing in this case as not a major.

[ScopeyNZ]

Are you able to provide any more detail on $this->isInDB() not working? I'm pretty interested in hearing/resolving blockers that people are coming across for using inline editing.

[thats4shaw]

Absolutely. Here's a basic example of what I was doing:

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

        if ($this-isInDB()) {
            if (!$this->CardImageID) {
                $validation->addError('Image is required');
            }
        }

        return $validation;
    }

After some further testing I don't think this is a problem directly related to inline editing as even disabling it causes the same symptoms when adding a new block.

Basically it seems to me you can't add validation on elements in 4.x.

[ScopeyNZ]

Would you recommend we re-open this issue? Sounds like there's a slightly undefined bug here - but it's definitely buggy.

[thats4shaw]

I think so as sometimes I don't think adding stricter checks in the templates for particular fields would be desirable. Hopefully i've explained this ok, let me know if you need me to expand on anything in particular.

[robbieaverill]

I've had a quick look at this, the problem is that the element's write() method is called twice, which causes it to be validated twice. The first time it's called by AddElementToAreaMutation which is doing $area->Elements()->add($newElement), which internally calls $newElement->write(), and then the ReorderElements service is called which also writes the element again. It's the second operation that triggers the validation error to occur since it is in the DB at that point.

I'm going to see if we can adjust the logic slightly so it only writes once regardless of whether the element is being reordered or not.

[robbieaverill]

PR at https://github.com/dnadesign/silverstripe-elemental/pull/691

The error shows at the top of the page, where it should show on the element itself. I think we can treat that as a separate issue, so the PR above fixes this bug.

[ScopeyNZ]

691 is merged. Please let us know if this problem is still occuring.