Since we're stuck with a Views-based node listing, we might as well make it a list controller.

Files: 
CommentFileSizeAuthor
#43 vdc-2021161.patch13.05 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,884 pass(es).
[ View ]
#39 vdc-2021161-39.patch12.99 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch vdc-2021161-39.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#39 interdiff.txt1.84 KBdawehner
#36 vdc-2021161-36.patch12.97 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,872 pass(es), 4 fail(s), and 2 exception(s).
[ View ]
#36 interdiff.txt2.57 KBdawehner
#34 node-2021161-34.patch12.07 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,111 pass(es).
[ View ]
#34 interdiff.txt1.59 KBdawehner
#32 node-2021161-32.patch11.96 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,944 pass(es), 1 fail(s), and 10 exception(s).
[ View ]
#32 interdiff.txt3.01 KBdawehner
#29 node-2021161-29.patch12.56 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,385 pass(es).
[ View ]
#15 drupal_2021161_15.patch10.16 KBXano
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#6 2021161-convert-node_admin_nodes-6.patch12.73 KBAjitS
FAILED: [[SimpleTest]]: [MySQL] 56,555 pass(es), 24 fail(s), and 0 exception(s).
[ View ]
#1 node-2021161-1.patch10.34 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch node-2021161-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

tim.plunkett’s picture

Status:Active» Needs review
StatusFileSize
new10.34 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch node-2021161-1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Status:Needs review» Needs work

The last submitted patch, node-2021161-1.patch, failed testing.

dawehner’s picture

So no pager, node access and status filter anymore?

+++ b/core/modules/node/lib/Drupal/node/NodeListController.phpundefined
@@ -0,0 +1,96 @@
+    $row['type'] = check_plain(node_get_type_label($entity));

String::checkPlain()

tim.plunkett’s picture

I'm assuming we need those three things back, yes.

AjitS’s picture

Status:Needs work» Needs review
StatusFileSize
new12.73 KB
FAILED: [[SimpleTest]]: [MySQL] 56,555 pass(es), 24 fail(s), and 0 exception(s).
[ View ]

Attaching patch from the dupe mentioned in #5.

Status:Needs review» Needs work

The last submitted patch, 2021161-convert-node_admin_nodes-6.patch, failed testing.

AjitS’s picture

Status:Needs work» Needs review

Tested locally. Seems to clear the tests with 50+ nodes.
#6: 2021161-convert-node_admin_nodes-6.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 2021161-convert-node_admin_nodes-6.patch, failed testing.

Xano’s picture

Can you please make those associative arrays multiline? I know it's not in the coding standards, but it makes them so much more readable, especially because they exceed 80 characters and run off my screen. There is really no need to put them on one line.

Xano’s picture

Assigned:Unassigned» Xano

Oh, whatever.

Xano’s picture

Would it be a good idea to make the node entity list controller implement FormInterface and in its render() method call drupal_get_form() on itself?

tim.plunkett’s picture

We should be removing ALL form elements. This is just a listing.

Xano’s picture

Status:Needs work» Needs review

#1: node-2021161-1.patch queued for re-testing.

Xano’s picture

Assigned:Xano» Unassigned
StatusFileSize
new10.16 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

This is a simple re-roll of #1.

Status:Needs review» Needs work

The last submitted patch, drupal_2021161_15.patch, failed testing.

Xano’s picture

Status:Needs work» Needs review

#15: drupal_2021161_15.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, drupal_2021161_15.patch, failed testing.

Xano’s picture

Status:Needs work» Needs review

#15: drupal_2021161_15.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, drupal_2021161_15.patch, failed testing.

dawehner’s picture

Issue tags:+VDC

Additional from the small problems from below, this basically needs #2027115: Allow views to override existing routing items to work.

  1. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,96 @@
    +    $row['type'] = check_plain(node_get_type_label($entity));

    Could be String::checkPlain and maybe even the entity_load for the node type could be used.

  2. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,96 @@
    +    $row['status'] = $entity->status ? t('published') : t('not published');

    t() could also be $this->t() all over the place.

neclimdul’s picture

Anyone working on this? Would like to start converting the local task.

tim.plunkett’s picture

Status:Needs work» Fixed
tim.plunkett’s picture

Status:Fixed» Postponed

NOT THE STATUS I CLICKED I SWEAR

tim.plunkett’s picture

Reopened #2029569: Convert node_admin_nodes to a new-style Controller as the phase 1 version of this.

andypost’s picture

Status:Postponed» Needs work
tim.plunkett’s picture

Status:Needs work» Postponed
webchick’s picture

Status:Postponed» Needs work

Now it is. :)

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new12.56 KB
PASSED: [[SimpleTest]]: [MySQL] 58,385 pass(es).
[ View ]

