Motivation

Follow-up of #2004408: Allow modules to alter the result of EntityListController::getOperations

We just need a way to tell field_ui which entity type is used as bundle for a given entity (or the other way round, might be easier and that's where the route thing already is) and then we can drop the module_exists() checks in the list controllers of those and get those links automagically.

Proposed resolution

Introduce a bundle_of annotation

and

+ * Implements hook_entity_operation_alter().
+ */
+function field_ui_entity_operation_alter(array &$operations, EntityInterface $entity) {

will add the field operations items generically so they dont have to be added specifically for each entity.

ui change (see #25)

taxonomy list is different

before there is no operation in the dropbutton for managing fields etc.
taxonomy-list-nomanagefields.png

after, the operations are there:

taxonomy-after.png

API changes

No.

Related Issues

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
2.91 KB

Something like this.

amateescu’s picture

Status: Needs review » Needs work
+++ b/core/modules/contact/lib/Drupal/contact/Plugin/Core/Entity/Category.php
@@ -32,6 +32,7 @@
+ *   bundle_of = "contact_message",

This needs to be added and documented in the Annotation class.

+++ b/core/modules/field_ui/field_ui.module
@@ -289,6 +290,30 @@ function field_ui_form_node_type_form_alter(&$form, $form_state) {
+    $operations['manage-fields'] = array(
+      'title' => t('Manage fields'),
+      'href' => $uri['path'] . '/fields',
+      'options' => $uri['options'],
+      'weight' => 11,
+    );
+    $operations['manage-display'] = array(
+      'title' => t('Manage display'),
+      'href' => $uri['path'] . '/display',
+      'options' => $uri['options'],
+      'weight' => 12,

We also need to check Field UI permissions here, otherwise looks great!

And we also need to implement it for all the other entity types as well :)

Berdir’s picture

Status: Needs work » Needs review
FileSize
10.77 KB

Re-roll.

- Documented the bundle_of key. Right now, only field_ui.module uses it but I can think of multiple uses for this, automatically generate bundle information for example, automatically trigger entity bundle hooks in the storage controller and so on, so documenting it makes sense I think.
- Defined it for node types, contact categories, vocabularies and custom block types.
- Shuffled the weights around a bit. I think the form display links that got added accidently moved the node type edit operation between manage form display and manage display, that seemed weird, so I moved it below.
- That and the added operations for vocabularies are I think the only visual changes here. The translate link weight of the config_translation module might need an update after this though.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Looks awesome now! Thanks, Sascha :)

amateescu’s picture

On another look..

+++ b/core/modules/field_ui/field_ui.module
@@ -294,6 +295,44 @@ function field_ui_form_node_type_form_alter(&$form, $form_state) {
+  global $user;

Should we replace this with Request stuff?

amateescu’s picture

Status: Reviewed & tested by the community » Needs work

Sorry for spamming, I'll answer myself: Yes, we should: https://drupal.org/node/2032447

Berdir’s picture

Status: Needs work » Needs review
FileSize
29.69 KB
30.04 KB
10.75 KB

I replaced it with nothing, because I'm not actually using it :) No idea what I was thinking.

I also forgot the upload the screenshots that I made.

node_type_operations.png

vocabulary_operations.png

Berdir’s picture

And the interdiff, just to make it complete ;)

amateescu’s picture

LOL, even better. Should we be worried about the test fail in #3? looks related..

Berdir’s picture

Wow, that's interesting test ;)

This should fix it.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Yay!

YesCT’s picture

Issue tags: +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

catch’s picture

+++ b/core/lib/Drupal/Core/Entity/Annotation/EntityType.phpundefined
@@ -155,6 +155,16 @@ class EntityType extends Plugin {
+   * The name of the entity type this entity type provides bundles for.

:) is there something more descriptive that could be used for this?

Also is it needed on the base class? That could be just on config entities no?

+++ b/core/lib/Drupal/Core/Entity/Annotation/EntityType.phpundefined
@@ -155,6 +155,16 @@ class EntityType extends Plugin {
+   */
+  public $bundle_of;
+
+  /**

Should be camelCase.

catch’s picture

Status: Reviewed & tested by the community » Needs work
catch’s picture

+++ b/core/modules/node/lib/Drupal/node/Plugin/Core/Entity/NodeType.phpundefined
@@ -31,6 +31,7 @@
  *   config_prefix = "node.type",

Node types are also bundles of comments (kind of) - what happens with those?

Berdir’s picture

@catch: This is the annotation class, there's no difference between config and content entities there. And we don't camel case annotation properties.

Not sure what to put on the description, any suggestions?

Node types are no direct match to comment bundles, and with the critical comment as a field issue, they won't be the only thing. I don't think we can do anything there.

Berdir’s picture

Status: Needs work » Needs review
Issue tags: -RTBC July 1

Status: Needs review » Needs work
Issue tags: +RTBC July 1

The last submitted patch, field_ui_operations-alter-2021063-10.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
11.39 KB

#2027131: Block type operations should be listed in the same order as Content type operations got in which conflicted with this and reminded me of this issue.

I don't have a better idea for a description, suggestions welcome, it IMHO describes what it is..

Re-roll, same approach for block type list controller now as for node type.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

I thought about that description but also couldn't find anything better :(

All the other feedback has been addressed, so let's try again?

apaderno’s picture

+   * The name of the entity type this entity type provides bundles for.
+   *
+   * This can be used by other modules to act accordingly, e.g. field_ui uses
+   * it to add operation links to manage fields and displays.

I would rather use "The name of the entity type for which [the] bundles are provided."

I would also avoid using e.g. to introduce another clause; e.g. should be used when introducing a list that is exauhistive, as in the following sentence.

The buffet provided excellent variety, e.g., vegetarian and non-vegetarian soups, Italian and French breads, and numerous sweets.

It should be replaced by for example, but in that case the sentence is a comma-splice. The correct sentence should be the following:

This can be used by other modules to act accordingly; for example, field_ui uses it to add operation links to manage fields and displays.

Probably, field_ui should also be replaced by the name of the module; we don't normally use the short name of a module when referring to it. We say the Node module not node.

apaderno’s picture

Status: Reviewed & tested by the community » Needs work
apaderno’s picture

Status: Needs work » Needs review
FileSize
11.41 KB
YesCT’s picture

I'll do a review. :)

YesCT’s picture

looked at fieldable things:

$ ag " *   fieldable = TRUE," core

Some like tests, we dont need to look at.

$ ag " *   fieldable = TRUE," core | grep -v tests
core/modules/aggregator/lib/Drupal/aggregator/Plugin/Core/Entity/Feed.php:34: *   fieldable = TRUE,
core/modules/aggregator/lib/Drupal/aggregator/Plugin/Core/Entity/Item.php:28: *   fieldable = TRUE,
core/modules/block/custom_block/lib/Drupal/custom_block/Plugin/Core/Entity/CustomBlock.php:40: *   fieldable = TRUE,
core/modules/comment/lib/Drupal/comment/Plugin/Core/Entity/Comment.php:36: *   fieldable = TRUE,
core/modules/contact/lib/Drupal/contact/Plugin/Core/Entity/Message.php:33: *   fieldable = TRUE,
core/modules/node/lib/Drupal/node/Plugin/Core/Entity/Node.php:40: *   fieldable = TRUE,
core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Term.php:37: *   fieldable = TRUE,
core/modules/user/lib/Drupal/user/Plugin/Core/Entity/User.php:38: *   fieldable = TRUE,

Some dont have lists which would have operations drop downs like: comment, message, user
After enabling aggregator, I dont see a way to add fields in the ui to feeds or feed items.

custom block type and contact type lists are the same before and after the patch

content type list is the same before and after

before-operations-contenttype.png

after-content-types.png

taxonomy list is different

before there is no operation in the dropbutton for managing fields etc.
taxonomy-list-nomanagefields.png

after, the operations are there:

taxonomy-after.png

Looking at the patch...

@@ -32,6 +32,7 @@
+ *   bundle_of = "custom_block",

@@ -32,6 +32,7 @@
+ *   bundle_of = "contact_message",

@@ -31,6 +31,7 @@
+ *   bundle_of = "node",

@@ -31,6 +31,7 @@
+ *   bundle_of = "taxonomy_term",

agrees with those here.

--

I looked for an test entity type of fieldable entities and didn't find one to unhide to see if the operations showed. so... I cant think of anything else to look at here.

--

@@ -6,6 +6,7 @@
 use Drupal\entity\EntityViewModeInterface;
+use Drupal\Core\Entity\EntityInterface;

only change I made is to have the use alphabetical. If we dont want that, patch in #23 is fine.

YesCT’s picture

Status: Needs review » Reviewed & tested by the community

only changes are in comment wording and the use reorder. was rtbc'd before.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll
git ac https://drupal.org/files/field_ui_operations-alter-2021063-25.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 11726  100 11726    0     0   8924      0  0:00:01  0:00:01 --:--:-- 14956
error: core/modules/block/custom_block/lib/Drupal/custom_block/Plugin/Core/Entity/CustomBlockType.php: does not exist in index
error: core/modules/contact/lib/Drupal/contact/Plugin/Core/Entity/Category.php: does not exist in index
error: core/modules/node/lib/Drupal/node/Plugin/Core/Entity/NodeType.php: does not exist in index
error: core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Vocabulary.php: does not exist in index
Berdir’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
FileSize
11.26 KB

Automerged by git rebase, only conflicted because we moved the entity classes, so back to RTBC.

alexpott’s picture

Title: Use hook_entity_operation_alter() for manage fields and manage display links » Change notice: Use hook_entity_operation_alter() for manage fields and manage display links
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed 703e9e4 and pushed to 8.x. Thanks!

Berdir’s picture

Title: Change notice: Use hook_entity_operation_alter() for manage fields and manage display links » Use hook_entity_operation_alter() for manage fields and manage display links
Priority: Critical » Normal
Status: Active » Fixed
Issue tags: -Needs change record

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

added resolution description, ui changes and api changes section.