Add placeholders and integration tests to indexables builders queries #20834

pls78 · 2023-11-09T14:59:10Z

Context

This PR takes care of the queries contained in:
- src/builders/indexable-post-type-archive-builder.php
- src/builders/indexable-author-builder.php
- src/builders/indexable-term-builder.php
- src/builders/indexable-home-page-builder.php

Summary

This PR can be summarized in the following changelog entry:

Refactors MySQL raw queries to use the new %i placeholder
Introduces integration tests

Relevant technical choices:

Integration tests have been added to cover the build method for each builder.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Preliminary step

Install the WP Data Access plugin

Without this PR

Go to Tools -> Yoast Test and click Reset indexables tables & migrations`
Perform the SEO Data optimization
Go to WP Data Access
In the upper menu, click on the SQL (Query Builder) tab

In the text field under the New Query tab paste the following query:

select id, object_last_modified, object_published_at from wp_yoast_indexable

Uncheck the Max rows checkbox and click Execute
Click on JSON where it says Export to

With this PR

Perform the exact same steps in the Without this PR section
Go to https://www.diffchecker.com
Open the first and second json file you have created and copy their content in the left and right textbox respectively
Click on Find difference and check the two files are identical

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:

This PR affects only the indexables timestamp generation, so it is sufficient to check that those timestamps are identical with and without the PR

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 that Yoast SEO provides integrations for.
I have added integration 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 #

coveralls · 2023-11-09T15:04:55Z

Pull Request Test Coverage Report for Build 6931962523

30 of 36 (83.33%) changed or added relevant lines in 4 files are covered.
2 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.08%) to 49.368%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/builders/indexable-post-type-archive-builder.php	0	6	0.0%

Files with Coverage Reduction	New Missed Lines	%
inc/class-wpseo-image-utils.php	1	35.71%
src/builders/indexable-post-type-archive-builder.php	1	42.11%

Totals
Change from base Build 6929250472:	0.08%
Covered Lines:	13271
Relevant Lines:	26882

💛 - Coveralls

Also fix corresponding unit test

Integration test for Indexable_Post_Type_Archive_Builder class. This ensures the refactoring of the MySql query doesn't break anything.

Fix corresponding unit test

Fix unit tests accordingly.

thijsoo

Some small comments 🚧

thijsoo · 2023-11-27T09:51:39Z

tests/WP/Builders/Indexable_Author_Builder_Test.php

+ *
+ * @coversDefaultClass Yoast\WP\SEO\Builders\Indexable_Author_Builder
+ */
+class Indexable_Author_Builder_Test extends TestCase {


The cases for author_has_public_posts_wp And $this->options_helper->get( 'noindex-author-noposts-wpseo', false ) and \apply_filters( 'wpseo_should_build_and_save_user_indexable' are not covered.
I think it would be nice to also include those.

thijsoo · 2023-11-27T10:04:11Z

tests/WP/Builders/Indexable_Home_Page_Builder_Test.php

+
+		$this->instance->set_social_image_helpers(
+			YoastSEO()->helpers->image,
+			YoastSEO()->classes->get( 'Yoast\WP\SEO\Helpers\Open_Graph\Image_Helper' ),


You can use YoastSEO()->helpers->open_graph->image here.

thijsoo · 2023-11-27T10:04:24Z

tests/WP/Builders/Indexable_Home_Page_Builder_Test.php

+		$this->instance->set_social_image_helpers(
+			YoastSEO()->helpers->image,
+			YoastSEO()->classes->get( 'Yoast\WP\SEO\Helpers\Open_Graph\Image_Helper' ),
+			YoastSEO()->classes->get( 'Yoast\WP\SEO\Helpers\Twitter\Image_Helper' )


You can use YoastSEO()->helpers->twitter->image here.

thijsoo · 2023-11-27T10:59:11Z

tests/WP/Builders/Indexable_Term_Builder_Test.php

+
+		$this->instance->set_social_image_helpers(
+			YoastSEO()->helpers->image,
+			YoastSEO()->classes->get( 'Yoast\WP\SEO\Helpers\Open_Graph\Image_Helper' ),


Same as above with using the ->open_graph-> and ->twitter->

pls78 mentioned this pull request Nov 13, 2023

Add placeholders for wordpress-seo #20702

Open

pls78 changed the title ~~Indexables queries~~ Indexables builders queries Nov 13, 2023

pls78 added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Nov 17, 2023

pls78 force-pushed the indexables-queries branch from c6ba8b8 to 7f1f994 Compare November 20, 2023 14:27

pls78 added 21 commits November 23, 2023 11:22

Introduce %i placeholder

ef1c669

Also fix corresponding unit test

Add integration test

cf0574e

Integration test for Indexable_Post_Type_Archive_Builder class. This ensures the refactoring of the MySql query doesn't break anything.

Fix cs

74a4eee

Use common array notation

765fd27

Introduce %i placeholder

5c7b47d

Fix corresponding unit test

Add integration test

3dcc34a

Use better variable names

4afe1b2

Introduce %i placeholder

981fe78

Fix corresponding unit test

Remove unused import

fe10a04

Use factories for objects creation

07d97b9

Fix trailing comma

1396545

Add integration test for the happy path only

f6b1700

Fix post_status

96fab16

Fix imports order

c4e1f8e

Fix cs

86be2b3

Introduce %i placeholder

2b9196c

Fix corresponding unit test

WIP: Add integration test

05fd9c8

Added another integration test

6b6a0a9

Fix unit test

27b5d36

Fix cs

078c655

Fix cs

8dc0d16

pls78 force-pushed the indexables-queries branch from e84ffe9 to 8dc0d16 Compare November 23, 2023 10:22

pls78 added 3 commits November 23, 2023 12:25

Update tests to the new standards

fc877cd

Use placeholder for field names too.

c3b0a21

Fix unit tests accordingly.

Extended integration test

8c8ced0

pls78 added 8 commits November 23, 2023 16:57

Fix cs

e24d0aa

Restructured and expanded integrationt test

ca84f92

Removed mocked classes

99a5663

Refactor and expand tests

450034f

Final polishing

2313691

Try to fix github fail

7cb00d0

Remove wrong test

6bda175

Fix cs

fcd13d7

pls78 marked this pull request as ready for review November 26, 2023 23:29

pls78 changed the title ~~Indexables builders queries~~ Add placeholders and integration tests to indexables builders queries Nov 27, 2023

thijsoo self-assigned this Nov 27, 2023

thijsoo requested changes Nov 27, 2023

View reviewed changes

pls78 added 2 commits November 27, 2023 14:25

Use correct helpers surface

09c8854

Add case to integration test

8fc0a13

thijsoo added this to the 21.7 milestone Nov 28, 2023

thijsoo merged commit 64df16f into trunk Nov 28, 2023
22 checks passed

thijsoo deleted the indexables-queries branch November 28, 2023 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add placeholders and integration tests to indexables builders queries #20834

Add placeholders and integration tests to indexables builders queries #20834

pls78 commented Nov 9, 2023 •

edited by enricobattocchi

Loading

coveralls commented Nov 9, 2023 •

edited

Loading

thijsoo left a comment

thijsoo Nov 27, 2023

thijsoo Nov 27, 2023

thijsoo Nov 27, 2023

thijsoo Nov 27, 2023

Add placeholders and integration tests to indexables builders queries #20834

Add placeholders and integration tests to indexables builders queries #20834

Conversation

pls78 commented Nov 9, 2023 • edited by enricobattocchi Loading

Context

Summary

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

Preliminary step

Without this PR

With this PR

Relevant test scenarios

Test instructions for QA when the code is in the RC

Impact check

UI changes

Other environments

Documentation

Quality assurance

Innovation

coveralls commented Nov 9, 2023 • edited Loading

Pull Request Test Coverage Report for Build 6931962523

💛 - Coveralls

thijsoo left a comment

Choose a reason for hiding this comment

thijsoo Nov 27, 2023

Choose a reason for hiding this comment

thijsoo Nov 27, 2023

Choose a reason for hiding this comment

thijsoo Nov 27, 2023

Choose a reason for hiding this comment

thijsoo Nov 27, 2023

Choose a reason for hiding this comment

pls78 commented Nov 9, 2023 •

edited by enricobattocchi

Loading

coveralls commented Nov 9, 2023 •

edited

Loading