Problem/Motivation

While working on #3445150: Add validation constraints to core.entity_view_mode.*.* I notices something weird.

 /**
   * Implements hook_entity_form_mode_presave().
   *
   * Transforms empty description into null.
   */
  #[Hook('hook_entity_form_mode_presave')]
  public function systemEntityFormModePresave(EntityInterface $entity): void {
    if ($entity->get('description') !== NULL && trim($entity->get('description')) === '') {
      @trigger_error("Setting description to an empty string is deprecated in drupal:11.2.0 and it must be null in drupal:12.0.0. See https://www.drupal.org/node/3452144", E_USER_DEPRECATED);
      $entity->set('description', NULL);
    }
  }

}

This reference seems wrong. This seems pretty important to get in, since it is not triggering now, which will make config invalid for d12 when it is slated for removal.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3559247

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:

Comments

bbrala created an issue. See original summary.

bbrala’s picture

Status: Active » Needs review
bbrala’s picture

Unrealted testfailure i think.

godotislate’s picture

I know there aren't always tests for deprecations, but does this need one?

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

I was on the fence about whether to request a test here. On the one hand, if we'd had one already, this would have been caught. On the other hand, I don't think this is likely to regress once fixed.

And on the other other hand, I would not be surprised if this gets backported, since the wrong hook name means this hook has never actually been invoked. So it's not like this issue is likely to be held to a tight beta deadline.

I guess we can leave that decision up to the committers, and mark this tentatively RTBC.

longwave’s picture

Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Agree that it's not really worth adding a test here, even if a test would have caught this in the first place. Let's just get this fixed and move on.

Committed and pushed ca97653a353 to 11.x and a62e112fb8d to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • longwave committed a62e112f on 11.3.x
    fix: #3559247 Wrong hook reference in SystemHooks.php
    
    By: bbrala
    By:...

  • longwave committed ca97653a on 11.x
    fix: #3559247 Wrong hook reference in SystemHooks.php
    
    By: bbrala
    By:...

mstrelan’s picture

Shouldn't the docblock also be updated to Implements hook_ENTITY_TYPE_presave?

Status: Fixed » Closed (fixed)

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