[4.0.3]: Custom validation causes an error saving a provisional draft

[MoritzLost]

What happened?

Description

I'm trying to add some custom validation rules to an entry, but I'm getting some errors when first saving an entry.

Here's a shortened version of my custom validation code that lives inside a module:

public function init()
    Event::on(
        Entry::class,
        Entry::EVENT_AFTER_VALIDATE,
        $this->validateScheduleEntry(...),
    );

    parent::init();
}

public function validateScheduleEntry(Event $e)
{
    /** @var craft\elements\Entry $entry */
    $entry = $e->sender;

    if ($entry->section->handle !== 'schedule') return;
    if (ElementHelper::isDraftOrRevision($entry)) return;

    $entry->addError('date_start', 'Some example error');
}

Now this works fine for entries that are already published. But for new entries, it causes errors:

1. I click on 'New entry' from the entry index page.
2. I fill some fields (in a way that triggers my custom validation rule) and click on 'Create entry'. My validation hook adds the error to the date_start field. As a sidenote, the error is displayed twice at this point, any ideas why?
3. I correct the error and click 'Create entry' again. At this point, I get a HTTP 400 error with the following message:

No element was identified by the request.

Here's the full stack trace:

2022-06-09 08:34:32 [web.ERROR] [yii\web\HttpException:400] yii\web\BadRequestHttpException: No element was identified by the request. in /path/to/project/vendor/craftcms/cms/src/controllers/ElementsController.php:848
Stack trace:
#0 [internal function]: craft\controllers\ElementsController->actionSave()
#1 /path/to/project/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#2 /path/to/project/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams(Array)
#3 /path/to/project/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction('save', Array)
#4 /path/to/project/vendor/craftcms/cms/src/web/Application.php(301): yii\base\Module->runAction('elements/save', Array)
#5 /path/to/project/vendor/craftcms/cms/src/web/Application.php(625): craft\web\Application->runAction('elements/save', Array)
#6 /path/to/project/vendor/craftcms/cms/src/web/Application.php(280): craft\web\Application->_processActionRequest(Object(craft\web\Request))
#7 /path/to/project/vendor/yiisoft/yii2/base/Application.php(384): craft\web\Application->handleRequest(Object(craft\web\Request))
#8 /path/to/project/web/index.php(11): yii\base\Application->run()

In the line that the error is coming from, the ElementsController checks if the post data specifies an element and that element is not a draft or revision. I tried to debug this to find out more – at the point that the error is thrown, $element->getIsDraft() returns true. So the controller is expecting a published entry at that point but the element is still a draft.

Not sure if this is a bug with the entry creation flow or if I'm just attaching my validation rules in a wrong way. I also looked at the defineRules event, but it's much more involved and verbose, so I prefer the validation and/or save hooks.

Steps to reproduce

1. Add the code above to a custom module that's bootstrapped automatically (remove the section handle check and adjust the field name to a field that exists in the testing environment).
2. Create a new entry from the entries index page.
3. Edit some fields and click 'Create entry'. This should cause a validation error to show up for the field specified in the code.
4. Optionally edit the field and/or uncomment the validation rule to make sure it's not being triggered for the next request.
5. Click 'Create entry' again. This will result in the error displayed above.

Craft CMS version

4.0.3

PHP version

8.1

Operating system and version

macOS 12.2

Database type and version

MySQL 8

Image driver and version

No response

Installed plugins and versions

No response

[brandonkelly]

Sorry for the delayed response. I think the issue here is the way you are calling your validateScheduleEntry() method. Your code example would have it called right at the same time you are calling Event::on(), rather than as a result of the event getting triggered. You would need to nest the call inside another function to have it called at the time the event is triggered:

public function init()
    Event::on(
        Entry::class,
        Entry::EVENT_AFTER_VALIDATE,
        function(Event $e) {
            $this->validateScheduleEntry($e);
        }
    );

    parent::init();
}

[MoritzLost]

