Problem/Motivation

The following code on the WorkspaceListBuilder.php is causing this problem.
'owner' => $entity->getOwner()->getDisplayName()
There is a chance to get a NULL value from the $entity->getOwner() function when there is no owner

Steps to reproduce

  • Create a workspace with any of the users as owner
  • Delete the user(workspace owner) using the following method "Delete the account and make its content belong to the Anonymous user."
  • Visit workspaces listing page(admin/config/workflow/workspaces)

Proposed resolution

Make sure the value of $entity->getOwner() is not null before calling the getDisplayName() function
eg : 

   $owner = $entity->getOwner();
    if ($owner instanceof UserInterface) {
      // code
    }

User interface changes

CommentFileSizeAuthor
#18 interdiff_15-18.txt1.43 KBranjith_kumar_k_u
#18 3167126-18.patch2.26 KBranjith_kumar_k_u
#15 Screenshot-3167126-afterpatch.jpg488.81 KBranjith_kumar_k_u
#15 3167126-15.patch2.33 KBranjith_kumar_k_u
#15 interdiff_11-15.txt1.11 KBranjith_kumar_k_u
#15 3167126-15-test-only.patch1.25 KBranjith_kumar_k_u
#8 workspaces-get-display-name-on-null-3167126.patch1.14 KBcedricl
#2 workspaces-get-display-name-on-null-3167126-2.patch1.09 KBmheip

Issue fork drupal-3167126

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

mheip created an issue. See original summary.

mheip’s picture

mheip commented
Assigned: mheip » Unassigned
Status: Active » Needs review
StatusFileSize
new workspaces-get-display-name-on-null-3167126-2.patch1.09 KB

Added a patch which checks for the owner and adds fallback to '/' if not available.

amateescu’s picture

amateescu commented
Status: Needs review » Postponed
Related issues: +#3043725: Provide a Entity Handler for user cancelation

Let's wait for the outcome of #3043725: Provide a Entity Handler for user cancelation, and then we can follow what's being done for other core entity types.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

cedricl’s picture

cedricl commented
Version: 9.5.x-dev » 9.4.x-dev
Category: Bug report » Support request
Priority: Major » Normal
Issue summary: View changes
Status: Postponed » Needs review
StatusFileSize
new workspaces-get-display-name-on-null-3167126.patch1.14 KB

Updated patch from #2 for 9.4 support.

smustgrave’s picture

smustgrave commented
Version: 9.4.x-dev » 10.1.x-dev
Category: Support request » Bug report
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs Review Queue Initiative, +Needs tests, +Needs steps to reproduce

The title sounds more like a bug then a support request.

There is no issue summary so that should be added.
This will need steps to reproduce
Test cases

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

rpayanm opened merge request !3769

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Title: Call to a member function getDisplayname() on null in WorkspaceListBuilder.php on line 120 » 3167126-10.1.x

Trying to fix the CM Failed.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Title: 3167126-10.1.x » Call to a member function getDisplayname() on null in WorkspaceListBuilder.php on line 120

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 3167126-15-test-only.patch1.25 KB
new interdiff_11-15.txt1.11 KB
new 3167126-15.patch2.33 KB
new Screenshot-3167126-afterpatch.jpg488.81 KB

Added tests please review

User interface changes
screenshot

do we need any meaningful text here instead of "/"?
or empty?

The last submitted patch, 15: 3167126-15-test-only.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Think it should have some default if no user exists.

Not sure if it's possible to know previous user. Or if some other text should work but just "/" is confusing.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
Status: Needs work » Needs review
StatusFileSize
new 3167126-18.patch2.26 KB
new interdiff_15-18.txt1.43 KB

Added text "User not found" instead of "/" and also updated the tests.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests, -Needs steps to reproduce

Think this is a good change.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 18: 3167126-18.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

Seems random.

amateescu’s picture

amateescu commented
Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs issue summary update

The common way to display information that is not known is the string N/A (stands for not available), let's use that here as well.

+++ b/core/modules/workspaces/src/WorkspaceListBuilder.php
@@ -121,7 +129,7 @@ public function buildRow(EntityInterface $entity) {
+      'owner' => $displayName,

The whole patch could be shortened to: 'owner' => ($owner = $entity->getOwner()) ? $owner->getDisplayName() : $this->t('N/A'),

amateescu opened merge request !6973

amateescu changed the visibility of the branch 3167126-10.1.x to hidden.

amateescu’s picture

amateescu commented
Title: Call to a member function getDisplayname() on null in WorkspaceListBuilder.php on line 120 » Call to a member function getDisplayname() on null in WorkspaceListBuilder.php
Status: Needs work » Needs review

Applied the suggestions from #22 and opened a fresh MR.

smustgrave’s picture

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

Cleaned up patches as fix is in MR.

Test-only was already ran 

1) Drupal\Tests\workspaces\Functional\WorkspaceTest::testWorkspaceList
Behat\Mink\Exception\ResponseTextException: The text "Summer event" was not found anywhere in the text of the current page.
/builds/issue/drupal-3167126/vendor/behat/mink/src/WebAssert.php:907
/builds/issue/drupal-3167126/vendor/behat/mink/src/WebAssert.php:293
/builds/issue/drupal-3167126/core/tests/Drupal/Tests/WebAssert.php:956
/builds/issue/drupal-3167126/core/modules/workspaces/tests/src/Functional/WorkspaceTest.php:309
/builds/issue/drupal-3167126/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 9, Assertions: 162, Errors: 1.

Main parts of issue summary is there.

Fix came from sub-maintainer so can assume fix is approved.

LGTM!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Backported to 10.2.x as an eligible bug fix. The "N/A" string is already used in core so no translation issues.

Committed and pushed 0aaa9b8d49 to 11.x and a69343b5b5 to 10.3.x and 099a9c2139 to 10.2.x. Thanks!

longwave closed merge request !6973

  • longwave committed 099a9c21 on 10.2.x 
    Issue #3167126 by ranjith_kumar_k_u, rpayanm, amateescu, mheip, CedricL...

  • longwave committed a69343b5 on 10.3.x 
    Issue #3167126 by ranjith_kumar_k_u, rpayanm, amateescu, mheip, CedricL...

  • longwave committed 0aaa9b8d on 11.x 
    Issue #3167126 by ranjith_kumar_k_u, rpayanm, amateescu, mheip, CedricL...

Status: Fixed » Closed (fixed)

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