Problem/Motivation

Split off/Follow up of #1740492: Implement a default entity views data handler

In order to generate the views data, views wants to display some one line description to the user about each entity type.
This functionality could be helpful in various other places.

Proposed resolution

Add a description key to the entity definition and provide a proper description for each entity type.

Remaining tasks

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#29 add_a_description_key-2323779-29.patch17.82 KBmglaman
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,527 pass(es).
[ View ]
#21 add_a_description_key-2323779-21.patch17.82 KBmglaman
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch add_a_description_key-2323779-21.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

mglaman’s picture

Status:Active» Needs work
StatusFileSize
new3.89 KB

Here's a patch with updated EntityType and EntityTypeInterface classes. I started to add a few descriptions but realized I don't have time at the moment to make it through all. Posting incomplete patch for now, so any one else can take a stab at it, or will continue later.

tstoeckler’s picture

Looks great already! Some remarks:

+++ b/core/lib/Drupal/Core/Entity/EntityType.php
@@ -211,6 +211,11 @@ class EntityType implements EntityTypeInterface {
+  protected $description;

@@ -675,4 +680,11 @@ public function getGroupLabel() {
+  public function getDescription() {

+++ b/core/lib/Drupal/Core/Entity/EntityTypeInterface.php
@@ -637,4 +637,12 @@ public function getUriCallback();
+  public function getDescription();

$description should probably default to an empty string, I think.

Super minor, but IMO this should be placed right after $label and getLabel(), respectively.

mglaman’s picture

StatusFileSize
new3.97 KB

Here's re-roll with suggestions from #2 :)

mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new7.69 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,231 pass(es).
[ View ]

Here's updated patch with descriptions added to all ContentEntityType annotations. I excluded all of the specific test entities, as their labels are descriptive and the EntityType class provides a default description of blank text.

dawehner’s picture

These descriptions should be translatable

mglaman’s picture

StatusFileSize
new7.86 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,101 pass(es).
[ View ]

Updated to add @Translation to description key.

dawehner’s picture

Well, these descriptions currently not explain a lot to be honest:

Here is an example what views used to have

"Comments are responses to node content."
mglaman’s picture

Status:Needs review» Needs work

I know, sorry on description fail. I'll work on improving them.

mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new8.16 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,104 pass(es).
[ View ]

Here's a crack at some better entity descriptions.

dawehner’s picture

@mglaman
Do you plan to add descriptions for all the entity types?

mglaman’s picture

When I did a grep on @EntityType and @ContentEntityType these are what I got back, except for the slew of tests. The various test entities have lengthy and descriptive titles, and since the description keys defaults to an empty string, I figured we could leave those as-is. If not, I can update them.

dawehner’s picture

There is also @ConfigEntityType

dawehner’s picture

Status:Needs review» Needs work
mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new19.53 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Here's an updated patch with ConfigEntityTypes having descriptions. Going on vacation until Monday, so won't be able to re-roll until this.

Status:Needs review» Needs work

The last submitted patch, 14: add_a_description_key-2323779-14.patch, failed testing.

Berdir’s picture

Aw, this is going to be a nasty conflict with #1976158: Rename entity storage/list/form/render "controllers" to handlers, which is changing the line right below most descriptions.

mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new17.82 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Reroll against HEAD. PHPStorm made that patch super easy to clean up!

Status:Needs review» Needs work

The last submitted patch, 17: add_a_description_key-2323779-17.patch, failed testing.

mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new17.81 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Typo fail. Filter had @Translations not @Translation. Carried over from pre-vacation patch on re-roll.

Status:Needs review» Needs work

The last submitted patch, 19: add_a_description_key-2323779-19.patch, failed testing.

mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new17.82 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch add_a_description_key-2323779-21.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Ugh. Ok, fixed and ran local tests.

Berdir’s picture

So the hard part here is coming up with good descriptions. Most of those that are added here are just reworded labels, or are worded in ways that we AFAIK try to avoid like "in Drupal". See some examples below.

  1. +++ b/core/modules/block/src/Entity/Block.php
    @@ -21,6 +21,7 @@
      *   label = @Translation("Block"),
    + *   description = @Translation("Block configurations created in Drupal."),

    I don't think the "created in Drupal" part makes much sense, all entities are created in drupal, where else? Not sure what a good label would be.

  2. +++ b/core/modules/block_content/src/Entity/BlockContent.php
    @@ -19,6 +19,7 @@
    + *   description = @Translation("Blocks are custom pieces of content."),

    That's not too useful either. No idea what we can see that's not somehow repeating/rewording the title, maybe just leave it out where we don't have anything useful to add?

  3. +++ b/core/modules/comment/src/Entity/Comment.php
    @@ -22,6 +22,7 @@
    + *   description = @Translation("Comments are responses to node content."),

    Comments are no longer specific to nodes. "other content" maybe?

mglaman’s picture

Berdir, I'll try to come up some more general, better descriptions. I wasn't sure if it'd be "Let's patch the ability to have descriptions, get this functionality." Then, just as there are API documentation patches, there'd be a patch to make a more awesome description, by the folks who are really awesome at doing that stuff.

Berdir’s picture

Yeah, was wondering about that too.

The reason this was started were the views data help strings. So maybe we could limit an initial patch to entity types that have views data integration and a help string and get inspiration from there?

mglaman’s picture

I think that'd be a good move, but we should ping dawehner. My patch in #9 didn't include all entity types then, got re-rolled to do so.

dawehner’s picture

The reason this was started were the views data help strings. So maybe we could limit an initial patch to entity types that have views data integration and a help string and get inspiration from there?

Current examples of these strings in views:

  • 'Taxonomy terms are attached to nodes'
  • 'Comments are responses to node content'
  • 'Users who have created accounts on your site.'

I though agree that they are problematic in the world of Drupal 8.

Status:Needs review» Needs work

The last submitted patch, 21: add_a_description_key-2323779-21.patch, failed testing.

mglaman’s picture

Status:Needs work» Needs review
StatusFileSize
new17.82 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,527 pass(es).
[ View ]

Came back out of my hovel. Here's a whack at some updated descriptions. Patch applied clean, still having issues running simpletest locally.