Problem/Motivation

Steps to reproduce

When exporting feedback, for every feedback entry without a message, a deprecation warning, as follows is logged:

Deprecated function: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in Drupal\admin_feedback\Controller\AdminFeedbackController->exportDbFeedback() (line 296 of /opt/drupal/web/modules/contrib/admin_feedback/src/Controller/AdminFeedbackController.php)

Proposed resolution

The issue can be easily mitigated by using a null coalescing operator to provide a blank string. Implementing this as suggested would change Line 296 from:

      $message_formatted = trim(preg_replace('/\s+/', '  ', $row->feedback_message));

to:

      $message_formatted = trim(preg_replace('/\s+/', '  ', $row->feedback_message ?? ''));

Remaining tasks

Create a merge request to ensure that null is not passed to the third argument of the preg_replace function.

CommentFileSizeAuthor
#12 after.png69.37 KBkalash-j
#12 befor.png101.47 KBkalash-j
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

winstonjf created an issue. See original summary.

winstonjf changed the visibility of the branch 3437584-deprecated-function-pregreplace to hidden.

immaculatexavier made their first commit to this issue’s fork.

immaculatexavier’s picture

Status: Active » Needs work

The above MR is to ensure that null is not passed to the third argument of the preg_replace function.

winstonjf changed the visibility of the branch 3437584-deprecated-function-pregreplace to active.

manish-31 made their first commit to this issue’s fork.

manish-31’s picture

@immaculatexavier Thanks for the MR.

I have udated the MR and verified that the warning is no longer triggered. Needs review.

MR Updates: Replaced null with NULL, replaced if/else statement with ternary operator.

manish-31’s picture

Status: Needs work » Needs review
vivek panicker’s picture

Status: Needs review » Needs work
manish-31’s picture

Status: Needs work » Needs review

Thanks @Vivek Panicker

Updated the MR. Needs review.

kalash-j’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new101.47 KB
new69.37 KB

I have tested this on my local and after applying the MR , it doesnt show any error msg.

bserem made their first commit to this issue’s fork.

bserem’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for the work and the test.

Merged

bserem’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.