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 Controller, using the instructions on http://drupal.org/node/1800686
Meta issue: #1971384: [META] Convert page callbacks to controllers
Comment | File | Size | Author |
---|---|---|---|
#30 | user-1938884-30.patch | 12.03 KB | dawehner |
#24 | interdiff.txt | 2.71 KB | Xano |
#24 | drupal_1938884_24.patch | 12.17 KB | Xano |
#21 | user-1938884-21.patch | 11.76 KB | tim.plunkett |
#14 | drupal-1938884-14.patch | 11.02 KB | dawehner |
Comments
Comment #1
Les Limit's MINE now.
Comment #2
Les LimSome reference info: currently, user_admin() is the page callback for the following hook_menu() routes:
Comment #3
Crell CreditAttribution: Crell commentedWow, that's ridiculous. :-)
That sounds like that should turn into 3 methods on the same class. I don't know what's up with that $_POST['op'] stuff, though. That doesn't seem right.
Comment #4
Les LimI split off the "create user" part, which now uses the already existing "register" method. No problem there.
The other part (the one checking for $_POST['operation'] == 'cancel') looks like a hacky implementation of a multi-step form. I think the reason why it's not a proper multi-step form in the first place is because there are actually two separately rendered forms on the "admin/people" page - one for setting filters, and separate form for selecting users for bulk operations.
I left that old logic in the new-style Controller as-is. I could probably re-implement it as a multi-step form, if it would be worthwhile.
Comment #5
Crell CreditAttribution: Crell commentedI'm going to ask Tim to weigh in here, since there looks to be some weirdness happening with forms here.
Comment #6
tim.plunkettThanks for assigning to me. A lot, or all, of that code is changed/removed in #1851086: Replace admin/people with a View.
I'll post more tonight
Comment #7
Crell CreditAttribution: Crell commentedAh, should we postpone this issue then, and won't-fix it if that one lands?
Comment #8
tim.plunkettAh. Yeah. Meant to do this.
Comment #9
andypostRelated issue #1946466-9: Convert all confirm_form() in user.module and user.pages.inc to the new form interface and convert route
I think better make account cancellation forms conversion in separate issue
Comment #10
tim.plunkettThis was all changed with the views conversion, here's what's left.
Comment #12
tim.plunkettWhen Views and User register a route with the same fit, it seems to choose by alphabetical order.
Comment #13
dawehnerThis should be rather "Constructs a new UserAdmin object".
Missing @return
What blocks us to use an EQ?
Comment #14
dawehnerHere is the EQ.
Comment #16
dawehnerI'm working on #2027115: Allow views to override existing routing items which really is needed to fix this problems.
Comment #17
tim.plunkettThis blocks removal of MENU_LOCAL_ACTION.
Comment #18
tim.plunkettPostponed on that issue
Comment #19
xjmThanks for your work on this issue! Please see #1971384-43: [META] Convert page callbacks to controllers for an update on the routing system conversion process.
Comment #20
tim.plunkettSee #2021161: Replace the fallback node listing with a list controller for inspiration.
Comment #21
tim.plunkettDidn't realize it was assigned to me still :)
Comment #22
jibranIt is all good RTBC. Just a minor issue.
Please can we make it mutli line.
Comment #23
XanoNuff said :D
Is this even possible, or did you just add it for good practice?
Comment #24
XanoComment #25
dawehnerXano wanted to open a follow up for user_role_names() so this looks fine now.
Comment #26
dawehnerAdded a follow up for format_interval/plural: #2111349: Move format_plural to the string translation service and format_interval to the date service.
Comment #27
XanoSee #2111341: Introduce EntityManager::options() to build options lists for a generic replacement for
user_role_names()
Comment #28
Xano#24: drupal_1938884_24.patch queued for re-testing.
Comment #29
alexpottPatch no longer applies.
Comment #30
dawehnerRerolled.
Comment #31
jibranback to RTBC.
Comment #32
jibran:/
Comment #32.0
jibranUpdated issue summary.
Comment #33
dawehnerThis patch still applies.
Comment #34
Xano30: user-1938884-30.patch queued for re-testing.
Comment #35
Xano30: user-1938884-30.patch queued for re-testing.
Comment #36
webchickNot for this issue, but that's a *shocking* amount of boilerplate code to accomplish this. The diffstat between reams and reams of custom code and re-using a nice common base library is almost equal. :( I would've expected EntityListController to make a lot of this transparent to implementers.
Could we get a follow-up to discuss how we might make the DX of EntityListControllers better? (If one does not exist already?)
At any rate though, yay for using said common base libraries instead of custom code!
Committed and pushed to 8.x. Thanks!