Needs work
Project:
Drupal core
Version:
main
Component:
entity system
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
8 Nov 2017 at 20:07 UTC
Updated:
18 Mar 2025 at 16:55 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
berdirgetTitle() was added in the original Node/EntityNG conversion on the request of @webchick because she thought that label() was confusing. That might have changed since then, it was a long time ago and generic entity API was new thing back then. You likely still want to involve here in this.
Comment #3
colanCan someone who knows how to get in touch with her let her know about this issue? She's not in #drupal-contribute and her contact form is disabled. Thanks!
Comment #4
idebr commentedIf/when this is greenlighted we might as well deprecate
Term::getName(), since it is a similar wrapper around $this->label()Nay sayers might argue the entity label should match the input label, so node's 'Title' matches the code Node::getTitle(). This is currently true for the content entity types in core: node, taxonomy term, media. Actually MediaInterface was recently expanded to explicitly use getName / setName in #2897009: MediaInterface is missing setName() and getName()
Comment #5
timmillwoodAssigning to webchick
Comment #6
webchickSure. I'm not gonna argue with people who actually use the subsystem every day. :)
Comment #7
colan#6: Great, thanks!
#4: Yes, I agree. I just updated the title and IS to reflect that we want to do this for all entities, not just Nodes. Please add any others I've missed.
Comment #8
colanComment #9
seanbWimLeers created #2897009: MediaInterface is missing setName() and getName() while working on #2835767-46: Media + REST: comprehensive test coverage for Media + MediaType entity types. I can't find why it was a bug, but maybe he can provide some insight?
Should we also deprecate wrapper methods around
bundle(), likeNodeInterface::getType()?Comment #10
colanMaybe let's keep that as a separate, but related, issue so that we don't try to do too many things as once here?
Comment #11
andypostThis needs separate issues per entity type to clean-up usage as well but first it needs approval
Comment #12
andypostComment #16
colanAs I just ran into this again, and none of those folks have commented yet, I thought it best to ping them on Slack.
Comment #17
hchonovI am +1 on doing the change across all entities. No need for multiple methods doing the same thing, which is actually what is confusing.
Comment #18
hchonovComment #19
colanThanks! Stub change record created. Working on the patch...
Comment #20
colanHere's a first run at this.
Comment #21
colanComment #22
catch+1 I think people are used to the generic entity methods, also sometimes I see something like getType() and have to remind myself it's an alias for bundle().
If this doesn't make it into 8.8.x, it could still go into 8.9.x but the deprecations will need to be for 10.0.x
Comment #23
berdirwhat about getType()/bundle()?
Not sure if we split the issue/issues on entity type or type of method. Lets see how big it will be with all the replacements done in tests and so on.
Comment #26
jungleIn addition to getType()/bundle(), what about setName($name)/set($name, $value, $notify = TRUE) ? Should it be deprecated as well? Whether do it in a separate issue?
Comment #27
berdirno, setName() and set() are not the same thing.
Comment #28
jungleRe #27, let me reword it.
setName() is just an variation of setTitle().
Node::getTitle() is getting deprecated which makes me wondering should Node::setTitle() be deprecated.
Further, I am thinking of introducing a common method setLabel() to the base class ContentEntityBase.
Then setTitle(), setName() calls can be deprecated in favour of using setLabel().
Or deprecate them directly, as setTitle() is just a wrapper of the set() method. (see below). Personally, I prefer the former way -- introduce setLabel(), and deprecate setTitle(), setName()
And here, should
Use \Drupal\Core\Entity\Entity::label() insteadbeUse \Drupal\node\Entity\Node::label() instead?Comment #38
dimitriskr commentedI have some comments regarding the changes and then we can proceed
Comment #39
dimitriskr commented