Part of #1971384: [META] Convert page callbacks to controllers

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#17 1987812-system_admin_index-controller-7.patch5.33 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,459 pass(es).
[ View ]
#16 1987812-16.patch5.33 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 55,441 pass(es).
[ View ]
#13 1987812-system_admin_index-controller-6.patch5.39 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,624 pass(es).
[ View ]
#10 1987812-system_admin_index-controller-5.patch5.4 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,871 pass(es).
[ View ]
#8 1987812-system_admin_index-controller-4.patch5.39 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,852 pass(es).
[ View ]
#6 1987812-system_admin_index-controller-3.patch5.39 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,638 pass(es).
[ View ]
#3 1987812-system_admin_index-controller-2.patch5.32 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,625 pass(es).
[ View ]
#1 1987812-system_admin_index-controller.patch3.64 KBoenie
PASSED: [[SimpleTest]]: [MySQL] 55,883 pass(es).
[ View ]

Comments

oenie’s picture

Assigned:Unassigned» oenie
Status:Active» Needs review
StatusFileSize
new3.64 KB
PASSED: [[SimpleTest]]: [MySQL] 55,883 pass(es).
[ View ]

First try at a patch, with help from the code in core/update/lib/Drupal/update/Controller/UpdateController.php

dawehner’s picture

This looks really good already. Here are just some small nitpicks needed.

+++ b/core/modules/system/lib/Drupal/system/AdminController.phpundefined
@@ -0,0 +1,78 @@
+ * Definition of Drupal\system\AdminController.

Should be \Contains...

+++ b/core/modules/system/lib/Drupal/system/AdminController.phpundefined
@@ -0,0 +1,78 @@
+  /**

new empty line.

+++ b/core/modules/system/lib/Drupal/system/AdminController.phpundefined
@@ -0,0 +1,78 @@
+   * Constructs admin data.

Constructs a AdminController object.

+++ b/core/modules/system/lib/Drupal/system/AdminController.phpundefined
@@ -0,0 +1,78 @@
+   * Prints a listing of admin tasks, organized by module.

Needs @return

+++ b/core/modules/system/lib/Drupal/system/AdminController.phpundefined
@@ -0,0 +1,78 @@
+    return theme('system_admin_index', array('menu_items' => $menu_items));

Let's return a render array instead of directly render it.

oenie’s picture

StatusFileSize
new5.32 KB
PASSED: [[SimpleTest]]: [MySQL] 55,625 pass(es).
[ View ]

Couldn't resist to do a quick new patch before bed.
Previous patch forgot to remove the page callback function from system.admin.inc

dawehner’s picture

Let's move this new file into a "Controller" directory, just to make it more consistent.

dawehner’s picture

Let's move this new file into a "Controller" directory, just to make it more consistent.

+++ b/core/modules/system/lib/Drupal/system/AdminController.phpundefined
@@ -0,0 +1,87 @@
+   *  a render array containing the listing

The start with an "A" and end with a dot :)

oenie’s picture

StatusFileSize
new5.39 KB
PASSED: [[SimpleTest]]: [MySQL] 55,638 pass(es).
[ View ]

Updated + REALLY added the removal of the old page callback function from system.admin.inc

dawehner’s picture

Status:Needs review» Needs work

I'm sorry but the patch does not apply anymore.

oenie’s picture

Status:Needs work» Needs review
StatusFileSize
new5.39 KB
PASSED: [[SimpleTest]]: [MySQL] 55,852 pass(es).
[ View ]

Reroll of patch 3

Status:Needs review» Needs work

The last submitted patch, 1987812-system_admin_index-controller-4.patch, failed testing.

oenie’s picture

Status:Needs work» Needs review
StatusFileSize
new5.4 KB
PASSED: [[SimpleTest]]: [MySQL] 55,871 pass(es).
[ View ]

Although it seems the testbot fluked, a new patch to correct the location of the ControllerInterface.

amateescu’s picture

Status:Needs review» Reviewed & tested by the community

Looks good to me.

alexpott’s picture

Status:Reviewed & tested by the community» Needs work

Needs a reroll

curl https://drupal.org/files/1987812-system_admin_index-controller-5.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  5533  100  5533    0     0  15209      0 --:--:-- --:--:-- --:--:-- 15674
error: patch failed: core/modules/system/system.routing.yml:87
error: core/modules/system/system.routing.yml: patch does not apply
oenie’s picture

Status:Needs work» Needs review
StatusFileSize
new5.39 KB
PASSED: [[SimpleTest]]: [MySQL] 55,624 pass(es).
[ View ]

Rerolled

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Great. The page also looks fine on manual testing.

alexpott’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Needs a reroll...

curl https://drupal.org/files/1987812-system_admin_index-controller-6.patch | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  5523  100  5523    0     0   4273      0  0:00:01  0:00:01 --:--:--  7493
error: patch failed: core/modules/system/system.routing.yml:101
error: core/modules/system/system.routing.yml: patch does not apply
jibran’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new5.33 KB
PASSED: [[SimpleTest]]: [MySQL] 55,441 pass(es).
[ View ]

Reroll
Conflict

++<<<<<<< HEAD
+system_status:
+  pattern: '/admin/reports/status'
+  defaults:
+    _controller: 'Drupal\system\Controller\SystemInfoController::status'
+  requirements:
+    _permission: 'administer site configuration'
+
+system_php:
+  pattern: '/admin/reports/status/php'
+  defaults:
+    _controller: 'Drupal\system\Controller\SystemInfoController::php'
+  requirements:
+    _permission: 'administer site configuration'
+
++=======
+ system_admin_index:
+   pattern: 'admin/index'
+   defaults:
+     _content: 'Drupal\system\Controller\AdminController::index'
+   requirements:
+     _permission: 'access administration pages'
++>>>>>>> 6

Resolved

+system_status:
+  pattern: '/admin/reports/status'
+  defaults:
+    _controller: 'Drupal\system\Controller\SystemInfoController::status'
+  requirements:
+    _permission: 'administer site configuration'
+
+system_php:
+  pattern: '/admin/reports/status/php'
+  defaults:
+    _controller: 'Drupal\system\Controller\SystemInfoController::php'
+  requirements:
+    _permission: 'administer site configuration'
+
+ system_admin_index:
+   pattern: 'admin/index'
+   defaults:
+     _content: 'Drupal\system\Controller\AdminController::index'
+   requirements:
+     _permission: 'access administration pages'
oenie’s picture

Issue tags:+Needs reroll
StatusFileSize
new5.33 KB
PASSED: [[SimpleTest]]: [MySQL] 55,459 pass(es).
[ View ]

Reroll

oenie’s picture

Issue tags:-Needs reroll

@jibran: thanks for the effort, but it's confusing when the issue is assigned and then someone else starts fixing it...

jibran’s picture

Status:Needs review» Reviewed & tested by the community

@oenie I do apologies for that. :) In my defiance I just want to reroll it ASAP and last patch was posted on "May 27, 2013".
Let's RTBC because no code change.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

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

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