Problem/Motivation
We use, for instance, NodeType::load('page')->label() to get a bundle's label. But there is also the entity_type.bundle.info, which acts like an entity bundle factory. In the case when hook_entity_bundle_info_alter() alters this label, NodeType::load('page')->label() will return a different value than \Drupal::service('entity_type.bundle.info')->getBundleInfo('node')['page']['label']. This might create confusion at API level as it's not clear which label getter is correct.
Steps to reproduce
A test to prove the bug https://www.drupal.org/pift-ci-job/1905506 (commit 740e84)
Proposed resolution
Deprecate the possibility to alter, in hook_entity_bundle_info_alter(), the label of a bundle defined as config entity. Third party code will have to use other methods to alter the bundle's label in such cases. Depending on the project several methods are available:
- The simplest way is to change the bundle entity label as is stored in database.
- Implement
hook_entity_load()orhook_ENTITY_TYPE_load(). - Swap the bundle entity class and implement a custom
::label()method.
Disallow using hook_entity_bundle_info_alter() to alter labels of bundles stored as config entities in Drupal 10.0.0.
Remaining tasks
None.
User interface changes
None.
API changes
Using hook_entity_bundle_info_alter() to alter labels of bundles stored as config entities is deprecated in Drupal 9.2.0.
Data model changes
None.
Release notes snippet
N/A
Issue fork drupal-3186688
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
Comment #3
claudiu.cristeaAdding test for "Steps to reproduce".
Comment #4
claudiu.cristeaComment #5
claudiu.cristeaComment #8
larowlanLeft a minor comment, but this looks good
We should reference bundle classes as the preferred way to do this
Comment #9
larowlanComment #10
claudiu.cristeaFixed the deprecation version in code & CR
Comment #11
larowlanLeft another comment, could be micro-optimization, but I think it's worth it
Comment #12
claudiu.cristeahook_entity_bundle_info_alter().Comment #13
larowlanThanks @claudiu.cristea, this looks good to me
Comment #15
catchI'm not sure about this change.
First of all I saw the 'change to an exception in 10.0' comment, and that made me realise this code would have to stick around permanently. Then I thought about whether we could just not fire the info hook for config bundle entities at all, but it's callled on the whole info array not per-bundle so that's no good either.
I think instead of adding quite a lot of code to enforce this, we should improve the documentation of hook_entity_bundle_info_alter() instead to mention that config entities shouldn't be altered in there.