Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Updated: Comment #N
Problem/Motivation
Once #2005716: Promote EntityType to a domain object goes in, we will have a lot of code like EntityTypeInterface $entity_info
, and $entity_type will be a string still. This is confusing.
Proposed resolution
Replace $entity_type with $entity_type_id
Any time that $entity_info is an array of all entity types, use $entity_types
When $entity_info is the entity type itself, use $entity_type
Remaining tasks
Write patch
User interface changes
N/A
API changes
N/A
Comment | File | Size | Author |
---|---|---|---|
#14 | rename-entity-info-2165155-14.patch | 277.52 KB | Berdir |
#13 | rename-entity-info-2165155-13-interdiff.txt | 29.29 KB | Berdir |
#13 | rename-entity-info-2165155-13.patch | 276.98 KB | Berdir |
#9 | rename-entity-info-2165155-9.patch | 276.12 KB | tim.plunkett |
#9 | interdiff.txt | 2.15 KB | tim.plunkett |
Comments
Comment #1
fago#2005716: Promote EntityType to a domain object went in.
Comment #2
BerdirWell, I guess that's what we get for renames like this :)
Only renamed $entity_info/$info and $entity-type just when it was in the way. No idea how to identify $entity_type's that are ID's, also not sure if we need to.
One part I didn't touch, and that's hook_entity_info() and the _alter(), not sure what to do about that' Just rename $entity_info and worry about the hook in a different issue?
Comment #3
tim.plunkettWe could do the renaming of the alter in #1981858: Rename hook_entity_info/alter() to hook_entity_type_build/alter() as well.
Comment #5
BerdirDoing the hook rename over there would be OK with me.
Missed to update route defaults/arguments, this should fix most of those tests, didn't check if all are due to this.
Comment #7
BerdirFixing more routing stuff...
Comment #9
tim.plunkettBad regex I guess, these changes to modules/config weren't needed.
Comment #10
BerdirI didn't do any regex-ing, that was all PhpStorm rename refactoring, weird. it did something similar to some other arguments.
I tried that form manually and it worked, so I didn't look closer, I guess I just tested the ajax part that still worked?
Comment #11
tim.plunkettYeah, those arguments are only used if you deep-link to a specific export, or use the form with AJAX off.
Comment #12
dawehnerLet's fill a follow up to change the entity query class to also use something like getEntityTypeId.
it is a bit sad that we should probably better update the documentation in another follow up. this patch is big enough already.
Another follow up: typehint all those instances.
My head thought for a while that entityId is an interface, luckily it won't and my head did not exploded.
Comment #13
BerdirThanks for the review!
1. #2191651: Rename Drupal\Core\Entity\Query\QueryInterface::getEntityType() to getEntityTypeId()
2. Fixed all "entity info" that made sense in this context, left a few related to entity_info_cache_clear() and hook_entity_info(), will clean that up in the issue mentioned above.
3. #2191655: Type hint $entity_type everywhere it is an instance of EntityTypeInterface.
4. Yeah, PhpStorm doesn't like our buildForm() trick with additional optional arguments. If you try to rename a variable there, it attempts to match that *somehow* in all buildForm() implemetnations we have. For example, I tried to rename that back using PhpStorm and it removed _$id from $action_id, $ban_id, $entity_type_id, $test_id and so on, $resulting in $ban_, $action_, o.0
Together with #1981858: Rename hook_entity_info/alter() to hook_entity_type_build/alter(), which I will address next, this should allow us to remove "entity_info" completely :)
Comment #14
BerdirRe-roll.
Comment #15
dawehnerAwesome
Comment #16
Berdir14: rename-entity-info-2165155-14.patch queued for re-testing.
Comment #17
Berdir14: rename-entity-info-2165155-14.patch queued for re-testing.
Comment #18
alexpottCommitted 6c2ed3f and pushed to 8.x. Thanks!