Skip to content

Conversation

@izzygld
Copy link
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
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
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;

},

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;

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.

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;

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

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
8 of 33 checks passed
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