Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Convert this page callback to a new-style Form object, using the instructions on http://drupal.org/node/1800686
Comment | File | Size | Author |
---|---|---|---|
#45 | user-1938888-45.patch | 15.27 KB | tim.plunkett |
#45 | interdiff.txt | 730 bytes | tim.plunkett |
#43 | user-1938888-43.patch | 15.27 KB | amateescu |
#40 | user-1938888-40.patch | 14.01 KB | tim.plunkett |
#40 | interdiff.txt | 1.76 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettTagging.
Comment #2
ayelet_Cr CreditAttribution: ayelet_Cr commentedComment #3
ayelet_Cr CreditAttribution: ayelet_Cr commentedComment #5
ayelet_Cr CreditAttribution: ayelet_Cr commentedComment #7
tim.plunkettThis is blocked on #1959122: Optional params are not possible in routes
Comment #8
Crell CreditAttribution: Crell commented#5: 1938888-convert_user_admin_permissions.patch queued for re-testing.
Comment #10
ayelet_Cr CreditAttribution: ayelet_Cr commentedComment #11
ayelet_Cr CreditAttribution: ayelet_Cr commentedComment #13
Crell CreditAttribution: Crell commentedThis appears to be blocked again on #1945418: New-style placeholders in menu_router table break breadcrumbs, menu tree, etc. *sigh*
Comment #14
tim.plunkettRerolled, revisiting the blocker.
Comment #15
tim.plunkettOkay I'm just merging that issue into this. We have a wonderful example sitting right here.
Comment #17
tim.plunkett#15: user-1938888-15-PASS.patch queued for re-testing.
Comment #19
tim.plunkettRerolled.
Comment #21
tim.plunkettSorry @ayelet_Cr.
Comment #22
tim.plunkettBad copypaste from the reroll after user roles were converted.
Comment #24
dawehnerThe classname and the documentation seem to disagree. I would vote for the classname.
If you just want to get the module info maybe accessing the info in the module handler would be better.
Comment #25
tim.plunkettSince 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.
Comment #27
tim.plunkettTurns out drupal_valid_path() doesn't handle routes correctly, at all.
Comment #29
tim.plunkettAh, needed to assign it to ['access']
Comment #30
dawehnerShould we just use $role_id for now, as there is not even a name property on roles these days.
Comment #31
Crell CreditAttribution: Crell commentedI'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.
This doesn't have a service yet? :-(
Comment #32
tim.plunkettLet's just do this. It's way cleaner looking anyway.
Note, we can't convert the local task until its parent is a route.
Comment #33
dawehnerThis could just use a single load()
Comment #34
tim.plunkettAhh I refactored that a bunch of times, originally the other form was just passing NULL and other weird stuff, and you're totally right :)
Comment #35
tim.plunkettNope, we need to return an array. I almost prefer #32 more...
Interdiff against #32
Comment #36
tim.plunkett#32: user-1938888-32.patch queued for re-testing.
Comment #38
tim.plunkett/facepalm
Comment #39
dawehnerThis class deals with roles, so maybe a better classname like UserPermissionsRoleSpecificForm
Needs docs for the additional parameter
Comment #40
tim.plunkettThanks @dawehner!
Comment #41
dawehnerThank you
Comment #42
alexpottNeeds a reroll...
Comment #43
amateescu CreditAttribution: amateescu commentedRerolled.
Comment #45
tim.plunkettThis was because of #1938938: Convert theme_user_admin_permissions() to table #type.
Comment #46
amateescu CreditAttribution: amateescu commentedUps, sorry for missing that.
Comment #48
tim.plunkettComment #49
tim.plunkettComment #50
alexpottFound #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.
Comment #51
tim.plunkett#2056895: Routes with optional placeholders can fatal when placed in a menu has the fix I removed in #32.