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

Fix deprecated notice when using PHP 8.1+ #293

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

Chouby
Copy link
Contributor

@Chouby Chouby commented Mar 14, 2023

The issue

Running Duplicate Post with PHP 8.1 fires a deprecated notice:

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in /app/polylang/tmp/wordpress/wp-includes/taxonomy.php on line 2751

How to test?

			$post = self::factory()->post->create_and_get();
			duplicate_post_admin_init();
			$new_id = duplicate_post_create_duplicate( $post, 'draft' );

How to fix

Pass array() instead of null as second param of wp_set_object_terms().
The reason is that null is transformed to array( null ) and WP applies trim() to all items in this array.

Copy link
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

I fully support this fix, but would love to see some unit tests added to safeguard the fix.

Copy link
Member

@enricobattocchi enricobattocchi left a comment

Choose a reason for hiding this comment

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

The changes make total sense, I'll fix the syntax to comply to CS rules

admin-functions.php Outdated Show resolved Hide resolved
src/post-duplicator.php Outdated Show resolved Hide resolved
@enricobattocchi
Copy link
Member

enricobattocchi commented Mar 24, 2023

I fully support this fix, but would love to see some unit tests added to safeguard the fix.

Agree. We'll try to add tests for that method in the upcoming cycle (though integration tests would really be needed here)

@vraja-pro
Copy link
Contributor

CR & AC ✅

@vraja-pro vraja-pro merged commit 626fc10 into Yoast:trunk Apr 11, 2023
@vraja-pro vraja-pro removed their assignment Apr 11, 2023
@Chouby Chouby deleted the fix-deprecated-notice branch April 13, 2023 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants