Skip to content

fix: Deprecated null value warning in titleRendered callback#3229

Merged
jasonbahl merged 4 commits intowp-graphql:release/v1.29.1from
izzygld:fixtrim
Nov 13, 2024
Merged

fix: Deprecated null value warning in titleRendered callback#3229
jasonbahl merged 4 commits intowp-graphql:release/v1.29.1from
izzygld:fixtrim

Conversation

@izzygld
Copy link
Copy Markdown
Contributor

@izzygld izzygld commented Nov 12, 2024

screencapture-sentry-oustatic-organizations-orthodox-union-issues-109053-2024-11-12-14_43_19

This PR addresses a deprecation warning in PHP 8+ caused by passing a null value to the trim() function within the titleRendered method. Previously, if $this->data->post_title was null, it would be passed to apply_filters, potentially resulting in a null value for trim(). This caused a warning in environments using PHP 8+.

Changes Made:

  • Added a default empty string '' as a fallback for $title if $this->data->post_title is null.
  • This ensures $title is always a string, preventing trim() from encountering a null value.

Reason for Change:

  • This change prevents deprecation warnings when using PHP 8+, improving compatibility and removing unnecessary noise in logs related to deprecated behavior.

Testing Notes:

  • Verified that titleRendered returns the post title if available, or an empty string if not, without causing warnings.
  • Checked that the HTML entity decoding and filter application still work as expected.

@justlevine
Copy link
Copy Markdown
Collaborator

Great catch!

We still want the resolver to return null on an empty string, so we probably want to break the existing return statement up too.

@justlevine justlevine changed the title Fix Deprecated Null Value Warning in titleRendered Function fix Deprecated null value warning in titleRendered callback Nov 12, 2024
@justlevine justlevine added type: bug Issue that causes incorrect or unexpected behavior component: model layer Relating to the Model Layer scope: code quality Refactoring, linting, and enforcing coding standards status: in review Awaiting review before merging or closing needs: reviewer response This needs the attention of a codeowner or maintainer labels Nov 12, 2024
@justlevine justlevine changed the title fix Deprecated null value warning in titleRendered callback fix: Deprecated null value warning in titleRendered callback Nov 12, 2024
@izzygld
Copy link
Copy Markdown
Contributor Author

izzygld commented Nov 12, 2024

Perhaps something like this:

'titleRendered' => function () {
$id = ! empty( $this->data->ID ) ? $this->data->ID : null;
$title = ! empty( $this->data->post_title ) ? $this->data->post_title : '';

// Apply filters and decode HTML entities if title is not empty
$processedTitle = !empty($title) ? $this->html_entity_decode( apply_filters( 'the_title', $title, $id ), 'titleRendered', true ) : '';

// Return null if processed title is empty, otherwise return the processed title
return $processedTitle === '' ? null : $processedTitle;

},

Comment thread src/Model/Post.php Outdated
return $this->html_entity_decode( apply_filters( 'the_title', $title, $id ), 'titleRendered', true );
$processedTitle = !empty($title) ? $this->html_entity_decode( apply_filters( 'the_title', $title, $id ), 'titleRendered', true ) : '';

return $processedTitle === '' ? null : $processedTitle;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid too many return statements within this method.

Comment thread src/Model/Post.php Outdated
Comment thread src/Model/Post.php Outdated
Comment thread src/Model/Post.php
return $this->html_entity_decode( apply_filters( 'the_title', $title, $id ), 'titleRendered', true );
$processedTitle = ! empty( $title ) ? $this->html_entity_decode( apply_filters( 'the_title', $title, $id ), 'titleRendered', true ) : '';

return empty( $processedTitle ) ? null : $processedTitle;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid too many return statements within this method.

@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit ebd83de and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2

View more on Code Climate.

@jasonbahl jasonbahl changed the base branch from develop to release/v1.29.1 November 13, 2024 05:20
@jasonbahl jasonbahl mentioned this pull request Nov 13, 2024
@jasonbahl jasonbahl merged commit 497ed11 into wp-graphql:release/v1.29.1 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: model layer Relating to the Model Layer needs: reviewer response This needs the attention of a codeowner or maintainer scope: code quality Refactoring, linting, and enforcing coding standards status: in review Awaiting review before merging or closing type: bug Issue that causes incorrect or unexpected behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants