Part of #2259445: Entity Resource unification

Problem/Motivation

Let's make any API changes now for the parent issue so that it doesn't block beta.

Proposed resolution

In order to get the API changes taken care of up-front for the parent issue, we are going to rename all of the entity-related routes to match what they will be once they are auto-generated. That way, once we start auto-generating them we can remove the static ones or not and there's no affect on module developers.

Remaining tasks

Rename all entity HTML routes to match a common format.

(Entities that have no links are not included below as they do not require any action.)

Config entities

Content entities

The common format is: entity.$entityname.$relationship, where $entityname is the machine name of the entity and $relationship is the relationship as defined in the entity annotation, machine-name-ified. (Convert - to _).

This requires a change to the entity annotation, the defined routes, and any generator calls to those routes.

User interface changes

None.

API changes

Many routes will have different names. Otherwise no change.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Issue summary: View changes
Crell’s picture

Issue summary: View changes
Crell’s picture

Issue summary: View changes
Crell’s picture

Issue summary: View changes

Holy crap we have a lot of entities... I'll make separate issues shortly unless someone wants to beat me to it. :-)

Crell’s picture

Issue summary: View changes
Crell’s picture

Issue summary: View changes

I got bored on the plane back from Design4Drupal so ploughed through these. :-) We'll see if the tests actually pass.

The only one remaining, I think, is EntityTest, which as noted in its issue has a number of subclassed entities that do weird things with their links.

If there's a links-using entity I forgot, please add it to the list (and file a patch!)

bojanz’s picture

As a result of this effort we now have inconsistent route names. Some are entity.$entity_type.$something, some aren't.

image.style_add:
  path: '/admin/config/media/image-styles/add'
  defaults:
    _entity_form: image_style.add
    _title: 'Add image style'
  requirements:
    _permission: 'administer image styles'


entity.image_style.edit_form:
  path: '/admin/config/media/image-styles/manage/{image_style}'
  defaults:
    _entity_form: image_style.edit
    _title: 'Edit style'
  requirements:
    _permission: 'administer image styles'


entity.image_style.delete_form:
  path: '/admin/config/media/image-styles/manage/{image_style}/delete'
  defaults:
    _entity_form: 'image_style.delete'
    _title: 'Delete'
  requirements:
    _permission: 'administer image styles'


image.style_list:
  path: '/admin/config/media/image-styles'
  defaults:
    _entity_list: 'image_style'
    _title: 'Image styles'
  requirements:
    _permission: 'administer image styles'

See how the add and list route names don't match the pattern? Yes, it's because that one is not semantically important, but at a glance it looks inconsistent, and it's bad DX having to think about and explain it.

Crell’s picture

This is a stepping stone toward most of the entity routes being auto-generated anyway, so any visual inconsistency in the routing.yml file will go away in time.

andypost’s picture

Status: Active » Fixed

The last one left #2314889: Standardize entity_test entity route names
So closing the meta

dawehner’s picture

Status: Fixed » Active
andypost’s picture

Status: Active » Fixed
dawehner’s picture

We also forgot about menu_ui_entity_type_build()

andypost’s picture

Status: Fixed » Needs review
FileSize
8.86 KB

@dawehner good catch!
Questions:
1) entity.menu_link_content.add_form is defined in menu link content module but used in menu (same we have for term-add)
2) \Drupal\menu_link_content\Form\MenuLinkContentDeleteForm::getCancelUrl() point to menu ui route but dependency is wrong

Status: Needs review » Needs work

The last submitted patch, 13: 2285413-13.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
1.13 KB
9.99 KB

Fixed the test

EDIT: maybe better leave the name of contextual link as it was?

dawehner’s picture

+1

RTBC once it is green.

  1. +++ b/core/modules/menu_ui/menu_ui.links.action.yml
    @@ -1,12 +1,12 @@
    -menu_ui.menu_add:
    -  route_name: menu_ui.menu_add
    +entity.menu.add_form:
    +  route_name: entity.menu.add_form
       title: 'Add menu'
    

    Its not your fault, so +1

  2. +++ b/core/modules/menu_ui/menu_ui.links.contextual.yml
    @@ -1,4 +1,4 @@
    -menu_ui_edit:
    +entity.menu.edit_form:
       title: 'Edit menu'
    -  route_name: 'menu_ui.menu_edit'
    +  route_name: 'entity.menu.edit_form'
    

    I am fine with that change!

andypost’s picture

Crell’s picture

Status: Needs review » Reviewed & tested by the community

It's green.

dawehner’s picture

There we go.

dawehner’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/menu_ui/menu_ui.links.action.yml
@@ -1,12 +1,12 @@
+entity.menu_link_content.add_form:
+  route_name: entity.menu_link_content.add_form

+++ b/core/modules/menu_ui/menu_ui.module
@@ -74,10 +74,10 @@ function menu_ui_entity_type_build(array &$entity_types) {
+    ->setLinkTemplate('delete-form', 'entity.menu.delete_form')
+    ->setLinkTemplate('edit-form', 'entity.menu.edit_form')
+    ->setLinkTemplate('add-link-form', 'entity.menu_link_content.add_form');
 }

Sorry but this simply can't work.

dawehner’s picture

On top of that I recommend here to fix action module and contact module directly as well. https://www.drupal.org/node/2281645#comment-9102521 contains those changes already as well.

andypost’s picture

makes sense to fix action and contact.
please elaborate #20 - I can;t get what's wrong

andypost’s picture

Status: Needs work » Needs review
FileSize
7.27 KB
14.64 KB

Fixed #20 & #21

Crell’s picture

Status: Needs review » Reviewed & tested by the community

I think we got 'em all this time? :-)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.0.x.

  • Dries committed ebace9e on 8.0.x
    Issue #2285413 by andypost: [Meta] Standardize entity route names.
    

Status: Fixed » Closed (fixed)

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

yukare’s picture

Just a question about this issue: the change record notice is still a draft, any reason to not publish it ?

andypost’s picture

published, also field_ui could update some routes in followup

andypost’s picture