-
Notifications
You must be signed in to change notification settings - Fork 898
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
Introduces indexables verification cron #20470
base: trunk
Are you sure you want to change the base?
Conversation
…e data instead of the create method.
…ron-indexation-verification
…ron-indexation-verification # Conflicts: # inc/options/class-wpseo-option-wpseo.php # src/integrations/settings-integration.php
…ron-indexation-verification # Conflicts: # inc/options/class-wpseo-option-wpseo.php
…ron-indexation-verification # Conflicts: # inc/options/class-wpseo-option-wpseo.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🏗️
Quite the PR you created here! 🙇
I think there might be more cases in the plugin deactivated hole though. I had to think of when the permalink structure changes.
Might be worth it to check our watchers for cases.
But could very well be done as a follow up of course 😉
I have not checked the tests and their coverage, because it is late and I'm not working tomorrow, but you are. So let's keep that in mind for the next CR round 😉
src/indexables/application/actions/verify-indexable-action-factory.php
Outdated
Show resolved
Hide resolved
src/indexables/application/actions/verify-indexable-action-factory.php
Outdated
Show resolved
Hide resolved
src/indexables/application/actions/verify-indexable-action-factory.php
Outdated
Show resolved
Hide resolved
src/indexables/user-interface/verification-posts-cron-callback-integration.php
Show resolved
Hide resolved
…ction.php Co-authored-by: Igor <35524806+igorschoester@users.noreply.github.com>
…st/wordpress-seo into feature/cron-indexation-verification
…ron-indexation-verification
* @throws No_Verification_Action_Left_Exception Throws when there are no verification actions left. | ||
* @return Current_Verification_Action | ||
*/ | ||
public function determine_next_verify_action( Current_Verification_Action $current_verification_action_object ): Current_Verification_Action { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be determine_next_verification_action
instead?
* @throws No_Verification_Action_Left_Exception Throws when there are no verification actions left. | ||
* @return Current_Verification_Action | ||
*/ | ||
public function determine_next_verify_action( Current_Verification_Action $current_verification_action_object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be determine_next_verification_action
instead?
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
Context
There are still some todo's left:
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
This PR trys to follow the discussed onion architecture flow.
Some notable new things I introduced here are the infrastructure folder and the port folder in the application.
The infrastructure folder holds the connection to WordPress. This is why I placed my verification actions here. These call wp functions and run queries (via repositories)
I used the port folder to describe the interface of the outdated post repository. This is then implemented in the infrastructure folder, since it then calls wp functions.
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
To make sure the tasks get scheduled when they are supposed to
not
created.add_filter('Yoast\WP\SEO\should_index_indexables','__return_false');
wp cron event run "wpseo_indexable_verify_post_indexables"
and make sure it is unscheduled after 1 run.add_filter('Yoast\WP\SEO\enable_cron_verification','__return_false');
wp cron event run "wpseo_indexable_verify_non_timestamped_indexables"
and make sure it is unscheduled after 1 run.Data prep
select * from wp_yoast_indexable where object_type = 'term' and object_sub_type = 'category';
select * from wp_yoast_indexable where object_type = 'term' and object_sub_type = 'post_tag';
select * from wp_yoast_indexable where object_type = 'post' and object_sub_type = 'page';
select * from wp_yoast_indexable where object_type = 'post' and object_sub_type = 'post';
select * from wp_yoast_indexable where object_type = 'post-type-archive';
Verification and re-syncing the data
wpseo_indexable_verify_non_timestamped_indexables
with the commandwp cron event run "wpseo_indexable_verify_non_timestamped_indexables"
cron until it is no longer scheduled. Now make sure the changes you made in the terms are also represented in your indexable.wpseo_indexable_verify_post_indexables
with the commandwp cron event run "wpseo_indexable_verify_post_indexables"
cron until it is no longer scheduled. Now make sure the changes you made in the posts/pages are also represented in your indexable.Testing the filters
For both the post indexables and the non timestamped indexables (terms) there is a different filter to increase or decrease the amount of indexables done in each batch.
For the post indexables:
- Run
select * from wp_yoast_indexable where object_type = 'post' and object_sub_type = 'post';
and choose 3 posts with their id's in a row (so 1,3,6 is fine as long as there is nog 2,4,5 in between).- Disable free and change something in those posts that is represented in the indexable.
- Add the following filter
add_filter('Yoast\WP\SEO\post_verify_indexing_limit_size',2);
- Run the cli command
wp cron event run "wpseo_indexable_verify_post_indexables"
and make sure only the first two post indexables are updated.For non timestamped indexables:
select * from wp_yoast_indexable where object_type = 'term' and object_sub_type = 'category';
and choose 3 categories with their id's in a row.add_filter('Yoast\WP\SEO\no_timestamped_indexable_verify_limit_size',2);
wp cron event run "wpseo_indexable_verify_non_timestamped_indexables"
and make sure only the first two post indexables are updated.Relevant test scenarios
Test instructions for QA when the code is in the RC
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
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #