Creating an context object for an entity that is being deleted causes a fatal error

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Adding patch against 10.0.1

Did a brief code review and the following comment doesn't make sense to me, I think we should reword it. Otherwise, the code looks good
// We might use the passed entity instead of the unchanged one.

Made change to the comment as per #30 so the comment is now quite relevant what it supposed to do.

Attempted to replicate this by running the code in the issue summary and I don't get any error in 10.1

Can someone please confirm if it's still an issue and update the issue summary with additional steps

Thanks!

I think the codepath described by the test case (introduced in #7) was at one time relevant to how Layout Builder was working, but is not reproducible anymore in core. And I can't think of a reason why you'd need to do it in custom code either.

There's still a reference in code to this issue in \Drupal\layout_builder\InlineBlockEntityOperations::handleEntityDelete(), let's see if calling isLayoutCompatibleEntity() is possible now.

Opened a MR, let's see how it goes.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Hiding the patches because the MR seems to work well :)

Updated title and solution to mention this seems to be cleaning up a todo and use isLayoutCompatibleEntity().

Added a question the MR.

Moving to NW for someone to test that proposal.

Hi all,
I removed the isLayoutCompatibleEntity($entity) check before calling removeByLayoutEntity($entity), as suggested.

Tests performed:

PHPUnit: All tests passed successfully, with no errors found. Only pre-existing skipped tests remained unchanged.

The change has been committed and submitted in the Merge Request. Awaiting feedback for any necessary adjustments!

Thank you!

MR is 1800+ commits back.

Hi @smustgrave

I have successfully rebased the branch with the latest changes from 11.x. The MR !7106 is now up to date, and the previous changes remain intact.

Additionally, I noticed that one test failed, but it does not appear to be related to the changes made in this MR.

Please let me know if you need any additional adjustments.

Thanks!

So reading the issue summary and title

update InlineBlockEntityOperations::handleEntityDelete() to use isLayoutCompatibleEntity()

Still seems to be needed.

Hi @smustgrave,

Apologies if I might be misunderstanding something here, but I just want to clarify the intent behind this change to ensure I'm following the best approach. I'm still getting started with contributing to Drupal, and I really appreciate your guidance.

The issue summary suggests that handleEntityDelete() should be updated to use isLayoutCompatibleEntity(), but after reviewing the implementation, I noticed that removeByLayoutEntity($entity) does not fail if the entity is not layout-compatible. Instead, it simply attempts to clean up inline_block_usage, setting layout_entity_id and layout_entity_type to NULL if applicable. If no matching records exist, nothing happens, which makes me wonder if the extra check is truly necessary.

I just want to make sure I'm not missing something here:

• Was there a specific issue that required adding isLayoutCompatibleEntity()?
• Is there any scenario where calling removeByLayoutEntity() directly could cause unintended side effects?

If the check is needed for a reason I’m not seeing, I’ll be happy to update it accordingly. Otherwise, removing it might help simplify the logic while still ensuring the cleanup happens.

Again, thanks for your patience and feedback—I really appreciate learning from this process!

No worries. So might help to look at the reference issue this was created from.

Not a layout builder expert to say what’s needed or not but the summary needs to match the MR. Helps committers quickly understand a ticket

If it’s determined nothing is needed then the summary could include your findings and just a simple solution of remove the todo

Can put back into review for more eyes but before RTBC summary will have to match