Skip to content

Add check for archive pages to make sure indexables are only built when needed.#20146

Merged
igorschoester merged 17 commits intofeature/cron-background-indexationfrom
19560-stop-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page
May 1, 2023
Merged

Add check for archive pages to make sure indexables are only built when needed.#20146
igorschoester merged 17 commits intofeature/cron-background-indexationfrom
19560-stop-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page

Conversation

@thijsoo
Copy link
Copy Markdown
Contributor

@thijsoo thijsoo commented Apr 12, 2023

Context

  • We don't want to build indexables for archives pages that are not public.

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where an entry in the indexable table would be created when an archive of a non-public but publicly queryable post type would be visited.
  • Fixes a bug where entries in the indexable table would be created for archives of excluded post types.

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:

  • Do the following without this PR.
  • Create a new post type (lets call it typeA) with the CPT ui plugin.
    • Set Public to false.
    • Set Publicly Queryable to true.
    • Set Has Archive to true.
    • Go to the archive page of this new CPT and verify that a new indexable is created with the object_type post-type-archive
  • Create a new post type (lets call it typeB) with the CPT ui plugin.
    • Set Public to true.
    • Set Publicly Queryable to true.
    • Set Has Archive to true.
    • Exclude that post type from indexable creation, with the following filter:
add_filter( 'wpseo_indexable_excluded_post_types', function( $post_types ) { 
    $post_types[] = '<POST_TYPE_SLUG>'; 
    return $post_types;
} );
  • Either go to the archive page of the typeB CPT or run the SEO optimization and verify that a new indexable is created with the object_type post-type-archive
  • Activate this PR/RC
  • Open a wp shell for your site and run wp transient delete --all
  • Add the following code to /wordpress-seo/admin/class-admin.php at the end of the constructor at around line 118.
    Notice the closing } after the admin init this is to close out the constructor.
		add_action( 'admin_init', [ $this, 'send' ], 1 );
	}

	public function send() {
		if ( filter_input( INPUT_GET, 'action' ) === 'test' ) {
			$collector = new WPSEO_Collector();

			$collector->add_collection(
				YoastSEO()->classes->get( To_Be_Cleaned_Indexables_Collector::class )
			);
			echo '<pre>';
			print_r( $collector->collect() );
			echo '</pre>';
			die;
		}
	}

And add the following use statement at the top of file with the other use statements.

use Yoast\WP\SEO\Analytics\Application\To_Be_Cleaned_Indexables_Collector;

Go to basic.wordpress.test/wp-admin/admin.php?page=wpseo_dashboard&action=test and make sure the cleanup_name with key indexables_for_non_publicly_viewable_post_type_archive_pages has atleast a count of 2.

  • Disable the plugin and re-enable to activate the cleanup routine.
  • With the crontrol plugin find the wpseo_start_cleanup_indexables task and hit run now.
  • Make sure the archive page indexables are now gone.
  • Refresh the archive pages of the CPTs and make sure the archive page indexable are not re-created. Also run an SEO optimization and again confirm that the archive page indexables are not re-created.

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.

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.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins 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.

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.

Fixes #

@thijsoo thijsoo added innovation Innovative issue. Relating to performance, memory or data-flow. changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog labels Apr 12, 2023
@leonidasmi leonidasmi added changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog and removed changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog changelog: enhancement Needs to be included in the 'Enhancements' category in the changelog labels Apr 13, 2023
Comment thread src/helpers/post-helper.php Outdated
@thijsoo thijsoo force-pushed the 19560-stop-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page branch from 5057cf7 to bab033a Compare April 18, 2023 11:28
@thijsoo thijsoo force-pushed the 19560-stop-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page branch from 3bf60e4 to 3919935 Compare April 18, 2023 13:19
@leonidasmi leonidasmi changed the base branch from trunk to feature/cron-background-indexation April 24, 2023 06:37
Copy link
Copy Markdown
Contributor

@leonidasmi leonidasmi left a comment

Choose a reason for hiding this comment

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

We need to also strip the types from the two new methods, as per our convo here. Probably best to add some defensive coding around the usages of those methods, to ensure no weird behavior with false-y values.

Comment thread src/helpers/post-helper.php Outdated
Comment thread src/integrations/cleanup-integration.php Outdated
Comment thread inc/class-upgrade.php
thijsoo and others added 6 commits April 26, 2023 11:13
…p-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page

# Conflicts:
#	src/integrations/cleanup-integration.php
#	tests/unit/integrations/cleanup-integration-test.php
…/wordpress-seo into 19560-stop-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page
Copy link
Copy Markdown
Contributor

@leonidasmi leonidasmi left a comment

Choose a reason for hiding this comment

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

There are a couple of things that need to be added/removed, which I'm gonna share below, but because they are totally independent from the rest of the PR, I want to add that once those are added/removed and tested, we can merge this as the rest is CRed and tested and 👍

We need to:

  • Add a respective count function for the new cleanup action and add it in the To_Be_Cleaned_Indexables_Collector. Make sure to add test instructions for that too.
  • Remove the scheduling of the cleanup from 20.7, because we moved it to 20.8 - unless I'm missing something?

Comment thread src/repositories/indexable-cleanup-repository.php Outdated
Copy link
Copy Markdown
Contributor

@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.

ACC ✅

@igorschoester igorschoester merged commit 2f3665b into feature/cron-background-indexation May 1, 2023
@igorschoester igorschoester deleted the 19560-stop-storing-indexables-for-archive-pages-for-non-public-queriable-post-types-when-accessing-the-archive-page branch May 1, 2023 11:04
@leonidasmi leonidasmi mentioned this pull request May 3, 2023
16 tasks
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 innovation Innovative issue. Relating to performance, memory or data-flow.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stop storing indexables for archive pages for non public queriable post types when accessing the archive page

3 participants