Updated: Comment #0

Problem/Motivation

There's a growing confusion about methods on entities to get their label, title, name and etc
There's unified read only label() but setters are different

Proposed resolution

Provide a EntityLabeledInterface with setLabel() method
Get rid of aliasing methods $node->setTitle(), $comment->setSubject(), $term->setName() and corresponding getters

Remaining tasks

tbd

User interface changes

no

API changes

no, just api addition

CommentFileSizeAuthor
#23 implement_entitylabeledinterface-2199917-23.patch49.35 KBsutharsan
#17 implement_entitylabeledinterface-2199917-17.patch49.64 KBsandrey
#13 drupal-implement_entitylabeledinterface-2199917-13.patch15.19 KBsandrey
#11 drupal-implement_entitylabeledinterface-2199917-11.patch12.64 KBsandrey
#4 drupal_core-implement_entitylabeledinterface-2199917-4.patch12.92 KBsandrey
#1 entity-labelled-2199917.1.patch8.52 KBlarowlan

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review
StatusFileSize
new entity-labelled-2199917.1.patch8.52 KB

Lets see what bot says

Also Feeds and Messages have setTitle - possible use-case to implement there too?

Status: Needs review » Needs work

The last submitted patch, 1: entity-labelled-2199917.1.patch, failed testing.

sandrey’s picture

sandrey commented
Assigned: Unassigned » sandrey

The task will be resolved during Summer Drupal Code Sprint Novosibirsk 2014 #1

sandrey’s picture

sandrey commented
StatusFileSize
new drupal_core-implement_entitylabeledinterface-2199917-4.patch12.92 KB

by sandrey and Andrew.Mikhailov

best regards!

sandrey’s picture

sandrey commented
Status: Needs work » Needs review
sandrey’s picture

sandrey commented
Assigned: sandrey » Unassigned

Please review

Crell’s picture

Crell commented

Should the getter method also move to this interface, or is there a good reason to keep those separate?

sandrey’s picture

sandrey commented

I think the correct solution is to move the getter method to this interface.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work 
+++ b/core/modules/comment/src/CommentInterface.php
@@ -105,6 +106,17 @@ public function getSubject();
+  public function setLabel($subject);

+++ b/core/modules/node/src/NodeInterface.php
@@ -45,6 +46,17 @@ public function getTitle();
+  public function setLabel($title);

+++ b/core/modules/taxonomy/src/TermInterface.php
@@ -70,6 +71,16 @@ public function getName();
+  public function setLabel($name);

You don't need to redefine this method at all.

andrew.mikhailov’s picture

andrew.mikhailov commented

Thanks!

sandrey’s picture

sandrey commented
Status: Needs work » Needs review
StatusFileSize
new drupal-implement_entitylabeledinterface-2199917-11.patch12.64 KB

The patch has been updated according to #7 and #9 comments. tim.plunkett thanks, that's good point.

by sandrey and Andrew.Mikhailov
Best regards!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
  1. +++ b/core/lib/Drupal/Core/Entity/EntityLabeledInterface.php
@@ -0,0 +1,29 @@
+namespace Drupal\Core\Entity;
+
+
+interface EntityLabeledInterface {

    Doesn't need the extra line here. 

  2. +++ b/core/lib/Drupal/Core/Entity/EntityLabeledInterface.php
@@ -0,0 +1,29 @@
+   * @return $this
+   */
+  public function getLabel();

    getLabel should return a string.

    Actually, why do we even have this? We should just be moving EntityInterface::label() to this interface, right? 

  3. +++ b/core/modules/comment/src/Entity/Comment.php
@@ -367,6 +367,13 @@ public function getSubject() {
+  public function getLabel() {
+    return $this->getSubject();

@@ -375,6 +382,13 @@ public function setSubject($subject) {
+    return $this->setSubject($subject);

    I think we should use the $this->get('subject')/$this->set('subject') bit directly here.

sandrey’s picture

sandrey commented
StatusFileSize
new drupal-implement_entitylabeledinterface-2199917-13.patch15.19 KB

corrected

Status: Needs review » Needs work

The last submitted patch, 13: drupal-implement_entitylabeledinterface-2199917-13.patch, failed testing.

Status: Needs work » Needs review

sandrey queued 13: drupal-implement_entitylabeledinterface-2199917-13.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 13: drupal-implement_entitylabeledinterface-2199917-13.patch, failed testing.

sandrey’s picture

sandrey commented
Status: Needs work » Needs review
StatusFileSize
new implement_entitylabeledinterface-2199917-17.patch49.64 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/lib/Drupal/Core/Entity/EntityLabeledInterface.php
@@ -0,0 +1,39 @@
+  public function label();
...
+  public function getLabel();

Please remove getLabel() from everywhere. It should always be label()

Status: Needs review » Needs work

The last submitted patch, 17: implement_entitylabeledinterface-2199917-17.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs reroll
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I don't really understand the reason for this issue, what is the use case for setting the label of an entity without knowing what kind of entity it is?

The label is by the current definition read-only by design and might be altered and not the same as the username, for example. Users are messed up right now, but with #2112679: getUsername() should return the username getDisplayName() for the formatted user name, getUsername() will return the raw username, while getDisplayName()/label() will return a possibly altered (for example by realname.module) username. So what would $user->setLabel() then do exactly?

andypost’s picture

andypost
he/him
Primary language Russian
commented

@berdir the primary reason is DX, the only confusing thing in the proposed is messing of hasKey('label') and newly instance_of EntityLabeledInterface

otoh we have a EntityChangedInterface and EntityOwnerInterface so subject looks sane.

So what would $user->setLabel() then do exactly?
getDisplayName() != label() is you said label() is readonly property it can't be a renderable/rendered stuff and result should be always sanitized.

As we have now label() - Returns the label of the entity. and once entity implements proper interface that result could be changed with defined method. This would allow contrib authors much easy operate with entities.

sutharsan’s picture

sutharsan commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new implement_entitylabeledinterface-2199917-23.patch49.35 KB

Rerolled. Changed in core:
* hook_comment_presave() removed.
* Method randomName() renamed to randomMachineName()

Status: Needs review » Needs work

The last submitted patch, 23: implement_entitylabeledinterface-2199917-23.patch, failed testing.

Status: Needs work » Needs review

rpayanm queued 23: implement_entitylabeledinterface-2199917-23.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 23: implement_entitylabeledinterface-2199917-23.patch, failed testing.

daffie’s picture

daffie commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Postponed
Parent issue: #2016679: Expand Entity Type interfaces to provide methods, protect the properties »

This will not pass the beta changes. Postponed to version 8.1.x, but most probably postponed until 9.x.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Related issues: +#2921988: Deprecate Node::getTitle() & other entity-type-specific calls in favour of Entity::label()

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Postponed » Needs work

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.