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

Auto sync parent terms #3145

Merged
merged 6 commits into from
Jan 20, 2023
Merged

Auto sync parent terms #3145

merged 6 commits into from
Jan 20, 2023

Conversation

MARQAS
Copy link
Contributor

@MARQAS MARQAS commented Nov 22, 2022

Description of the Change

Auto-sync the parent terms when child terms are selected in the posts

Closes #3105

How to test the Change

  • Go to WordPress default categories
  • Add 2 terms, make one of them child to the other
  • Go to WordPress default posts
  • Add a new post and link the child term with it
  • Edit the parent term slug
  • Go to Elasticsearch and check the post document and note that parent slug is also updated

Changelog Entry

Fixed - Auto sync issue with parent term

Credits

Props @MARQAS

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@felipeelia felipeelia added this to the 4.5.0 milestone Nov 23, 2022
@felipeelia
Copy link
Member

@MARQAS a couple of questions:

  1. Can we add some unit tests for this?
  2. Do we also have the bug when a parent term is deleted? If so, this covers that scenario as well?
  3. To increase readability, do you mind wrapping variables in double quote strings with {}, like in "SELECT object_id FROM {$wpdb->term_relationships} WHERE term_taxonomy_id...
  4. Will we have a performance problem if too many posts are associated with child terms?

Copy link
Member

@felipeelia felipeelia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MARQAS do you mind making the suggestions I've added? Just so we keep var names consistent. Thanks!

tests/php/indexables/TestPost.php Outdated Show resolved Hide resolved
tests/php/indexables/TestPost.php Outdated Show resolved Hide resolved
@MARQAS
Copy link
Contributor Author

MARQAS commented Dec 13, 2022

@felipeelia I have added the changes you asked for along with a notice. I observed that it wasn't showing notice when the parent term was not attached to the posts.
Steps to reproduce:

  • Add parent and child terms
  • Attach child term to the posts
  • Make Content Items per Index Cycle less than the posts attached to the child term.
  • While editing a child term, you can see the notice.
  • But it will not show the notice when editing a parent term.
  • It will only show the notice on the parent term if the posts attached to the parent term are greater than Content Items per Index Cycle

With this PR, it will show the notice on the parent term as well if posts associated with one of its child terms are greater than the Content Items per Index Cycle

@felipeelia felipeelia merged commit 1fe570b into develop Jan 20, 2023
@felipeelia felipeelia deleted the fix/parent-term-sync-issue-3105 branch January 20, 2023 14:43
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.

BUG: Parent taxonomy term auto sync bug
2 participants