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.

CommentFileSizeAuthor
#13 field_purge-1264756-13.patch11.8 KByched
FAILED: [[SimpleTest]]: [MySQL] 33,339 pass(es), 14 fail(s), and 13 exception(es). View
#11 field_purge-1264756-11.patch11.67 KByched
FAILED: [[SimpleTest]]: [MySQL] 33,319 pass(es), 27 fail(s), and 26 exception(es). View
#10 field_modules_enabled-1264728-9.patch4.35 KByched
PASSED: [[SimpleTest]]: [MySQL] 34,167 pass(es). View
#9 field_modules_enabled-1264728-9.patch4.35 KByched
PASSED: [[SimpleTest]]: [MySQL] 34,173 pass(es). View
#6 field_purge-1264756-6.patch10.7 KByched
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_purge-1264756-6.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View
#5 field_purge-1264756-5.patch10.61 KByched
PASSED: [[SimpleTest]]: [MySQL] 32,937 pass(es). View
#3 field_purge-1264756-3.patch10.47 KByched
PASSED: [[SimpleTest]]: [MySQL] 32,957 pass(es). View
#1 field_purge-1264756-1.patch9.18 KByched
FAILED: [[SimpleTest]]: [MySQL] 32,924 pass(es), 10 fail(s), and 0 exception(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

yched’s picture

Status: Active » Needs review
FileSize
9.18 KB
FAILED: [[SimpleTest]]: [MySQL] 32,924 pass(es), 10 fail(s), and 0 exception(es). View

Tentative patch.

Status: Needs review » Needs work

The last submitted patch, field_purge-1264756-1.patch, failed testing.

yched’s picture

Title: Make field_purge_batch() work in one pass » Inline field purge within field_delete_field()
Status: Needs work » Needs review
FileSize
10.47 KB
PASSED: [[SimpleTest]]: [MySQL] 32,957 pass(es). View

Should fix tests accordingly.

+ more explicit title.

catch’s picture

Subscribing.

yched’s picture

FileSize
10.61 KB
PASSED: [[SimpleTest]]: [MySQL] 32,937 pass(es). View

#3 had a leftover phpdoc @todo.

yched’s picture

FileSize
10.7 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch field_purge-1264756-6.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View

Rerolled after the move to /core.

sun’s picture

Issue tags: -needs backport to D7

#6: field_purge-1264756-6.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +needs backport to D7

The last submitted patch, field_purge-1264756-6.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
FileSize
4.35 KB
PASSED: [[SimpleTest]]: [MySQL] 34,173 pass(es). View

Huh ? AFAICT #6 still applied with offset. Anyway, reroll.

yched’s picture

FileSize
4.35 KB
PASSED: [[SimpleTest]]: [MySQL] 34,167 pass(es). View

Testbot stalled ? Reuploading.

yched’s picture

FileSize
11.67 KB
FAILED: [[SimpleTest]]: [MySQL] 33,319 pass(es), 27 fail(s), and 26 exception(es). View

Oops, I rerolled the wrong patch, forget #9 and #10.
#6, rerolled.

Status: Needs review » Needs work

The last submitted patch, field_purge-1264756-11.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
FileSize
11.8 KB
FAILED: [[SimpleTest]]: [MySQL] 33,339 pass(es), 14 fail(s), and 13 exception(es). View

Should be better ?

Status: Needs review » Needs work

The last submitted patch, field_purge-1264756-13.patch, failed testing.

geerlingguy’s picture

I 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).

andypost’s picture

does the issue still valid?

yched’s picture

Well, 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.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.