Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update/delete taxanomy fix upstream #2349

Merged

Conversation

pschoffer
Copy link
Contributor

@pschoffer pschoffer commented Sep 16, 2021

Description of the Change

Deleting term needs to do 2 actions:

  • Delete the term
  • Reindex all the child terms

However, if the hook is invoked on delete_term the term is already deleted in WP by then. The operations needed to get the list of child terms then fails.

This change switches the two actions into 2 hooks. On pre_delete_term which happens both before the term is deleted but also before the child terms were updated in WP already, we will enqueue child terms to sync queue. Then on delete_term (same as before) we will remove the term from ES.

The sync queue is then processed on shutdown, that is after the children terms were already updated with the correct parent.

Another change in this PR is adding the permission check bypass for CLI and cron on term resyncing.

Alternate Designs

Benefits

Possible Drawbacks

Verification Process

  1. wp elasticpress activate-feature terms

  2. wp elasticpress index --setup

  3. Create some hierarchy of categories

  4. Delete middle one `wp term delete category

  5. No errors

  6. Term is deleted

  7. Check that index is correctly showing the children of deleted term pointing to the parent of deleted term as their parent now

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

Fixes: Term deletion propagation to ElasticSearch

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

Successfully merging this pull request may close these issues.

3 participants