Clean up todo in InlineBlockEntityOperations::handleEntityDelete() and use isLayoutCompatibleEntity()

I wonder what's the use case for validating an entity after it has been deleted from the storage. The issue title says that the entity is being deleted, but that's not really accurate, the entity is already gone.

I think the test could benefit from a comment explaining why we're calling validate(), but otherwise this looks good and is ready to go.

I like it. RTBC on the assumption that it will look like Christmas in an hour.

I'd like to take another look at the patch as well, hopefully there's no rush to commit it..

I've been staring for a while at \Drupal\layout_builder\InlineBlockEntityOperations::handleEntityDelete() and all its possible code paths, and I can't find what would trigger a call to EntityContext::fromEntity($entity) in there..

Is there some other part of the code that I should also be looking at? :)

Oops, didn't want to un-assign, I'm still looking into it.

+++ b/core/lib/Drupal/Core/Entity/Plugin/Validation/Constraint/ValidReferenceConstraintValidator.php
@@ -124,7 +124,7 @@ public function validate($value, Constraint $constraint) {
-        $existing_entity = $this->entityTypeManager->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id());
+        $existing_entity = $this->entityTypeManager->getStorage($entity->getEntityTypeId())->loadUnchanged($entity->id()) ?: $entity;

This looks very reasonable - but perhaps a code comment might be a good idea - so people know when we might use the passed in entity instead of the unchanged one.

Setting to NW for the code comment mentioned in #16.

Tagging to get some eyes to add the comment

Updated patch with reroll diff & addressed #16

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!

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.