Just another rerole.

Crell’s picture

Status:Needs review» Needs work
+++ b/core/modules/node/lib/Drupal/node/NodeListController.php
@@ -0,0 +1,163 @@
+   * Constructs a new DateFormatListController object.

Looks like a copy paste oops on the class name.

jibran’s picture

Thanks @dawehner nice work. Some more minor issues.

  1. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,163 @@
    +      $header['language_name'] = array('data' => $this->t('Language'), 'class' => array(RESPONSIVE_PRIORITY_LOW));

    Please can we convert this to multiline as well.

  2. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,163 @@
    +      '#href' => 'node/' . $entity->id(),
    ...
    +        'href' => 'node/' . $entity->id() . '/translations',

    $node->uri()

  3. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,163 @@
    +  protected function t($string, array $args = array(), array $options = array()) {

    We should move this to base class.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new3.01 KB
new11.96 KB
FAILED: [[SimpleTest]]: [MySQL] 58,944 pass(es), 1 fail(s), and 10 exception(s).
[ View ]

We should move this to base class.

We do have indeed a t() function already in the base.

jibran’s picture

+++ b/core/modules/node/lib/Drupal/node/NodeListController.php
@@ -113,7 +104,7 @@ public function buildRow(EntityInterface $entity) {
+      '#href' => $entity->uri(),

It should be

+++ b/core/modules/node/lib/Drupal/node/NodeListController.php
@@ -113,7 +104,7 @@ public function buildRow(EntityInterface $entity) {
+       $uri = $entity->uri();
+      '#href' => $uri['path'],
+      '#options' => $uri['options'],
dawehner’s picture

StatusFileSize
new1.59 KB
new12.07 KB
PASSED: [[SimpleTest]]: [MySQL] 59,111 pass(es).
[ View ]

You are so right!

tim.plunkett’s picture

  1. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,148 @@
    +    $header['operations'] = $this->t('Operations');
    +    return $header;

    return $header + parent::buildHeader();

  2. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,148 @@
    +    $row['operations']['data'] = $this->buildOperations($entity);
    +    return $row;

    return $row + parent::buildRow($entity);

  3. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,148 @@
    +    if ($entity->isTranslatable()) {
    +      $uri = $entity->uri();
    +      $operations['translate'] = array(
    +        'title' => $this->t('Translate'),
    +        'href' => $uri['path'] . '/translations',
    +        'options' => $uri['options'],
    +      );
    +    }

    This can be added in by content_translation_entity_operation_alter() now (which doesn't exist but should), we want that for admin/people as well.

dawehner’s picture

StatusFileSize
new2.57 KB
new12.97 KB
FAILED: [[SimpleTest]]: [MySQL] 58,872 pass(es), 4 fail(s), and 2 exception(s).
[ View ]

This can be added in by content_translation_entity_operation_alter() now (which doesn't exist but should), we want that for admin/people as well.

This is pretty cool.

Status:Needs review» Needs work

The last submitted patch, vdc-2021161-36.patch, failed testing.

jibran’s picture

It is RTBC for me but it is not green so some more minor issues.

  1. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,139 @@
    +    parent::__construct($entity_type, $entity_info, $storage, $module_handler);
    +
    +    $this->dateService = $date_service;

    I think we can remove blank line.

  2. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,139 @@
    +      $header['language_name'] = array('data' => $this->t('Language'), 'class' => array(RESPONSIVE_PRIORITY_LOW));

    Mutliline please.

  3. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,139 @@
    +    $row['type'] = String::checkPlain(node_get_type_label($entity));

    NodeManager *sigh*. I think we have a issue about it.

  4. +++ b/core/modules/node/lib/Drupal/node/NodeListController.php
    @@ -0,0 +1,139 @@
    +      $operations[$key]['query'] = $destination;
    +
    +    }

    We can also remove this blank line.

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new1.84 KB
new12.99 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch vdc-2021161-39.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I think we can remove blank line.

I do prefer to separate the parent call.

dawehner’s picture

39: vdc-2021161-39.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 39: vdc-2021161-39.patch, failed testing.

Status:Needs review» Needs work

The last submitted patch, 39: vdc-2021161-39.patch, failed testing.

dawehner’s picture

Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new13.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,884 pass(es).
[ View ]

This is a rerole.

tim.plunkett’s picture

Status:Needs review» Reviewed & tested by the community

Thanks!

Xano’s picture

43: vdc-2021161.patch queued for re-testing.

webchick’s picture

Status:Reviewed & tested by the community» Fixed

Copy/paste comments form #1938884-36: Replace the fallback user listing with a list controller.

Committed and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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