Convert this page callback to a new-style Form object, using the instructions on http://drupal.org/node/1800686

Files: 
CommentFileSizeAuthor
#45 user-1938888-45.patch15.27 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,355 pass(es).
[ View ]
#45 interdiff.txt730 bytestim.plunkett
#43 user-1938888-43.patch15.27 KBamateescu
FAILED: [[SimpleTest]]: [MySQL] 57,117 pass(es), 47 fail(s), and 14 exception(s).
[ View ]
#40 user-1938888-40.patch14.01 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,284 pass(es).
[ View ]
#40 interdiff.txt1.76 KBtim.plunkett
#38 user-1938888-38.patch13.91 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,165 pass(es).
[ View ]
#38 interdiff.txt500 bytestim.plunkett
#35 user-1938888-35.patch13.91 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 56,954 pass(es), 11 fail(s), and 8 exception(s).
[ View ]
#35 interdiff.txt644 bytestim.plunkett
#34 user-1938888-34.patch13.88 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#32 user-1938888-32.patch13.9 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 56,591 pass(es), 11 fail(s), and 510 exception(s).
[ View ]
#32 interdiff.txt5.87 KBtim.plunkett
#29 user-1938888-29.patch12.8 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 56,541 pass(es).
[ View ]
#29 interdiff.txt574 bytestim.plunkett
#27 interdiff.txt1.79 KBtim.plunkett
#27 user-1938888-27.patch12.79 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 56,810 pass(es), 26 fail(s), and 20 exception(s).
[ View ]
#25 user-1938888-25-1.patch10.99 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 56,754 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#25 interdiff1.txt640 bytestim.plunkett
#25 user-1938888-25-2.patch11 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 56,770 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#25 interdiff2.txt1.68 KBtim.plunkett
#22 user-1938888-22.patch10.99 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 58,124 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#22 interdiff.txt724 bytestim.plunkett
#19 user-1938888-19.patch10.99 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 58,341 pass(es), 21 fail(s), and 0 exception(s).
[ View ]
#15 user-1938888-15-FAIL.patch10.61 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 51,727 pass(es), 302 fail(s), and 196 exception(s).
[ View ]
#15 user-1938888-15-PASS.patch11.35 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-1938888-15-PASS.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#14 user-1938888-14.patch10.36 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-1938888-14.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#10 1938888-convert_user_admin_permissions.patch9.78 KBayelet_Cr
FAILED: [[SimpleTest]]: [MySQL] 54,419 pass(es), 3 fail(s), and 52 exception(s).
[ View ]
#5 1938888-convert_user_admin_permissions.patch9.83 KBayelet_Cr
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1938888-convert_user_admin_permissions_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 1938888-convert_user_admin_permissions.patch9.5 KBayelet_Cr
FAILED: [[SimpleTest]]: [MySQL] 53,277 pass(es), 16 fail(s), and 0 exception(s).
[ View ]

Comments

Issue tags:+FormInterface

Tagging.

Assigned:Unassigned» ayelet_Cr

Status:Active» Needs review
StatusFileSize
new9.5 KB
FAILED: [[SimpleTest]]: [MySQL] 53,277 pass(es), 16 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1938888-convert_user_admin_permissions.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.83 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1938888-convert_user_admin_permissions_0.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, 1938888-convert_user_admin_permissions.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+WSCCI, +FormInterface, +#SprintWeekend, +WSCCI-conversion

The last submitted patch, 1938888-convert_user_admin_permissions.patch, failed testing.

StatusFileSize
new9.78 KB
FAILED: [[SimpleTest]]: [MySQL] 54,419 pass(es), 3 fail(s), and 52 exception(s).
[ View ]

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1938888-convert_user_admin_permissions.patch, failed testing.

Status:Needs work» Postponed

StatusFileSize
new10.36 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-1938888-14.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled, revisiting the blocker.

Status:Postponed» Needs review
StatusFileSize
new11.35 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user-1938888-15-PASS.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new10.61 KB
FAILED: [[SimpleTest]]: [MySQL] 51,727 pass(es), 302 fail(s), and 196 exception(s).
[ View ]

Okay I'm just merging that issue into this. We have a wonderful example sitting right here.

Status:Needs review» Needs work
Issue tags:-WSCCI, -FormInterface, -#SprintWeekend, -WSCCI-conversion

The last submitted patch, user-1938888-15-PASS.patch, failed testing.

Status:Needs work» Needs review

#15: user-1938888-15-PASS.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+WSCCI, +FormInterface, +#SprintWeekend, +WSCCI-conversion

The last submitted patch, user-1938888-15-PASS.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new10.99 KB
FAILED: [[SimpleTest]]: [MySQL] 58,341 pass(es), 21 fail(s), and 0 exception(s).
[ View ]

Rerolled.

Status:Needs review» Needs work

The last submitted patch, user-1938888-19.patch, failed testing.

Assigned:ayelet_Cr» tim.plunkett

Sorry @ayelet_Cr.

Status:Needs work» Needs review
StatusFileSize
new724 bytes
new10.99 KB
FAILED: [[SimpleTest]]: [MySQL] 58,124 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Bad copypaste from the reroll after user roles were converted.

Status:Needs review» Needs work

