In addition to _content
, _controller
, and _form
, the entity system provides _entity_form
as an easy way to call EntityFormControllers from routes.
Recently in #1983844: Add EntityListController and convert picture.module callbacks to routes/controllers to provide a use-case (which hasn't even had a change notice yet!) we added \Drupal\Core\Entity\Controller\EntityListController to handle this for entity lists, but that was partly confusing since it called through to something usually extending \Drupal\Core\Entity\EntityListController, and it was more verbose than _entity_form.
Here's what it looks like in HEAD now:
user_role_list:
pattern: '/admin/people/roles'
defaults:
_content: '\Drupal\Core\Entity\Controller\EntityListController::listing'
entity_type: 'user_role'
requirements:
_permission: 'administer permissions'
And here it is with this proposal
user_role_list:
pattern: '/admin/people/roles'
defaults:
_entity_list: 'user_role'
requirements:
_permission: 'administer permissions'
Eventually, once our page callbacks are converted, we'll likely want one for _entity_render as well (think node/%node, user/%user).
Comment | File | Size | Author |
---|---|---|---|
#21 | entity-list-2008356-21.patch | 12.17 KB | tim.plunkett |
#18 | entity-list-2008356-18.patch | 12.16 KB | tim.plunkett |
#18 | interdiff.txt | 413 bytes | tim.plunkett |
#16 | entity-list-2008356-16.patch | 12.16 KB | tim.plunkett |
#16 | interdiff.txt | 1.01 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettSee attached.
Comment #2
dawehnerCan't this just set _content and let the HtmlPageController do what's needed? Everything else causes problems with anything else then full rendered pages.
Inject the entity manager!
Comment #3
tim.plunkettI'm just following what HtmlEntityFormController does now, but that seems sane.
HtmlEntityListController mimics HtmlFormController and others by being container aware, I don't know that it can get anything else injected.
Comment #4
dawehnerLet's just reuse the existing class, which is from my perspective already in a better place and shape in general.
Comment #5
dawehnerThis time it should actually work.
Comment #6
tim.plunkettThat's tricky! I like it.
I think I wrote too much of this to RTBC myself.
Comment #7
chx CreditAttribution: chx commentedI think
$entity_type
is a better variable name for an entity type than$_entity_list
-- that variable I would expect to contain a list.Comment #8
dawehnerCood point, let's avoid this uglyness if possible.
Comment #9
smiletrl CreditAttribution: smiletrl commentedI think
+ * @param string $_entity_list
is trying to keep variable name consistency, because it's using this "_entity_list" in yml file
+ _entity_list: 'custom_block_type'
to match new routing system.
But yeah, it's ambiguous to me too, how about '"_entity_list_type'?:P
Here's another question, but it's non-related to this specifc issue, sorry for bugging you here:(
Will every route_enhancer.** class provided by core.services.yml will be invoked by the new routing system? For instance, this issue provides a _entity_list: for defaults at routing yml file, routing system will call every route_enhancer.** class based on this specific routing file to get a final "defaults" status for this route. Is this understanding right?
It seem there's no conflicts between all these route_enhancer classes. They check each defaults property respectively. For example, ContentControllerEnhancer is to check '_content' property for 'defaults', EntityFormEnhancer is to check '_entity_form' property. And the new route_enhancer EntityRouteEnhancer in this issue is to check defaults property '_entity_list'
So, other modules could provide its own routing-enhancer class to manipulate the routing system too?
Comment #10
dawehnerThere is currently already _entity_form so let's be consistent with that.
Yes every route enhancer is fired on every request and you can implement your own if needed. In general I would rather avoid that and rely more on the configuration in yml files as it's easier to understand. These entity related functionality though is worth as there are so many implementations in core.
Comment #11
andypost+1 to
_entity_list
also I think it's ok to name argument as 'entity_list'Comment #12
damiankloip CreditAttribution: damiankloip commentedThis patch is looking great, just some questions about things more than feedback.
I guess this is future proofing it, atm it seems like we can't really call that a library :)
So having a controller could always override this. Not saying it's wrong, is that what we want? could be.
Stuff like this makes it all worth it!
Also, seems like this could do with a unit test? maybe I missed it somewhere else. I guess there are lots of tests that test the actual listing pages. I don't know.
Comment #13
dawehnerThat's the common pattern we use in every route enhancer. It solves the problem to not let different route enhancers get into conflict.
Here is a unit test.
Comment #14
ParisLiakos CreditAttribution: ParisLiakos commentedone more DX win:)
one could say providers could be used for the test, but its not that consistent..so i am gonna rtbc this
Comment #15
andypost+1 to rtbc
Comment #16
tim.plunkettAdding two more converted listings.
Comment #18
tim.plunkett/me facepalms
Comment #19
ParisLiakos CreditAttribution: ParisLiakos commentedComment #20
alexpottNeeds a reroll
Comment #21
tim.plunkettI was trying to delete
_controller: '\Drupal\views_ui\Routing\ViewsUIController::listing'
Now I'm deleting
_controller: '\Drupal\views_ui\Controller\ViewsUIController::listing'
Comment #22
ParisLiakos CreditAttribution: ParisLiakos commentedconflict with #2015377: Move Controllers in 'Routing' to 'Controller' namespace
Comment #23
alexpottCommitted 5bf89a4 and pushed to 8.x. Thanks!
Comment #24
tim.plunkettNeeds a change notice.
Comment #25
dawehnerHere is one change: https://drupal.org/node/1799642/revisions/view/2730271/2739547
Comment #26
andypostAwesome!
Comment #27
dawehnerPerfect