Fix attempt to read post ID on update_post_thumbnail_cache #6636

itzmekhokan · 2024-05-26T13:54:02Z

Fix attempt to read post ID on update_post_thumbnail_cache function as some cases $wp_query->posts return array of post IDs instead of WP_Post object.

Trac ticket: #59521

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2024-05-26T13:54:20Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props khokansardar, peterwilsoncc, mukesh27, rajinsharwar.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

mukeshpanchal27

Thanks @itzmekhokan for PR. Could you please add unit tests that cover that scenario?

mukeshpanchal27 · 2024-05-27T05:54:51Z

src/wp-includes/post-thumbnail-template.php

+		/*
+		 * Check $post is valid post object of WP_Post or not,
+		 * while some cases it can be an array of $posts IDs.
+		 * 


Suggested change

*

*

Fix PHPCS

@mukeshpanchal27 fixed and updated test cases.

github-actions · 2024-05-27T09:43:36Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Rajinsharwar · 2024-06-20T04:12:22Z

tests/phpunit/tests/post/thumbnails.php

@@ -79,11 +80,28 @@ public function test_update_post_thumbnail_cache() {
 			)
 		);

+		// Test case where `$query1->posts` should return Array of post IDs.
+		$query1 = new WP_Query(


Nit pick, let's use 2.

Suggested change

$query1 = new WP_Query(

$query2 = new WP_Query(

Rajinsharwar · 2024-06-20T04:13:19Z

tests/phpunit/tests/post/thumbnails.php

+		$this->assertFalse( $query1->thumbnails_cached );
+
+		update_post_thumbnail_cache( $query1 );
+
+		$this->assertTrue( $query1->thumbnails_cached );


Suggested change

$this->assertFalse( $query1->thumbnails_cached );

update_post_thumbnail_cache( $query1 );

$this->assertTrue( $query1->thumbnails_cached );

$this->assertFalse( $query2->thumbnails_cached );

update_post_thumbnail_cache( $query2 );

$this->assertTrue( $query2->thumbnails_cached );

Rajinsharwar · 2024-06-20T04:13:46Z

tests/phpunit/tests/post/thumbnails.php

@@ -79,11 +80,28 @@ public function test_update_post_thumbnail_cache() {
 			)
 		);

+		// Test case where `$query1->posts` should return Array of post IDs.


Suggested change

// Test case where `$query1->posts` should return Array of post IDs.

// Test case where `$query2->posts` should return Array of post IDs.

Rajinsharwar · 2024-06-20T04:14:09Z

tests/phpunit/tests/post/thumbnails.php

 		$this->assertFalse( $query->thumbnails_cached );

 		update_post_thumbnail_cache( $query );

 		$this->assertTrue( $query->thumbnails_cached );
+
+		// Check test cases for `$query1`.


Suggested change

// Check test cases for `$query1`.

// Check test cases for `$query2`.

mukeshpanchal27 · 2024-06-20T04:36:00Z

src/wp-includes/post-thumbnail-template.php

+		 *
+		 * See https://core.trac.wordpress.org/ticket/59521.
+		 */
+		if ( ! ( $post instanceof WP_Post ) ) {


Suggested change

if ( ! ( $post instanceof WP_Post ) ) {

if ( ! $post instanceof WP_Post ) {

Nit-pick

peterwilsoncc

I've added some notes.

The main one is to avoid repeating code that already exists in get_post_thumbnail_id()

src/wp-includes/post-thumbnail-template.php

peterwilsoncc

This looks good to me.

In order to be able to get it in the current release, I took the liberty of pushing some changes to this PR (sorry I missed them in my prior review):

renamed a variable and improve the comment I originally suggested
split the tests in to two: one for querying post objects, one for querying the post IDs
merged in trunk.

peterwilsoncc

@aaronjorbin has validated the tests changes so I'll put this in for beta 3.

peterwilsoncc · 2024-10-14T22:21:29Z

Merged in r59235 / 7685041

Thank you!

Fix attempt to read post ID on update_post_thumbnail_cache

0b8fdf7

mukeshpanchal27 suggested changes May 27, 2024

View reviewed changes

Update test cases

1960525

itzmekhokan requested a review from mukeshpanchal27 May 27, 2024 09:31

Fix phpcs missing comma

134f2c7

Rajinsharwar approved these changes Jun 20, 2024

View reviewed changes

mukeshpanchal27 reviewed Jun 20, 2024

View reviewed changes

Fix suggested changes

56c8eed

itzmekhokan requested review from mukeshpanchal27 and Rajinsharwar June 23, 2024 15:45

peterwilsoncc requested changes Oct 7, 2024

View reviewed changes

src/wp-includes/post-thumbnail-template.php Outdated Show resolved Hide resolved

src/wp-includes/post-thumbnail-template.php Show resolved Hide resolved

itzmekhokan and others added 4 commits October 10, 2024 11:19

Addressed suggested changes for post thumbnail cache

91d9de0

Improve comment and variable name.

8000b0b

Split tests.

7035e75

Merge branch 'trunk' into ticket/59521

97a37d5

peterwilsoncc approved these changes Oct 13, 2024

View reviewed changes

peterwilsoncc force-pushed the ticket/59521 branch from 7106868 to 4cec893 Compare October 13, 2024 22:57

Test any call to prime posts contains the parent IDs.

729e7da

peterwilsoncc force-pushed the ticket/59521 branch from 4cec893 to 729e7da Compare October 13, 2024 23:09

Merge branch 'trunk' into ticket/59521

40344ba

peterwilsoncc reviewed Oct 14, 2024

View reviewed changes

peterwilsoncc closed this Oct 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix attempt to read post ID on update_post_thumbnail_cache #6636

Fix attempt to read post ID on update_post_thumbnail_cache #6636

itzmekhokan commented May 26, 2024

github-actions bot commented May 26, 2024 •

edited

Loading

mukeshpanchal27 left a comment

mukeshpanchal27 May 27, 2024

itzmekhokan May 27, 2024

github-actions bot commented May 27, 2024

Rajinsharwar Jun 20, 2024

Rajinsharwar Jun 20, 2024

Rajinsharwar Jun 20, 2024

Rajinsharwar Jun 20, 2024

mukeshpanchal27 Jun 20, 2024

peterwilsoncc left a comment

peterwilsoncc left a comment

peterwilsoncc left a comment

peterwilsoncc commented Oct 14, 2024

	// Test case where `$query1->posts` should return Array of post IDs.
	// Test case where `$query2->posts` should return Array of post IDs.

	// Check test cases for `$query1`.
	// Check test cases for `$query2`.

	if ( ! ( $post instanceof WP_Post ) ) {
	if ( ! $post instanceof WP_Post ) {

Fix attempt to read post ID on update_post_thumbnail_cache #6636

Fix attempt to read post ID on update_post_thumbnail_cache #6636

Conversation

itzmekhokan commented May 26, 2024

github-actions bot commented May 26, 2024 • edited Loading

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

mukeshpanchal27 May 27, 2024

Choose a reason for hiding this comment

itzmekhokan May 27, 2024

Choose a reason for hiding this comment

github-actions bot commented May 27, 2024

Test using WordPress Playground

Some things to be aware of

Rajinsharwar Jun 20, 2024

Choose a reason for hiding this comment

Rajinsharwar Jun 20, 2024

Choose a reason for hiding this comment

Rajinsharwar Jun 20, 2024

Choose a reason for hiding this comment

Rajinsharwar Jun 20, 2024

Choose a reason for hiding this comment

mukeshpanchal27 Jun 20, 2024

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

peterwilsoncc commented Oct 14, 2024

github-actions bot commented May 26, 2024 •

edited

Loading