Copied from #943772-190: field_delete_field() and others fail for inactive fields :
- I don't like the explicit "field_purge_batch()" call added after the "field_delete_field()" in forum_uninstall().
That's a convoluted DX pattern for module developers - "on uninstall, after deleting the fields you created on install, don't forget to run an explicit purge pass, so that users with few field data (e.g people just evaluating a module) can disable the field type modules right after that"
field_purge_batch(reasonable N) should be taken care of within field_delete_field(). Problem is, node type deletion (or, worse, uninstall of an entity type, = deletion of all bundles) can cascade to 10(0)s of field deletions, so we probably want to control that automatic purge by an optional param for field_delete_field().
Besides, the current field_purge_batch() call won't work as intended right now :
- If there are already fields/data pending deletion, there's no guarantee that this purge pass will clear the very data you want to clear. This would require an additional $field_name param for field_purge_batch() to clear the data from a specific field.
- If the field had data, completely clearing the data+instance+field requires at least *two* field_purge_batch() calls.
See the code logic over there : if the EFQ returns no result (all data has been purged), then wipe the instance and field.
Should be switched to : if the EFQ returns less results than the number we asked for, then wipe the instance and field.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | field_purge-1264756-13.patch | 11.8 KB | yched |
| #11 | field_purge-1264756-11.patch | 11.67 KB | yched |
| #10 | field_modules_enabled-1264728-9.patch | 4.35 KB | yched |
| #9 | field_modules_enabled-1264728-9.patch | 4.35 KB | yched |
| #6 | field_purge-1264756-6.patch | 10.7 KB | yched |
Comments
Comment #1
yched commentedTentative patch.
Comment #3
yched commentedShould fix tests accordingly.
+ more explicit title.
Comment #4
catchSubscribing.
Comment #5
yched commented#3 had a leftover phpdoc @todo.
Comment #6
yched commentedRerolled after the move to /core.
Comment #7
sun#6: field_purge-1264756-6.patch queued for re-testing.
Comment #9
yched commentedHuh ? AFAICT #6 still applied with offset. Anyway, reroll.
Comment #10
yched commentedTestbot stalled ? Reuploading.
Comment #11
yched commentedOops, I rerolled the wrong patch, forget #9 and #10.
#6, rerolled.
Comment #13
yched commentedShould be better ?
Comment #15
geerlingguy commentedI just spent a bit of time trying to figure out why fields weren't getting deleted by field_delete_field(), which led me down a rabbit hole to this issue. Would really love to see this implemented for better DX. (So, mostly just a +1 from me... will test if I can find some time).
Comment #16
andypostdoes the issue still valid?
Comment #17
yched commentedWell, the APIs have changed quite a bit, but striclty speaking the interest is still there.
Although, on top of the cases mentioned in the OP where we don't want the mini-auto-purge to run (e.g deletion of whole bundles), we also don't want it to run during field deletion in config sync - since field purge is taken care of in a separate sync step.
So, maybe that's a lot of cases and the complexity might not be worth the DX gains.
Comment #31
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #32
smustgrave commentedSince it's been 3+ months without a follow up going to close out. If still valid in D11 please re-open
Thanks all