Skip to content

fix(local-fields): cascade remove on group delete - #524

Open
faisalahammad wants to merge 1 commit into
WordPress:trunkfrom
faisalahammad:fix/458-cascade-remove-local-fields
Open

fix(local-fields): cascade remove on group delete#524
faisalahammad wants to merge 1 commit into
WordPress:trunkfrom
faisalahammad:fix/458-cascade-remove-local-fields

Conversation

@faisalahammad

Copy link
Copy Markdown

acf_remove_local_field_group() removed the group from local-groups but left its fields orphaned in the local-fields store. Orphaned fields remained discoverable by key/name lookups after their parent group was gone.

New _acf_cascade_remove_local_fields() helper recursively queries by parent key and removes each child field, including nested sub-fields under repeaters. Both the data entry and name alias are cleaned up per field.

Closes #458

Use of AI Tools

This PR was developed with AI assistance. All generated code has been reviewed, tested, and verified by the author.

When acf_remove_local_field_group() ran, it removed the group from
the local-groups store but left its fields orphaned in local-fields.
Orphaned fields remained discoverable by key/name lookups.

New _acf_cascade_remove_local_fields() helper recursively queries
by parent key and removes each child field, including nested
sub-fields under repeaters. Both data entries and name aliases
are cleared.

Fixes WordPress#458
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props faisalahammad, cbravobernal.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@faisalahammad

Copy link
Copy Markdown
Author

CI check note — pre-existing WP trunk flake

The only failing CI check on this PR is E2E - WP trunk / Shard 4/4 (run 30203543529, job 89797567940). It does NOT appear linked to this PR:

  • PR branches only touch includes/local-fields.php (+34/-1) and tests/php/includes/test-local-fields.php (+71/-11). No .spec.ts, JS, or admin-tools code was changed.
  • All other CI checks pass: phpcs, PHPStan, PHP 7.4, PHP 8.4, Node 20, E2E WP 6.9 (all 4 shards), E2E WP latest (all 4 shards), E2E WP trunk (shards 1-3/4).
  • The failing tests (should update existing post type / taxonomy / options page on import with same key in tests/e2e/import-export.spec.ts at lines 1398/1534/1666, plus bulk-action checkbox timeouts in post-type-admin.spec.ts around line 664) live in WP's import and bulk-action admin UI — code paths this PR does not touch.
  • Local reproduction on this branch against WP 7.0.2 (current latest, the version wp-env serves by default) shows all 3 Import Overwrite tests pass.

The cascade helper _acf_cascade_remove_local_fields() runs only inside acf_remove_local_field_group(), which is called from teardown in PHP tests only. The import flow (admin-tools.phpacf_import_internal_post_typeupdate_post) never invokes it.

Recommendation: treat E2E - WP trunk / Shard 4/4 as pre-existing WP-trunk-specific flake, unrelated to this PR. Worth tracking separately if it recurs on other PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

acf_remove_local_field_group() leaves the group's fields registered (orphans in local-fields store)

1 participant