Problem/Motivation
Currently, when e.g. a media entity is being removed, if this entity was being referenced in a required field, upon editing the node referencing the entity, the required field will show the entity is missing...though the field will still be required and will fail validation when saving the node.
This is a usability issue because the content author might wish to edit something totally unrelated and will fail to save content due to this entity missing. Whether or not the editor will be able to sort out the validation issue is up in the air and something that would need to be addressed.
Proposed resolution
TBD
Remaining tasks
TBD
User interface changes
TBD
API changes
TBD
Data model changes
TBD
Comment | File | Size | Author |
---|---|---|---|
#2 | 2940394-2.patch | 8.19 KB | marcoscano |
Comments
Comment #2
marcoscanoVery rough POC, but serves for us to validate the approach.
Comment #3
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedThe approach in #2 is very close to what we do in entity_reference_integrity. If we do indeed want to protect entities from being deleted if they have associated references, perhaps it makes sense to bring across the associated services/tests etc too?
Comment #4
marcoscanoThanks @Sam152 for pointing it out! In fact if this ends up moving forward, @Sam152 should also be credited because the patch in #2 is partly inspired in entity_reference_integrity's code.
However, I'm not sure if we should make this logic a broader API/Service, or just some internal logic constrained to
ContentEntityDeleteForm
. No strong opinions about this, but I'd lean towards keeping it internal for now. Tests should definitely be added, I can do that once there seems to be consensus on the approach.Comment #5
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedAre we confident that form is the only place we'll give users the option to delete something?
Comment #14
LendudeFeels like a feature to me.
Also the the title/IS are talking about different things then the current patch. Would the idea be to 'allow saving incomplete entities' or 'not allowing deleting entities that would leave required fields empty'.
The IS talks about the first and the patch does the second right?
Either way, this feels like it should be optional.
Comment #16
joachim CreditAttribution: joachim as a volunteer commented> Either way, this feels like it should be optional.
Agreed, which means that the problem stated in the IS still needs fixing as an entity validation issue, because the 'referenced entity got deleted' problem could still happen on a field that doesn't have this enabled.
It needs to happen at the validation constraint level, rather than the form level, because of things like REST and JSONAPI.
Referential integrity is a really complicated area, too!
For example, with the current patch, what happens if there is are references that point both ways, A <-> B? You'd never be able to delete either entity!
Thinking more widely, the behaviour in the patch is only one of a variety of behaviours that a site might want:
- prevent deletion of a referenced entity
- allow deletion, but clean up the value on the referencing entity (there's a module for that)
- allow deletion, and cascade the deletion to the referencing entity (there's a module for that too)
Comment #17
smustgrave CreditAttribution: smustgrave at Mobomo commentedMoving back to NW for an issue summary update.
Ready the current one (which was also mentioned before) it seems like a valid issue. If the entity is gone and the field is required it should still throw validation no?
Think one of the modules mentioned in #16 is https://www.drupal.org/project/entity_reference_purger which we use on a number of projects.