[5.x]: Adding nested elements programmatically does not work when saving a provisional draft

[MoritzLost]

What happened?

Description

I need to create some nested elements programmatically – specifically, an Address elements in an Addresses fields and some Entry elements in some Matrix fields. Here's a minimal example, triggered on Entry::EVENT_BEFORE_SAVE:

public function handleBeforeEntrySave(ModelEvent $e)
{
    $entry = $e->sender;

    if (!$entry->section || $entry->section->handle !== 'offers') {
        return;
    }

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

    $entry->setFieldValue('contact', [
        'sortOrder' => ['new:1'],
        'entries' => [
            'new:1' => [
                'type' => 'contact_block',
                'fields' => [
                    'myCustomField' => 'foobar',
                ],
            ],
        ],
    ]);
}

Steps to reproduce

1. In the code above, replace offers with any section handle, contact with the name of a matrix field, contact_block with the handle of an entry type that's allowed in the Matrix fields, and the fields array with some field values for that entry type.
2. Trigger that function on Entry::EVENT_BEFORE_SAVE.
3. Modify some fields on an entry in that section and save it.

Expected behavior

I would expect my hook to always create a nested entry of type contact_block in the contact field.

Actual behavior

The hook only works if the entry form is submitted without any pending changes, i.e. if the user does not have a provisional draft for that entry.

If any field in the entry is modified (which means a provisional draft) is created, the nested field in the matrix entry is not created. I tried to find out what's going on:

• I checked with the debugger – the field value is definitely being set, even if the entry is saved from a provisional draft. It just has no visible effect.
• I tested this with an Addresses field as well. I can see the new row in the addresses and the elements table, but it gets a dateDeleted value set to the same value as dateCreated immediately.

I think this might be an issue with the lifecycle of nested entries.

I found one workaround. If I execute the hook for drafts as well as canonical entries, it works correctly:

if ($entry->getIsRevision()) {
    return;
}

But I'm not sure if that has any unintended side effects. And since the field value is still being set in the scenario above, I assume it's just a bug in Craft.

Craft CMS version

5.8.18

PHP version

8.3

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

[linear]

CMS-1669

[brandonkelly]

The issue is this condition:

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

Provisional drafts are drafts. So the event isn’t getting involved in provisional draft saves.

When you press “Save” for a provisional draft (or press “Apply draft” on a normal draft), the following happens:

1. The draft is saved one last time with any posted field values.
2. Any upstream changes on the canonical element are merged into the draft.
3. The draft is duplicated with the canonical ID & UUID values, overwriting the canonical element.
4. The draft element is discarded.

When step 3 occurs, it’s assumed that the draft is already in the exact state that the canonical element should be saved as. So when NestedElementManager (the class responsible for managing nested Matrix entries, addresses, Commerce product variants, etc.) handles the duplication, it just duplicates the nested elements over to the canonical element verbatim.

cms/src/elements/NestedElementManager.php

Line 681 in 2fdb203

$this->duplicateNestedElements($owner->duplicateOf, $owner, true, !$isNew);

That said, we are discussing adding a new property to elements that would probably help you here, which would identify when a draft is being saved in preparation for being applied to the canonical element.

[MoritzLost]

@brandonkelly Thanks for the explanation, Brandon!

Yes, I assumed it's because of the provisional draft. However, it seems that's not the full story. My hook is still getting executed when the canonical element is updated, because the element is still being saved. And with any regular text field, this works as intended:

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

$entry->setFieldValue('myPlaintextField', 'My custom text');

This works even when the entry is being saved via a provisional draft. That's why I assumed it should work with nested elements as well. And I could see my code being executed through the debugger. Feels like the NestedElementManager should behave differently in this scenario. This inconsistency between regular fields and relational / nested element fields when all I'm trying to do is update a field value any time an entry is saved feels like a leaky abstraction.

I also tried to create the nested element (Asset / Entry) directly, setting the fieldId and ownerId manually and saving that, but the element was immediately marked as deleted (via dateDeleted). Probably also by the NestedElementManager.

That said, we are discussing adding a new property to elements that would probably help you here, which would identify when a draft is being saved in preparation for being applied to the canonical element.

That would be helpful. However, taking a broader look: There are already a ton of states and lifecycle issues like this that we have to be aware of when doing anything on element saves and/or setting values programmatically. The most common advice on Discord, canonical Stack Exchange answers, etc, is to use ElementHelper::isDraftOrRevision() to ensure a hook only runs for canonical, published entries. Feels like this is something that should 'just work'. Put another way, setting fields programmatically is such a common task that it shouldn't require considerations of the various states of the entry lifecycle. So I would prefer if the hook above worked as (reasonably) expected, instead of introducing another property that I have to check manually.

That's my take from the outside developer perspective. I know that this all makes sense with your intricate knowledge of the architecture. But I feel I know that architecture and element lifecycle pretty well by know, and I still bang my head against the wall with stuff like this. My team even more so.

Maybe it's a documentation issue. But to summarize: In my opinion, the difference between the different ways to save an entry should be abstracted away when all I'm trying to do is update an entry before it's being saved.

[brandonkelly]

Yes, I assumed it's because of the provisional draft. However, it seems that's not the full story. My hook is still getting executed when the canonical element is updated, because the element is still being saved.

Right, but NestedElementManager is ignoring the posted value, per that line I posted in the last reply. It’s being told that the saved element is being duplicated from a source element (the draft), and so it isn’t bothering to check what the target element’s value is, and just blindly duplicating the source element’s nested elements over. Which makes sense in the context.

I generally agree with you – it would be nice if the code you posted just worked, but it’s a pretty complex situation with multiple elements getting saved/duplicated at once, and I’m not sure how it could be improved right now, beyond giving you a tool to make your edits a bit earlier in the process, before the draft gets duplicated as the new canonical.

[brandonkelly]

ElementTrait::$applyingDraft was just added for 5.9. So going forward, you can solve this by changing that condition to:

if (ElementHelper::isDraftOrRevision($entry) && !$entry->applyingDraft) {
    return;
}

[MoritzLost]

@brandonkelly Fair enough, that will be useful, thanks!