Problem/Motivation

After disabling the field "subject", in the message creation page, we get a PHP warning:

Warning: Undefined array key "subject" in Drupal\private_message\Form\PrivateMessageForm->buildForm() (line 301 of modules/contrib/private_message/src/Form/PrivateMessageForm.php).

Steps to reproduce

  1. In the "Manage form display" page (admin/structure/private-message/private-message-thread/form-display), disable the field "Subject".
  2. Go to the "Create Private Message" page (private-message/create), you'll see the mentioned PHP warning.

Issue fork private_message-3480219

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:

About issue forks

Comments

dxvargas created an issue. See original summary.

dxvargas’s picture

dxvargas commented
Title: Field subject can be disabled » Warning when the field subject is disabled

dxvargas opened merge request !128

dxvargas’s picture

dxvargas commented
Status: Active » Needs review

I've created a MR.

PS: I'm (not so) sorry to use the "@", I think no one likes it except me. I think here it's the shortest and cleanest fix. Feel free to add the much more consensual "?? NULL" instead, that's just about the same AFAIK :)

dxvargas’s picture

dxvargas commented
Assigned: dxvargas » Unassigned

Unassigning for someone to review.

alorenc’s picture

alorenc
Location Wolsztyn, 🇵🇱
commented

I think we should not even set the variable is such case

`
// The field subject can be disabled.
if (isset($form_copy['subject'])) {
$form['subject'] = $form_copy['subject'];
}
`

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

zaryab_drupal’s picture

zaryab_drupal commented

Updated code with this below : 

if (isset($form_copy['subject'])) {
$form['subject'] = $form_copy['subject'];
}

The use of the @ symbol in PHP is generally discouraged because it suppresses errors, which can make debugging difficult. Instead, we should use proper checks like isset() or empty() to safely handle variables.

alorenc’s picture

alorenc
Location Wolsztyn, 🇵🇱
commented

MR contains many not-related changes. Yes, some parts are valid, but we should stick to a specific issue.

alorenc’s picture

alorenc
Location Wolsztyn, 🇵🇱
commented
Status: Needs review » Needs work
zaryab_drupal’s picture

zaryab_drupal commented
Status: Needs work » Needs review
dxvargas’s picture

dxvargas commented
Status: Needs review » Reviewed & tested by the community

I agree @zaryab_drupal, like this is better.

There are failing tests which are not related with this MR (I see them also in main branch).
I mark the issue as RTBC.

claudiu.cristea made their first commit to this issue’s fork.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Reviewed & tested by the community » Fixed

Thank you

Status: Fixed » Closed (fixed)

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