Skip to content

Bugfix: Create indexable hierarchy the first time an indexable is created#19332

Merged
thijsoo merged 2 commits intotrunkfrom
build-hierarchy-on-post-save
Dec 7, 2022
Merged

Bugfix: Create indexable hierarchy the first time an indexable is created#19332
thijsoo merged 2 commits intotrunkfrom
build-hierarchy-on-post-save

Conversation

@diedexx
Copy link
Copy Markdown
Member

@diedexx diedexx commented Dec 2, 2022

Context

When performing a full reindex, I noticed that indexable hierarchy is not created. This is due to a problem where we rely on the indexable being saved to the database (and thus having an indexable id) when we generate hierarchy, but when we index a post we:

  1. Create the indexable
  2. Generate hierarchy (for indexableId = null)
  3. Save the indexable to the database, giving it an indexable id.

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where indexable hierarchy wasn't built when the Indexable is built for the first time.

Relevant technical choices:

  • I've added some (very basic) integration tests to prove that my solution works and to pinpoint the origin of the problem. There's still a lot of tests that need to be added, which I've described in a comment.
  • I've added an early return in the indexable builder if the indexable has already been saved. This is the case for posts and authors. This should have a small performance increase. I verified that there is no need for saving the indexable again after creating hierarchy, author indexables and primary terms.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Indexing through post events

  1. With the test helper plugin, reset all indexables with the "Reset Indexables tables & migrations" button.
  2. Make sure the author archives are active. This is just to make sure author indexables still work.
  3. Create a new post with a parent page. You can use the page post type or any hierarchical post type for this.
  4. Create a new term with a parent term. You can use the category post taxonomy or any hierarchical taxonomy for this.
  5. Check that your wp_yoast_indexable table find the indexable ID for your parent and child posts and terms (usually the ones with the highest number)
  6. Check that your wp_yoast_indexable_hierarchy table now includes 2 records:
    • one with ancestor_id = 0 and indexable_id = [the indexable_id of the parent post]
    • one with ancestor_id = [the indexable_id of the other record and the parent post] and indexable_id = [the indexable id of your child post].
  7. Also make sure there is an indexable for the author of your post.

Indexing through frontend indexing

  1. Have at least one page with a parent page. You can use the page post type or any hierarchical post type for this.
  2. Have at least one term with a parent term. You can use the category post taxonomy or any hierarchical taxonomy for this.
  3. With the test helper plugin, reset all indexables with the "Reset Indexables tables & migrations" button.
  4. Check that your wp_yoast_indexable_hierarchy table is empty.
  5. Go the the Tools section of Yoast SEO and click Optimize now.
  6. Check that your wp_yoast_indexable table find the indexable ID for your parent and child posts and terms (usually the ones with the highest value for object_id)
  7. Check that your wp_yoast_indexable_hierarchy table now includes at least these 2 records:
    • one with ancestor_id = 0 and indexable_id = [the indexable_id of the parent post]
    • one with ancestor_id = [the indexable_id of the other record and the parent post] and indexable_id = [the indexable id of your child post].

Indexing through CLI

  1. Have at least one page with a parent page. You can use the page post type or any hierarchical post type for this.
  2. Have at least one term with a parent term. You can use the category post taxonomy or any hierarchical taxonomy for this.
  3. With the test helper plugin, reset all indexables with the "Reset Indexables tables & migrations" button.
  4. Check that your wp_yoast_indexable_hierarchy table is empty.
  5. Run the following WP_CLI command: wp yoast index
  6. Check that your wp_yoast_indexable table find the indexable ID for your parent and child posts and terms (usually the ones with the highest values for object_id)
  7. Check that your wp_yoast_indexable_hierarchy table now includes at least these 2 records:
    • one with ancestor_id = 0 and indexable_id = [the indexable_id of the parent post]
    • one with ancestor_id = [the indexable_id of the other record and the parent post] and indexable_id = [the indexable id of your child post].

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Block/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

The impact is limited to creating and updating indexables for post and terms.

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

  • I have written documentation for this change.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unit tests to verify the code works as intended
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label and noted the work hours.

DUPP-831

@diedexx diedexx added the changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog label Dec 5, 2022
@diedexx diedexx marked this pull request as ready for review December 5, 2022 08:27
@diedexx diedexx added this to the 19.13 milestone Dec 5, 2022
Copy link
Copy Markdown
Contributor

@thijsoo thijsoo left a comment

Choose a reason for hiding this comment

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

ACC + CR 👍 Looks good

@thijsoo thijsoo merged commit 6a59b7f into trunk Dec 7, 2022
@thijsoo thijsoo deleted the build-hierarchy-on-post-save branch December 7, 2022 10:45
@enricobattocchi enricobattocchi modified the milestones: 19.13, 19.14 Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants