Problem/Motivation

When \Drupal\filter\Entity\FilterFormat::$name is NULL deprecations are triggered on PHP 8.1

Steps to reproduce

Run \Drupal\Tests\comment\Kernel\CommentActionsTest

Proposed resolution

Set the default to an empty string? Given we're doing a trim on save() it's probably okay. The other solution is to ensure filter formats always have a label when created in a test.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3240247-6.patch4.26 KBalexpott
#6 2-6-interdiff.txt4.49 KBalexpott
#2 3240247-2.patch498 bytesalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 3240247-2.patch498 bytes
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

I find that's enough, moreover there's todo in the property docs to rename it to label so default anyway should be empty string - for PHP 7.4 it could become protected ?string $label;

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review

I'm not sure - if this is just tests then I think we should do what we did for roles and fix the tests and leave the whole can a label for entities ever be NULL to the follow-up.

daffie’s picture

daffie commented
Status: Needs review » Needs work

To me, the change feels wrong. Almost everywhere in core have entity properties a default value of null. Why do it here now? What is the alternative change to fix this problem for PHP 8.1?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 2-6-interdiff.txt4.49 KB
new 3240247-6.patch4.26 KB

@daffie regardless of your feeling since FilterFormat::preSave() does:

  /**
   * {@inheritdoc}
   */
  public function preSave(EntityStorageInterface $storage) {
    // Ensure the filters have been sorted before saving.
    $this->filters()->sort();

    parent::preSave($storage);

    $this->name = trim($this->label());
  }

the affect of that is that the NULL return of a label() on a saved filter format is impossible.

That said we should explore #4 and see where we're saving a filter format without a label.

Here's a patch that fixes all that I can find and adds an assert.

daffie’s picture

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

I like this change much more.
Looks good to me.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 3fc999a and pushed to 9.3.x. Thanks!

  • catch committed 3fc999a on 9.3.x 
    Issue #3240247 by alexpott, daffie: \Drupal\filter\Entity\FilterFormat...
catch’s picture

catch
he/him
Primary language English
commented

Status: Fixed » Closed (fixed)

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