@brandonkelly Thanks for the response! But this is not the issue. I'm not calling the function directly, the three dots are the first class callable syntax introduced in PHP 8.1. The notation $this->validateScheduleEntry(...) creates a closure for this function, identical to [$this, 'validateScheduleEntry']. It's functionally identical to the anonymous function in your code example. Just to be sure, I also tried this syntax, with the same result.

Any other ideas what might be causing this?

[brandonkelly]

Ah, TIL!

So the issue is most likely that you are skipping validation for drafts and revisions:

if (ElementHelper::isDraftOrRevision($entry)) return;

That means the validation won’t run when Craft is validating the provisional draft. Craft assumes that if the provisional draft validates with the live scenario, that the draft is ready to be applied to the canonical entry.

Maybe change that condition to:

if (
    $entry->scenario !== Entry::SCENARIO_LIVE &&
    ElementHelper::isDraftOrRevision($entry)
) {
    return;
}

[MoritzLost]

@brandonkelly Thank you, that solves the issue indeed! Adding the scenario condition also got rid of the duplicated errors, now the error messages show only once as expected.

I still don't fully understand this fix though. I thought a provisional draft was an unsaved draft for an existing entry. When I click on "New entry" in the entries list, does this already create a provisional draft? And just so I get it right: When I have finished my draft and click on "Create entry", Craft first validates the draft with SCENARIO_LIVE, then saves the draft as a published entry and validates it again? So the validation is essentially done twice and this is why I was seeing duplicate error messages? I'm a bit confused now, would you mind explaining this in a bit more detail? Thanks!

[brandonkelly]

When you first create an entry, you are working with an “unpublished draft” – which is an a la carte draft; it doesn’t reference a “real” entry like other drafts do.

When you press “Create entry” on the unpublished draft, the draft is validated with the live scenario (if it’s enabled), and if it validates, the entry will be re-validated and re-saved with its draft status removed.

When you apply a normal/provisional draft to an entry, and the draft is enabled, first the draft will be validated with the live scenario, and if it validates, all of the draft’s modified field/attribute values will be copied over to the real entry, and then the real entry will be validated and saved. If there are any additional validation errors at this point which weren’t caught when validating the draft, there’s no way to handle them gracefully, because they were reported on a different element from the one the user was actually editing (the entry instead of the draft). So an exception is thrown to abort the save process, rather than showing the validation errors like Craft would have if they had been caught when validating the draft.

[MoritzLost]

@brandonkelly Thanks for the explanation! But if unpublished drafts are validated as drafts with the live scenario and then immediately published without further validation, why was I getting the error in this case? The first version of my hook was skipping drafts using ElementHelper::isDraftOrRevision. So pressing the Create Entry button on the unpublished draft shouldn't have caused my custom validation to run at all, right? The HTTP 400 response I got looks more like it matches your second scenario, applying a draft to an existing entry, not publishing an unpublished draft. Or am I missing something here?

[brandonkelly]

Sorry, I got the details a little wrong in my last comment. Validation is in fact run twice when saving an unpublished draft – once when updating the draft with the post data, and again when removing its draft status. (Comment is updated to reflect that.)

I looked into the error you were getting because I realized it didn’t make any sense – it was coming from the elements/save action, which should only be called when saving a normal entry, not an unpublished draft. Turns out there was a bug where, if a custom validation error was added at that last stop (when removing the draft status), the draft status wasn’t getting re-added to the entry. So when the entry form reloaded with the custom validation error, the form thought it was being rendered for a normal entry rather than an unpublished draft, so its action was getting set to elements/save rather than elements/apply-draft. That is now fixed for the next release, as well as the double validation error bug you mentioned.

[MoritzLost]

@brandonkelly Awesome, thanks for the explanation, and for taking the time to explain this in detail! Got my hook working exactly right now, it's not throwing the exception anymore and only displaying my validation messages once. Good catch regarding the bug, looking forward to the next release! 💯

[brandonkelly]

Glad you’re sorted! Craft 4.2.2 is out now with those two fixes. Thanks for the persistence :)