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
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:
- 3559247-wrong-hook-reference
changes, plain diff MR !13901
Comments
Comment #3
bbralaComment #4
bbralaUnrealted testfailure i think.
Comment #5
godotislateI know there aren't always tests for deprecations, but does this need one?
Comment #6
phenaproximaI 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.
Comment #7
longwaveAgree 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!
Comment #12
mstrelan commentedShouldn't the docblock also be updated to
Implements hook_ENTITY_TYPE_presave?