Problem/Motivation
Registration uses this pattern:
$handler = $this->entityTypeManager->getHandler('registration', 'host_entity');
$host_entity = $handler->createHostEntity($entity);
This means there is only ever one host_entity handler, that defined for the registration entity type.
It should use this pattern:
$this->entityTypeManager->getHandler($entity->getEntityTypeId(), 'registration_host');
$host_entity = $handler->createHostEntity($entity);
This would allow sites to define different handlers (and thuse host entity wrappers) for different entity types. Which is the whole point of using a handler AFAIK.
Additionally, by namespacing the handler id as registration_host we prevent collision with a module that has its own idea of host entity.
Steps to reproduce
Handlers are documented here:
https://www.drupal.org/docs/drupal-apis/entity-api/handlers
The method signature is:
* @param string $entity_type_id
* The entity type ID for this handler.
* @param string $handler_type
* The handler type to create an instance for.
*
* @return object
* A handler instance.
*
* @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
*/
public function getHandler($entity_type_id, $handler_type);Proposed resolution
1. Trigger a deprecation error when RegistrationHostEntityHandler is used on the registration entity type. Registration entities can't/shouldn't be hosts.
2. Use hook_entity_type_build() to set RegistrationHostEntityHandler as the default registration_host handler on all entity types.
3. Update all handler uses to use the new pattern.
4. Provide a BC shim using hook_entity_type_alter() and hook_module_implements_alter() in case the host_entity handler is overriden.
Remaining tasks
1. Proof of concept including a single change to use the new pattern in the registration entity class.
2. Fix all the other occurences.
User interface changes
None.
API changes
Using the provided handler for the registration entity type is deprecated.
Data model changes
None.
Issue fork registration-3462121
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
jonathanshawComment #4
jonathanshawComment #5
jonathanshawComment #6
jonathanshawComment #7
john.oltman commentedThanks @jonathanshaw, good catch and what you have looks right to me. There are many instances of "getHandler('registration', 'host_entity')" in the module - I am thinking we should correct those to use the new pattern "getHandler($entity->getEntityTypeId(), 'registration_host_entity');" rather than using the deprecated form of the function call.
Comment #8
jonathanshawI wonder if there's also value in adding a static helper to HostEntity:
Comment #9
john.oltman commentedAdded a commit and tests are now passing. I will try to add a new test this week that proves that the old method triggers a deprecation warning and the new method doesn't. Then I can merge it.
Comment #10
jonathanshawI am having a lot of trouble getting the deprecation test to work on both D10 and D11.
Comment #11
jonathanshawI finally got the deprecation tests working.
Comment #12
john.oltman commentedThis is really close. Left a couple of comments which may result in code changes. Once those comments are addressed we're good. Thanks for adding the deprecation tests.
Comment #13
jonathanshawAnswered on the MR, the code is all good AFAICS.
Comment #14
john.oltman commentedAh, makes sense, and I do now recall you mentioning this earlier. Will merge shortly.
Comment #17
john.oltman commented