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

21404 when indexable creation is disabled cleanups are being scheduled upon post creation #21438

Conversation

thijsoo
Copy link
Contributor

@thijsoo thijsoo commented Jun 12, 2024

Context

  • We want to make sure that our cleanup cron is not running and not scheduled when indexables are disabled.

Summary

This PR can be summarized in the following changelog entry:

  • Disabled an unneeded cleanup cronjob when indexables are disabled.

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Note: Wherever add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); is mentioned, it can also mean that you are on a non-production site instead (aka, have the WP_ENVIRONMENT_TYPE constant indicate a staging or local or development site).

  • Make sure you have the crontrol plugin enabled. And make sure there is currently no cron with the name wpseo_start_cleanup_indexables. If there is remove it by hovering over the cron and clicking delete. Refresh the page to make sure it has happend.
  • Also make sure you have the CPT ui plugin enabled with a taxonomy and post type created. Add one post and one taxonomy item, and run the SEOO

Activation of the plugin for the second time.

  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Disable Free and re-enable free. The wpseo_start_cleanup_indexables should not be present in the cron overview.
  • Remove the filter and repeat the steps. Now the wpseo_start_cleanup_indexables cron should be visible.

Disable attachment indexables

  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Make sure you remove the previously made wpseo_start_cleanup_indexables cron.
  • Go to /admin.php?page=wpseo_page_settings#/media-pages and enable media pages. Disable it again and amke sure the cron is not shown in the overview.
  • Disable the filter and repeat. The cron should now show up.

Disable author archives

  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Make sure you remove the previously made wpseo_start_cleanup_indexables cron.
  • Go to /admin.php?page=wpseo_page_settings#/author-archives and enable author archives. Disable it again and make sure the cron is not shown in the overview.
  • Disable the filter and repeat. The cron should now show up.

Remove a taxonomy

  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Make sure you remove the previously made wpseo_start_cleanup_indexables cron.
  • Go to custom post type ui en remove your taxonomy.
  • Make sure the cron is not there
  • Disable the filter and repeat. Create a new taxonomy and remove this after creating it and make sure the cron now does show up.

Remove a post type

  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Make sure you remove the previously made wpseo_start_cleanup_indexables cron.
  • Go to custom post type ui en remove your post type.
  • Make sure the cron is not there.
  • Disable the filter and repeat. Create a new post type and remove this after creating it and make sure the cron now does show up.

Remove post from author

  • Enable author archives
  • Make sure you have an author with exactly 1 post.
  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Make sure you remove the previously made wpseo_start_cleanup_indexables cron.
  • Remove the post you added to the author.
    Make sure the cron is not there.
  • Disable the filter and repeat. Create or add a new post to the author and remove that post. The cron should now show up.

Run the cleanup

To test if the cleanup will not run with the filter enabled we will try to remove author archive indexables that are no longer needed.

  • Without the filter enabled. Make sure author archives are enabled and visit the author archive page of and author with 1 or more posts.
  • Verify that you have an author indexable.
  • Now disable author archives. You should have the wpseo_start_cleanup_indexables cron.
  • Have the add_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' ); filter active somewhere on your site.
  • Click run now on the cron. And make sure the author indexable is still there.
  • Also make sure the cron has unscheduled itself.
  • Now repeat the steps without the filter and make sure the indexables are actually removed.

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 (Default Block/Gutenberg/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.

QA can test this PR by following these steps:

Impact check

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

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. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • 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.
  • I have checked that the base branch is correctly set.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Fixes #

@thijsoo thijsoo added the changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog label Jun 12, 2024
@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9d27661dd02a2b09f18724a5d3d2f87e91460c2b

Details

  • 13 of 22 (59.09%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 52.857%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/integrations/watchers/indexable-attachment-watcher.php 1 2 50.0%
src/integrations/admin/activation-cleanup-integration.php 2 4 50.0%
src/integrations/cleanup-integration.php 4 7 57.14%
src/integrations/watchers/indexable-author-archive-watcher.php 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
src/integrations/cleanup-integration.php 1 85.29%
Totals Coverage Status
Change from base Build a8818d4d5624f16d9f78369aa77336463be0838a: 0.001%
Covered Lines: 28350
Relevant Lines: 54233

💛 - Coveralls

@thijsoo thijsoo marked this pull request as ready for review June 13, 2024 14:01
@coveralls
Copy link

coveralls commented Jun 13, 2024

Pull Request Test Coverage Report for Build 8e4c246a9641ab9403126878277d2a9402e15398

Details

  • 13 of 23 (56.52%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 52.857%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/integrations/watchers/indexable-attachment-watcher.php 1 2 50.0%
src/integrations/admin/activation-cleanup-integration.php 2 4 50.0%
src/integrations/watchers/indexable-author-archive-watcher.php 0 3 0.0%
src/integrations/cleanup-integration.php 4 8 50.0%
Files with Coverage Reduction New Missed Lines %
src/integrations/cleanup-integration.php 1 84.67%
Totals Coverage Status
Change from base Build a8818d4d5624f16d9f78369aa77336463be0838a: 0.001%
Covered Lines: 28350
Relevant Lines: 54234

💛 - Coveralls

@thijsoo thijsoo changed the base branch from trunk to feature/plugin-fixes June 25, 2024 07:26
Copy link
Member

@igorschoester igorschoester left a comment

Choose a reason for hiding this comment

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

CR 🏗️

Some small comments.

Plus I wonder if it would be worth the refactor now. Creating a helper method for scheduling instead of repeating more logic all over? The only place with another time is the activation-cleanup-integration; Using a day instead of 5 minutes. But could pass that still?
Might also be overkill, it is just a thought 🙂

thijsoo and others added 4 commits June 26, 2024 09:35
… into 21404-when-indexable-creation-is-disabled-cleanups-are-being-scheduled-upon-post-creation

# Conflicts:
#	composer.json
No longer retrieving the option
@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 4ae3d69694156ef946f5bd62add6a8bc45c9fa00

Details

  • 14 of 23 (60.87%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.001%) to 53.98%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/integrations/watchers/indexable-attachment-watcher.php 1 2 50.0%
src/integrations/admin/activation-cleanup-integration.php 2 4 50.0%
src/integrations/watchers/indexable-author-archive-watcher.php 1 3 33.33%
src/integrations/cleanup-integration.php 4 8 50.0%
Files with Coverage Reduction New Missed Lines %
src/integrations/cleanup-integration.php 1 84.67%
Totals Coverage Status
Change from base Build 1399c5599ca932a9e6cf003b6fdc8c47f1614127: 0.001%
Covered Lines: 29222
Relevant Lines: 54442

💛 - Coveralls

Copy link
Member

@igorschoester igorschoester left a comment

Choose a reason for hiding this comment

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

CR && AC ✅

@igorschoester igorschoester added this to the feature/plugin-fixes milestone Jun 27, 2024
@igorschoester igorschoester merged commit aadef3b into feature/plugin-fixes Jun 27, 2024
23 checks passed
@igorschoester igorschoester deleted the 21404-when-indexable-creation-is-disabled-cleanups-are-being-scheduled-upon-post-creation branch June 27, 2024 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When indexable creation is disabled, cleanups are being scheduled upon post creation
3 participants