Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Owning user deletion and errors on default formatters (and others) #158

Open
DiegoPino opened this issue Mar 21, 2023 · 1 comment
Open
Assignees
Labels
bug Something isn't working

Comments

@DiegoPino
Copy link
Member

DiegoPino commented Mar 21, 2023

What?

If a SBF on any entity (custom or not) is present on an Object owned by a no longer existing user, an uncaught error happens in this form

The website encountered an unexpected error. Please try again later.
Error: Call to a member function id() on null in Drupal\strawberryfield\Plugin\Field\FieldFormatter\StrawberryDefaultFormatter->viewElements() (line 69 of modules/contrib/strawberryfield/src/Plugin/Field/FieldFormatter/StrawberryDefaultFormatter.php).

The longer story

Drupal handles user removal v/s entities belonging to them as a batch action performed over those entities on the User module. Because we have our own custom entities (provided by every module) we either need to do that too OR we need to allow an Admin to correct ownership after the fact without a whitescreenofdeath

Extra info

Some entities deal with this by exposing the user as anonymous instead of failing. e.g Comment one does this:

/**
   * {@inheritdoc}
   */
  public function getOwner() {
    $user = $this->get('uid')->entity;
    if (!$user || $user->isAnonymous()) {
      $user = User::getAnonymousUser();
      $user->name = $this->getAuthorName();
      $user->homepage = $this->getHomepage();
    }
    return $user;
  }

But e.g for AMI Set Entities we do it wrong

  /**
   * {@inheritdoc}
   */
  public function getOwner() {
    return $this->get('user_id')->entity;
  }

@alliomeria @aksm

@DiegoPino
Copy link
Member Author

partially solved via a9bbb81
We need to check any other custom entity for the same code. Are Metadata Displays dealing with this correctly?
My concern is that just changing the user ownership (via the return) might lead to exposing a private Entity to the public?

@aksm I merged it for now but we need to be sure that returning the Anonymous user is not dangerous, maybe we should do the opposite, return the Admin one?

@DiegoPino DiegoPino transferred this issue from esmero/strawberryfield Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant