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).

Files: 
CommentFileSizeAuthor
#21 entity-list-2008356-21.patch12.17 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,554 pass(es).
[ View ]
#18 entity-list-2008356-18.patch12.16 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 56,016 pass(es).
[ View ]
#18 interdiff.txt413 bytestim.plunkett
#16 entity-list-2008356-16.patch12.16 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 57,207 pass(es), 22 fail(s), and 5 exception(s).
[ View ]
#16 interdiff.txt1.01 KBtim.plunkett
#13 entity_route_enhancer-2008356-13.patch11.15 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,843 pass(es).
[ View ]
#13 interdiff.txt2.59 KBdawehner
#8 entity_route_enhancer-2008356-8.patch8.38 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,975 pass(es).
[ View ]
#8 interdiff.txt1.6 KBdawehner
#5 interdiff.txt1.69 KBdawehner
#5 drupal-2008356-5.patch7.46 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,300 pass(es).
[ View ]
#4 drupal-2008356-4.patch7.21 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#4 entity_list-2008356-1.patch9.52 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#4 interdiff.txt3.45 KBdawehner
#1 entity_list-2008356-1.patch9.52 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new9.52 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

See attached.

Status:Needs review» Needs work

+++ b/core/lib/Drupal/Core/Entity/Enhancer/EntityRouteEnhancer.phpundefined
@@ -37,8 +37,13 @@ public function __construct(ContentNegotiation $negotiation) {
+      elseif (!empty($defaults['_entity_list'])) {
+        $defaults['_controller'] = '\Drupal\Core\Entity\HtmlEntityListController::content';
+      }

Can't this just set _content and let the HtmlPageController do what's needed? Everything else causes problems with anything else then full rendered pages.

+++ b/core/lib/Drupal/Core/Entity/HtmlEntityListController.phpundefined
@@ -0,0 +1,42 @@
+    $content = $this->container->get('plugin.manager.entity')->getListController($_entity_list)->render();

Inject the entity manager!

I'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.

Status:Needs work» Needs review
StatusFileSize
new3.45 KB
new9.52 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new7.21 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Let's just reuse the existing class, which is from my perspective already in a better place and shape in general.

StatusFileSize
new7.46 KB
PASSED: [[SimpleTest]]: [MySQL] 57,300 pass(es).
[ View ]
new1.69 KB

This time it should actually work.

+++ b/core/lib/Drupal/Core/Entity/Enhancer/EntityRouteEnhancer.phpundefined
@@ -37,8 +37,14 @@ public function __construct(ContentNegotiation $negotiation) {
+        $defaults['_controller'] = 'controller.page:content';
+        $defaults['_content'] = '\Drupal\Core\Entity\Controller\EntityListController::listing';

That's tricky! I like it.

I think I wrote too much of this to RTBC myself.

    * Provides the listing page for any entity type.
    *
+   * @param string $_entity_list
+   *   The entity type to render.
-  public function listing($entity_type) {
+  public function listing($_entity_list) {

I think $entity_type is a better variable name for an entity type than $_entity_list -- that variable I would expect to contain a list.

StatusFileSize
new1.6 KB
new8.38 KB
PASSED: [[SimpleTest]]: [MySQL] 55,975 pass(es).
[ View ]

Cood point, let's avoid this uglyness if possible.

I 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?

But yeah, it's ambiguous to me too, how about '"_entity_list_type'?:P

There 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.

+1 to _entity_list also I think it's ok to name argument as 'entity_list'

This patch is looking great, just some questions about things more than feedback.

+++ b/core/lib/Drupal/Core/Entity/Enhancer/EntityRouteEnhancer.phpundefined
@@ -0,0 +1,54 @@
+   * Content negotiation library.

I guess this is future proofing it, atm it seems like we can't really call that a library :)

+++ b/core/lib/Drupal/Core/Entity/Enhancer/EntityRouteEnhancer.phpundefined
@@ -0,0 +1,54 @@
+    if (empty($defaults['_controller']) && $this->negotiation->getContentType($request) === 'html') {

So having a controller could always override this. Not saying it's wrong, is that what we want? could be.

+++ b/core/modules/views_ui/views_ui.routing.ymlundefined
@@ -1,7 +1,7 @@
+    _entity_list: 'view'

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.

StatusFileSize
new2.59 KB
new11.15 KB
PASSED: [[SimpleTest]]: [MySQL] 55,843 pass(es).
[ View ]

So having a controller could always override this. Not saying it's wrong, is that what we want? could be.

That'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.

Status:Needs review» Reviewed & tested by the community

one more DX win:)
one could say providers could be used for the test, but its not that consistent..so i am gonna rtbc this

+1 to rtbc

StatusFileSize
new1.01 KB
new12.16 KB
FAILED: [[SimpleTest]]: [MySQL] 57,207 pass(es), 22 fail(s), and 5 exception(s).
[ View ]

Adding two more converted listings.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, entity-list-2008356-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new413 bytes
new12.16 KB
PASSED: [[SimpleTest]]: [MySQL] 56,016 pass(es).
[ View ]

/me facepalms

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

Needs a reroll

curl https://drupal.org/files/entity-list-2008356-18.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 12448  100 12448    0     0   9241      0  0:00:01  0:00:01 --:--:-- 10871
error: core/modules/views_ui/lib/Drupal/views_ui/Routing/ViewsUIController.php: does not exist in index
error: patch failed: core/modules/views_ui/views_ui.routing.yml:1
error: core/modules/views_ui/views_ui.routing.yml: patch does not apply

Status:Needs work» Needs review
StatusFileSize
new12.17 KB
PASSED: [[SimpleTest]]: [MySQL] 57,554 pass(es).
[ View ]

I was trying to delete
_controller: '\Drupal\views_ui\Routing\ViewsUIController::listing'

Now I'm deleting
_controller: '\Drupal\views_ui\Controller\ViewsUIController::listing'

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Fixed

Committed 5bf89a4 and pushed to 8.x. Thanks!

Title:Provide a _entity_list route default to replace Controller\EntityListController and mimic _entity_formChange notice: Provide a _entity_list route default to replace Controller\EntityListController and mimic _entity_form
Priority:Normal» Critical
Status:Fixed» Active

Needs a change notice.

Status:Active» Needs review

Title:Change notice: Provide a _entity_list route default to replace Controller\EntityListController and mimic _entity_formProvide a _entity_list route default to replace Controller\EntityListController and mimic _entity_form
Priority:Critical» Normal
Status:Needs review» Reviewed & tested by the community

Awesome!

Status:Reviewed & tested by the community» Fixed

Perfect

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