Wont delete reference if referenced entity is the entity being deleted

hi this looks to be known behavior when referencing if i am right ..
so if i am not saying non-sense, module's links below may help you if you could wait for the port to drupal 8..

For now a workaround (i am not saying non-sense)could be to have(create) a view display that have a(or additional if it related to an already exiting criteria with another operator) "FILTER CRITERIA" with operator setting's set to Is not empty (NOT NULL)

Module's links that may help if could wait for them : (PLEASE NOTICE THE DIFFERENCE IN NAME CORRECTLY)

• Field reference delete
• Field referenced delete

ISSUES ON , FOR PORTING TO 8

• first module port to 8 issue
• Port to 8 issue for second module

Thanks for the detailed report and additional links! I'll get back to this ASAP and see what can be done to resolve, or at least work around, the issue.

I simply added a hook_entity_delete() to this module and now references are syncing correctly when an entity is deleted.

Perhaps this was an oversight in the D8 port?

Ignore my previous comment. Here's a better patch.

When deleting an entity, there's no "original" property, which is only there temporarily during saving an entity. Since CER consults the original version of an entity to determine if any changes need to be sync'd, we need another way of informing CER that an entity was just deleted and references need to also be removed.

Unfortunately, I couldn't find a reliable way to inspect an entity and determine that it's deleted. The ->isNew() method is unreliable since the API allows this to be set even after an entity is already saved.

Instead I added a $deleted parameter which informs CER that the entity is deleted and that references to it should be removed. This patch actually works in my testing.

Patch looks Ok.
Tested and works as advertised.

Alternatively a flag $entity->isBeginDeleted = TRUE can be set in hook_entity_delete and used in CorrespondingReference::calculateDifferences. But not sure if this is a cleaner solution. The solution in this patch is more self-documenting.

I thought about this a little more, and I believe that this issue is best handled by using the Decorator pattern.

Setting the flag in cer_entity_delete():

  $decoratedEntity = new cerDecoratedEntity($entity)
	decoratedEntity->setIsBeingDeleted();
  cer_sync_corresponding_references($decoratedEntity);

Using the flag in CorrespondingReference::calculateDifferences:

// If entity is deleted, remove references to it.
if ($entity instanceOf cerDecoratedEntityInterface && $entity->isBeingDeleted()) {
  ...
}

I leave the issue at rtbc, as the current solution is working. I am prepared to make a new patch if the maintainer (@bmcclure) accepts this solution.

@Sutharsan I'm definitely open to that approach! While the last patch does seem adequate to me, the decorator approach seems a little cleaner and more decoupled.

I think I'll leave this as RTBC for the moment as I would be fine merging it in as-is, but if you get around to submitting a patch with the mentioned changes, that would be my preference.

Thanks, everyone!

Since a year has passed it seems the provided patch isn't the final work getting submitted. So, setting to Needs work seems the proper status.

Additionally: we should consider to make the sync delete optional since this isn't maybe always wanted and extends the possible use cases for the project.

Facing the same problem and the patch worked for us thx!
I would be in favor to submit this patch in a new release as it fixes the issue and create a separate (feature request / task) issue to eventually improve this with Decorator pattern...
Marking this RTBC

Regarding #8 and the time passed I strongly recommend to be careful setting RTBC only on the basis that it "works" in one "newly" installed project. The issue has been moved to latest dev AFTER latest patch has been provided and needs more testing since references are sensitive data which shouldn't changed by any surprise. But I leave it up to others to decide. Just a careful warning.

That was my bad about how much time has passed without this being merged. You make a good point that we should verify it works in more than just one circumstance on the dev version of the module.

If this patch is working for other people on the dev branch, I would be happy to get it merged in as-is.

I do agree that making the deletion optional would be great! And using the Decorator pattern would be nice, but I am OK with making that a separate issue if the existing patch properly fixes the original issue here.

Oh dear, I know how that feels :-) time runs like the white rabbit... I never get sync'd to this :)

The additional issue for the Decorator method is a good point and makes things easier and better trackable.

To the other part let me clarify where my thoughts came from to make it optional with form element for this behaviour:

A) Since this new behaviour is an API break against how the module has worked before, I thought, we should strongly consider the optionset form for both issues to not surprise users. Otherwise you maybe go thru the maintainer mess of breaking API, new release branches and issue queue floods. *hides* ... That's why simulatously "Just let's add a simple checkbox with 'Deleting XXX will delete the field reference' description to the module settings form" came in mind. - But ...

B) But after rereading this: "Now look at entity A.1. You see that it has an empty reference inside it, because the referenced entity was deleted but the field instance was not." from the OT again twice, I am not sure if this option is any useful in any way, since we are not on a feature request level here but on bug. My bad.

So, to speed things up let me encourage you to merge this in as-is. Since you are on alpha I do not see any dirtyness to do so for now when you consider to use this fix to jump to Beta. You are right, this would be an important fix to make.

Please take care and stay safe!

You make some good points. I do think this solution is adequate for resolving the mentioned bug and that perhaps we should make a separate issue for improving this behavior later, or just cleaning it up with the decorator pattern.

Since nobody has brought up issues with this patch and it seems that some have been using it, I am good with merging it. The patch didn't apply cleanly but it was due to a very minor difference in a function docblock. It seems to be working well for me, so I'm going to get it merged into the dev branch, and will plan to make a release that includes the change soon.

Thanks for the patch, thanks to all who helped test, and thanks for the detailed original report! Finally got it merged.

woop. thanks! 🚀