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.This is a follow-up after #1818560-92: Convert taxonomy entities to the new Entity Field API
depth could actually be different per parent, so it might make sense to make that a property of $term->parent->depth (and $term->parent[1]->depth), which in turn would be a computed field.
vid()/id()/label() should be used when possible, in case of terms, there's the mess with taxonomy_get_tree() which this issue did *not* introduce, just used it in more places.
In e.g. an edit form for example, you don't want to edit the (possibly altered) label of a term, you want to edit the *name*, so, $term->name->value, not $term->label().
The last example there is because that class does a manual query to load terms, it was broken in HEAD, has no test coverage and this issue fixed it. Yes, it should use entity load/EFQ but that's outside the context of this issue.
Comments
Comment #1
jibran@andypost can you please have a look at it again I think this has been fixed elsewhere.
Comment #2
andypostSuppose we use methods everywhere, and it still makes sense to make depth a computed field
seems we need @berdir here, IS is a copy of his comment (checked)
because I have no idea how to add depth to ER field, we can add a sort of
TermInterface::getDepth($parent_id)with exception if term have no such parentComment #3
BerdirWow, digging up old issues ;)
Yeah, I honestly don't know what to do with this exactly. It's related to the getTree() issue that was moved to 8.1.x. It's ugly, but I'm not sure we can do much about it.
There were many ideas and issues to deal with the whole tree stuff, like @amateescu's plans to introduce generic tree/hierarchy fields.
I think it's too late to do much about it, but we should cross-reference it with those related issues.