The last submitted patch, user-1938888-22.patch, failed testing.

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsForm.phpundefined
@@ -0,0 +1,158 @@
+class UserPermissionsForm implements FormInterface, ControllerInterface {
...
+   * Constructs a new UserAdminPermissionForm.

The classname and the documentation seem to disagree. I would vote for the classname.

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsForm.phpundefined
@@ -0,0 +1,158 @@
+    $module_info = system_get_info('module');

If you just want to get the module info maybe accessing the info in the module handler would be better.

Status:Needs work» Needs review
StatusFileSize
new1.68 KB
new11 KB
FAILED: [[SimpleTest]]: [MySQL] 56,770 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new640 bytes
new10.99 KB
FAILED: [[SimpleTest]]: [MySQL] 56,754 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Since we need the module's human readable names, we can't use the module handler.
We can get closer by doing this (see patch 2).
If you think that's worse, I've included patch 1, which is just the UserPermissionsForm doc fix.

Status:Needs review» Needs work

The last submitted patch, user-1938888-25-2.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Stalking Crell
StatusFileSize
new12.79 KB
FAILED: [[SimpleTest]]: [MySQL] 56,810 pass(es), 26 fail(s), and 20 exception(s).
[ View ]
new1.79 KB

Turns out drupal_valid_path() doesn't handle routes correctly, at all.

Status:Needs review» Needs work

The last submitted patch, user-1938888-27.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new574 bytes
new12.8 KB
PASSED: [[SimpleTest]]: [MySQL] 56,541 pass(es).
[ View ]

Ah, needed to assign it to ['access']

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsForm.phpundefined
@@ -0,0 +1,158 @@
+  public function buildForm(array $form, array &$form_state, $role_name = NULL) {

Should we just use $role_id for now, as there is not even a name property on roles these days.

+++ b/core/includes/menu.inc
@@ -2788,11 +2788,11 @@ function menu_router_build($save = FALSE) {
function _menu_router_translate_route($route_name) {
-  $route = Drupal::service('router.route_provider')->getRouteByName($route_name);
-  $path = trim($route->getPattern(), '/');
-
-  // Translate placeholders, e.g. {foo} -> %.
-  return preg_replace('/{' . DRUPAL_PHP_FUNCTION_PATTERN . '}/', '%', $path);
+  $outline = Drupal::service('router.route_provider')
+    ->getRouteByName($route_name)
+    ->compile()
+    ->getPatternOutline();
+  return trim($outline, '/');
}

I'm surprised this passes; the pattern outline is not quite the same as the pattern. It also drops optional ending placeholders, I think. (Or it did. May have changed?) I don't know if this will have any knock-on effects.

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsForm.php
@@ -0,0 +1,158 @@
+    $role_names = user_role_names();

This doesn't have a service yet? :-(

StatusFileSize
new5.87 KB
new13.9 KB
FAILED: [[SimpleTest]]: [MySQL] 56,591 pass(es), 11 fail(s), and 510 exception(s).
[ View ]

Let's just do this. It's way cleaner looking anyway.

Note, we can't convert the local task until its parent is a route.

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsSpecificForm.phpundefined
@@ -0,0 +1,37 @@
+    return $this->roleStorage->loadMultiple(array($this->roleId));

This could just use a single load()

StatusFileSize
new13.88 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Ahh I refactored that a bunch of times, originally the other form was just passing NULL and other weird stuff, and you're totally right :)

StatusFileSize
new644 bytes
new13.91 KB
FAILED: [[SimpleTest]]: [MySQL] 56,954 pass(es), 11 fail(s), and 8 exception(s).
[ View ]

Nope, we need to return an array. I almost prefer #32 more...
Interdiff against #32

Status:Needs review» Needs work
Issue tags:+WSCCI, +Stalking Crell, +FormInterface, +#SprintWeekend, +WSCCI-conversion

The last submitted patch, user-1938888-35.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new500 bytes
new13.91 KB
PASSED: [[SimpleTest]]: [MySQL] 57,165 pass(es).
[ View ]

/facepalm

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsSpecificForm.phpundefined
@@ -0,0 +1,37 @@
+class UserPermissionsSpecificForm extends UserPermissionsForm {

This class deals with roles, so maybe a better classname like UserPermissionsRoleSpecificForm

+++ b/core/modules/user/lib/Drupal/user/Form/UserPermissionsSpecificForm.phpundefined
@@ -0,0 +1,37 @@
+  /**
+   * {@inheritdoc}
...
+  public function buildForm(array $form, array &$form_state, $role_id = NULL) {

Needs docs for the additional parameter

StatusFileSize
new1.76 KB
new14.01 KB
PASSED: [[SimpleTest]]: [MySQL] 57,284 pass(es).
[ View ]

Thanks @dawehner!

Status:Needs review» Reviewed & tested by the community

Thank you

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

Needs a reroll...

git ac https://drupal.org/files/user-1938888-40.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 14343  100 14343    0     0  10695      0  0:00:01  0:00:01 --:--:-- 12483
error: patch failed: core/modules/user/user.admin.inc:97
error: core/modules/user/user.admin.inc: patch does not apply

Status:Needs work» Needs review
StatusFileSize
new15.27 KB
FAILED: [[SimpleTest]]: [MySQL] 57,117 pass(es), 47 fail(s), and 14 exception(s).
[ View ]

Rerolled.

Status:Needs review» Needs work

The last submitted patch, user-1938888-43.patch, failed testing.

Status:Needs work» Needs review
Issue tags:-Needs reroll, -Stalking Crell
StatusFileSize
new730 bytes
new15.27 KB
PASSED: [[SimpleTest]]: [MySQL] 57,355 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Ups, sorry for missing that.

Assigned:tim.plunkett» Unassigned
Issue tags:+FormInterface, +WSCCI-conversion

Status:Reviewed & tested by the community» Fixed

Found #2049681: User permissions table header is no longer sticky whilst doing manual testing.

Committed d3d54f1 and pushed to 8.x. Thanks!

I'm ignoring the injection of string translation until #2018411: Figure out a nice DX when working with injected translation lands and will continue this policy until that's resolved. We are going to have to second passes of all conversions due to drupal_set_message() and watchdog() anyway.

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