Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The major parts of #1781372: Add an API for listing (configuration) entities that met resistance was providing a base menu item for each List controller, and providing a hookMenu() method for the addition of even more.
Pros:
- Less duplication: Currently if you change the path for that listing, you have to change it in at least two places, plus once more for each operation
- Ease of adding a list controller: Adding the controller itself and it's definition in hook_entity_info() is it, no need to touch hook_menu()
- The controller knows where it exists: Often times an operation might need to redirect or reload, and it shouldn't have to hardcode the destination
Cons:
- Discoverability of menu items: With the exception of field_ui_menu(), menu items are all in your own .module, in hook_menu() or hook_menu_alter()
- localize.drupal.org: It currently parses the hook_menu() definitions directly
Proposed resolution
- Allow entities, or their controllers, to specify their path.
- Allow controllers to provide menu items
Remaining tasks
@todo
User interface changes
N/A
API changes
N/A
Comments
Comment #1
tim.plunkettActually, because this is about more than just list controllers, unpostponing and unassigning. Discuss!
Comment #2
sunI'm tempted to rewrite/revamp the issue summary, because the actual problem is different from my perspective:
For me, it's primarily about derived router paths of entities.
Every List controller needs at minimum
Every Form controller needs at minimum
$form_state['redirect']
after submission)Every View/Render controller needs at minimum
To clarify what I mean with derived :
Given what @EclipseGc did with Contextual Administration, I'm pretty sure he'll say that this information needs to be injected into each controller, so as to be able to view/edit/list entities on completely different URIs and not having them hard-coded within the controllers.
Comment #3
tim.plunkettI definitely agree that the path should be injected.
Comment #4
tstoecklerIt was mentioned in some other issue, and I wanted to mention that here, because it wasn't clear to me at first:
We already have a precedent for this in core with Field UI. More specifically from the API doc on hook_entity_info:
That's obviously not the nicest way to do this, but...
Comment #5
catchFor a while I was thinking about trying to reduce the amount of data we store in the router. So just store information that has to be queried, plus a string pointing to a class name, and then move the title callback, description callback and similar in there. The idea would be to make building the router itself a lot cheaper since we'd not need to instantiate any of those classes until they were used, and you'd also be able to have router items that inherit from each other etc. I've forgotten how much of that is done by the new router patch (and deliberately not checking that now), but if we do that, then how much stays in hook_router_info() or hook_menu_item_info()? If we do this we also need to be careful about hook_menu_alter() - it'll be as easy to completely replace a router item but much harder to change just one thing (or for two modules to change two different things).
Comment #6
joachim CreditAttribution: joachim commentedIf I understand #4 correctly, we could have a 'base path' property in hook_entity_info, so for example in for nodes that would be just 'node/'.
That would be an enormous help to contrib modules, as currently there's no way to do something like add a tab to every entity.
Comment #7
xjmFollowup issue: #1798540: Add link to add a new entity in an empty entity list controller table
Comment #7.0
xjmUpdated issue summary.
Comment #7.1
plachAdded related issue
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commented@joachim, I suggested something kind of similar in #1803818: Make it possible to dynamically provide additional routes for entities. We will need have that kind of information in order to provide serialization routes for arbitrary entities.
Comment #9
Crell CreditAttribution: Crell commentedA couple of things:
1) We really need to rename entity controllers. "Controller" is "callable that the Kernel calls", basically formerly "page callback". Like "bundle" that's an inherited term from Symfony so we cannot really change it. For the remainder of this comment I will refer to Entity Storage Drivers (ESD) (what are currently called NodeController, UserController, etc.), to avoid confusion.
2) This is one of the reasons I've been calling for a Generator (#1705488: Implement a generator for Drupal paths) for a long time. Once we are able to refer to routes by name to generate URLs, a lot of the "injection problem" that EclipseGc is concerned about is taken care of. (Not all, but a lot.)
3) It sounds like what we're talking about, then, is each ESD exposing standardly-named routes that point to some standard Controller. $entity_type/{$id}/{?operation} seems like a reasonable default for that. And then we would need to piggyback on to that for each supported mime type. Then what an ESD would be specifying is: 1) Whether or not to generate routes. 2) A pattern (if not the default above). 3) A class to use for the controllers for those routes (if not the default above), possibly split by mime?
4) I haven't kept up on List "controllers", so I don't know what to offer there. Are there docs I can see first?
4) The discussion in #1801570: DX: Replace hook_route_info() with YAML files and RouteBuildEvent is highly relevant, as at the moment the leading contender is to use YAML, not a hook, to define routes. That would definitely impact dynamically generated routes of this sort.
Comment #10
effulgentsia CreditAttribution: effulgentsia commented#1807042: Reorganizie entity storage/list/form/render controller annotation keys
Comment #11
YesCT CreditAttribution: YesCT commented#1810350: Remove menu_* entity keys and replace them with entity links is postponed on this issue. (updated issue summary)
Comment #11.0
YesCT CreditAttribution: YesCT commentedAdded the exact comment
Comment #12
Crell CreditAttribution: Crell commentedRetitling for clarity.
Comment #13
tim.plunkettComment #14
sun#1188388: Entity translation UI in core just landed, which introduced multiple entity plugin properties to define various paths of an entity.
That shouldn't have been introduced there, so this issue becomes even more major than it was.
Also, we have some very closely related issues:
#1332058: Need a way to distinguish "public/final" URIs for entities from admin and redirected ones
#1803586: Give all entities their own URI
#1823574-9: [Meta] Improve the Views Bulk Operations (VBOs) that are in core
I fear we need a meta issue to consolidate all of those discussions and change proposals. :-/
Comment #15
Lukas von BlarerI am also coming from a similar issue: #1783964: Allow entity types to provide menu items
Comment #16
Lukas von BlarerI created a patch implementing a small part of this to demo the usage: http://drupal.org/node/1832778#comment-6698138 What do you think of it?
Comment #17
andypostThere's a contrib project with a some UI that uses entity_uri() http://drupal.org/project/entity_path
Comment #18
sunSorry, this issue focuses too much on an aspect that I only consider an artifact of a larger problem space, and various other issues are running into that identical problem space again and again, so I created a dedicated issue for the actual, larger task that we need to address for D8:
#1839516: Introduce entity operation providers
Comment #19
andypostSuppose first we need to make a widget for menu_link edit. Currently each module introduces own form for links.
Related issues:
#916388: Convert menu links into entities
#1882552: Deprecate menu_list_system_menus() and menu_ui_get_menus()
#1882218: Remove static menu link creation for menus in menu_enable() and elsewhere
Comment #20
tim.plunkettI had no idea that #1188388: Entity translation UI in core snuck in part of this.
EntityManager documents the following keys:
Which is used by CustomBlock, Term, and various test entity types.
Um, what!?
Comment #21
plachYep, it was the only way to make that stuff work at the time. We need to get rid of those/refactor them in a sane way, as part of #1839516: Introduce entity operation providers I guess.
Comment #22
sunI agree with @tim.plunkett in #20.
Either this, or #1839516: Introduce entity operation providers, or perhaps even both issues need to be bumped to critical.
There is no way we can release with these E.T. router path declarations in entity plugin definitions. They break encapsulation and separation of concerns on multiple fronts.
P.S.: It's about time to send translation.module to /dev/null, and ET to take over its namespace. Until then, all overly ambiguous "E.T." pun is intended :P
Comment #23
tim.plunkettIt's actually translation_entity.module, who knew?
See #1968970: Standardize module-provided entity info documentation and clean-up the @EntityType annotation for cleaning that mess up.
Comment #24
Crell CreditAttribution: Crell commentedRelated: #1970360: Entities should define URI templates and standard links
Comment #25
PanchoRe #14:
If this is a release blocker, then it's critical per definition.
If not, then it might be just major plus "Revisit before release".
Comment #26
catchComment #26.0
catchUpdated issue summary. adding that #1810350: Remove menu_* entity keys and replace them with entity links is postponed on this issue.
Comment #27
tim.plunkettI think #2089757: Auto generate routing entries for entities based on an 'admin_path' and 'admin_permission' annotation would supersede this issue. Can we mark this as a dupe?
Comment #28
catchYep.