Applying a recipe that provides content that has been trashed crashes

When applying recipes and/or when importing default_content we should trigger an event.
Right now that only happens when a recipe has been already installed (we need an issue in core).

We need to subscribe to that event, and apply the ignore trash context as we already do for workspaces in TrashIgnoreSubscriber::onWorkspacePrePublish.

The problem I'm facing is that AFAIK, this context apply per request, and this content import could happen in batches. So that applied context is lost.

I think the problem here is only content import, not applying a recipe. So the content import process needs to dispatch a "default_content pre-import" event when starting a batch, and that's what Trash should subscribe to.

@amateescu You are right. To be honest I think both should trigger events.

I have a working solution locally :-)

Just created ✨ When applying a recipe, we need to trigger an event pre importing content Active

Might need tests?

Postponed on ✨ When applying a recipe, we need to trigger an event pre importing content Active . No sense to work on this anymore if we might need to rename the class or whatever. Will serve as a POC it works.

I discussed this with @tim.plunkett and @pameeela and we decided it is not a Drupal CMS stable blocker. It's a nasty bug, but likely an edge case. And besides, the fix depends on an upstream change in core that is not itself a Drupal CMS stable blocker. So by extension, this isn't either. I'm downgrading it to a release target, as it would be nice to get it fixed if core can get the requisite change into 11.1.2 (and hopefully that gets tagged before we launch 1.0.0).

✨ When applying a recipe, we need to trigger an event pre importing content Active landed.

The final solution allows to skip uuids giving a reason, and we have information for knowing if skipping or erroring). But to be honest, I think we just should set the trash "ignore" context as that's something the importer should be taking care anyway.

What's the behavior of setting the inactive trash context when importing default content? Does the import process provide an error or something that it couldn't import a node because it already exists?

I defer the decision on how to handle this issue to the Drupal CMS / Recipes initiative folks, but my impression is that we should use the new API as it was designed: skip the content that can't be imported, and provide the reason for it.

The current behavior is a cryptic mysql error because of duplicated uuids.

The behavior with this patch and setting inactive context is that nothing happens with this content, because recipe runner uses skip and is detected properly: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

If recipe runner (or any other service calling default_content import APIs) wanted to use the error behavior instead, it would have to handle the error, because with this patch the content will be detected as existing, something that doesn't happen now.

Merged into 3.x, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.