Attempt to read property "value" on array in Drupal\do_username\DOComputedFields->getValue()

Created on 18 October 2023, 8 months ago
Updated 20 October 2023, 8 months ago

Problem/Motivation

The module generates an error in some cases.
Warning: Attempt to read property "value" on array in Drupal\do_username\DOComputedFields->getValue() (line 37 of /app/web/modules/contrib/do_username/src/DOComputedFields.php).

Steps to reproduce

Set the field value to a user where the bio is not set.

Proposed resolution

Check if the `field_bio` exists and contains properties before attempting to read them.

Remaining tasks

Fix the error. We can use the null-safe operator here (making it a single-character fix).

🐛 Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

🇨🇦Canada hussainweb

Live updates comments and jobs are added and updated live.
  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Comments & Activities

  • Issue created by @hussainweb
  • Assigned to Shreya_98
  • First commit to issue fork.
  • 🇮🇳India sarwan

    Hi @hussainweb,
    I have fixed this issue " Attempt to read property "value" on array in Drupal\do_username\DOComputedFields->getValue()" and also attached patch ,
    please review and verify.

  • 🇮🇳India jagraj_singh_gill

    Hello @hussainweb,
    I have fixed this issue 'Attempt to read property "value" on array in Drupal\do_username\DOComputedFields->getValue()' and also attached patch.
    I feel there were only one change required as all other fields were not fetching any array data.
    Please review and verify.

    Thank You !!

  • @shreya_th opened merge request.
  • Issue was unassigned.
  • Status changed to Needs review 8 months ago
  • 🇮🇳India Shreya_98

    @sarwan_verma i have applied your patch on my local but it doesn't resolved my error.

  • 🇮🇳India Shreya_98

    Hi @hussainweb,
    I have fixed this issue and it is also working fine on my local . Also created MR for this issue . Kindly review the changes.

    Thank you!

  • 🇨🇦Canada hussainweb

    Thanks for the fixes.

    @Shreya_th, I see your patch and MR are different. The patch has accidentally changed the indentation.

    +++ b/src/DOComputedFields.php
    @@ -33,42 +33,59 @@ class DOComputedFields extends TypedData {
    +    return is_array($userInformation->field_bio) && isset($userInformation->field_bio[0]['value']) ? $userInformation->field_bio[0]['value'] : 'N/A';
    

    I see you are treating the `$userInformation` as a Drupal field value. It is not. It is a simple PHP class and the `field_bio` (magic) property is not an array (unless it is empty). So the above code will always return "N/A". Yes, it won't throw an error.

    In your MR, on the other hand, you are using a function called `get` but that function doesn't exist on the `User` class at all (nor on the base class).

  • Status changed to Needs work 8 months ago
  • 🇨🇦Canada hussainweb

    @jagraj_singh_gill, your change is good in theory but we should return an empty string rather than `NULL`. This value gets used within Drupal and I am worried this NULL will throw other errors. We only hit this case when the bio is empty on Drupal.org. In that case, empty string is good enough. Please make this change and I can commit.

  • 🇮🇳India Shreya_98

    Hi @hussainweb,
    Sorry for the mistake, I have made changes as per your requirement . kindly review the changes.

  • 🇨🇦Canada hussainweb

    Thanks, @Shreya_th. I am afraid this doesn't work either. You are reading the expression into a variable and the warning is generated at that point. By the time you check for `!empty`, the warning is already generated.

    The fix is to use the null-coalescing operator.

  • 🇮🇳India jagraj_singh_gill

    Hi @hussainweb,
    Thank you for your feedback. I have revised my patch and attached it. Please review and verify. Thanks again !!

  • Status changed to Fixed 8 months ago
  • 🇨🇦Canada hussainweb

    Committed and pushed. Thanks!

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

Production build 0.69.0 2024