Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
This issue is follow up of a suggestion given in Delete dependent config entities that don't implement onDependencyRemoval() when a config entity is deleted.
The main motivation is that API for deletion of field should not be responsible for deletion of the field storage. It should do only what the API method says that it will do and nothing more.
Proposed resolution
The code responsible to delete the field storage when the last field is deleted will be moved in the UI.
Comment | File | Size | Author |
---|---|---|---|
#15 | 2442757-field-delete-dependency-13-interdiff.txt | 4.54 KB | Berdir |
#15 | 2442757-field-delete-dependency-13.patch | 9.73 KB | Berdir |
#10 | interdiff-2442757-6-10.txt | 1.02 KB | lanchez |
#10 | field_storage_deletion-2442757-10.patch | 6.25 KB | lanchez |
#8 | field_storage_deletion-2442757-6.patch | 6.29 KB | boyan.borisov |
Comments
Comment #1
boyan.borisov CreditAttribution: boyan.borisov commentedComment #2
boyan.borisov CreditAttribution: boyan.borisov commentedComment #3
boyan.borisov CreditAttribution: boyan.borisov commentedComment #4
boyan.borisov CreditAttribution: boyan.borisov commentedI've created a patch.
Comment #6
boyan.borisov CreditAttribution: boyan.borisov commentedIt seems that I've missed to update a few tests which check exactly what we are trying to change.
Comment #8
boyan.borisov CreditAttribution: boyan.borisov commentedand one more try
Comment #9
swentel CreditAttribution: swentel commentedexceeds 80 chars
I think we can remove the '!$this->entity->isUninstalling()' part
Comment #10
lanchez CreditAttribution: lanchez commentedMade those alterations.
Comment #11
BerdirAs discussed at the sprint at Barcelona, this is the critical part of #2468045: When deleting a content type field, users do not realize the related View also is deleted, we need this to be able to show if a field storage will be deleted, so that we can show that the view will be deleted.
Comment #12
BerdirThe patch is a good start, but to actually make this work, we need tests for that behavior and more importantly, we need to be able to show the dependencies that will be affected by deleting the storage together with the field dependencies.
Comment #13
BerdirGoing to work on this.
Comment #14
BerdirSorry @Bojhan :)
Comment #15
BerdirFirst part, this provides the UI dependency information in the UI.
Remaining steps:
1. Write tests for this, going to be a bit complicated I fear, I need a view or something else that depends on the storage, a number of combinations
2. It now shows the field itself as a dependency that will be deleted, which is correct for the storage, but we obviously know that. Going to try and hide that.
I think I also incorrectly merged one part but I think we are removing too many tests anyway.
Comment #16
Jaesin CreditAttribution: Jaesin at Chapter Three commented@Berdir++
Comment #17
yched CreditAttribution: yched commentedSo you said we could also leave the deletion at the API level rather than at the UI level ? If so, I'd rather be +1 on that ?
Minor : should we leave that as a parent:: call ?
Comment #19
BerdirYes. Which is why I created a new issue: #2575605: Field delete form does not display configuration dependencies that will be updated/deleted
Comment #21
Bojhan CreditAttribution: Bojhan as a volunteer commented@Berdid Haha :) I was already wondering what the UI aspect here was.