Update: As of October 8, the legacy routing BC layer has been removed, and core includes page controllers for all new routes. The next step is to complete the conversions in the new page controllers to remove dependencies on procedural code and implement best practices. See https://groups.drupal.org/node/315498 for details. And an example of one of the subtasks that was done the new way is #1987778-105: Convert node_show() and node_page_view() to a new style controller
See also: #1945406: [meta] Convert all of confirm_form() to ConfirmFormBase

Remaining tasks

Outstanding child issues:

  1. #2066445: Convert a bunch of AjaxResponse callbacks in system.module's test's ajax_test.module to a new style controller
CommentFileSizeAuthor
#24 callbacks.txt29.45 KBxjm
#24 form_builders.txt15.49 KBxjm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Tagging.

ayelet_Cr’s picture

Issue summary: View changes

Adding more issues.

catch’s picture

Priority: Major » Critical

Very definition of critical.

webchick’s picture

Can someone explain how this is the same or different than #1848648: [meta] Convert menu system to new routing system (also critical)? If this one is purely just about page callback conversions, seems like it ought to be "major" rather than "critical." While I don't think we can ship Drupal 8 with both the router parts of hook_menu() and module.routing.yml files (which I believe is what the other issue is saying), I can see us shipping with a few legacy page callbacks in .module files if we don't get them all done by release. It's very rare for modules to call other modules' page callback functions, so this OO conversion might even be doable in subsequent point releases, not sure.

mitchellshelton’s picture

Title: [META] Convert page callbacks to controllers » Convert forum_page() to a Controller
Component: base system » forum.module
Issue tags: -WSCCI, -FormInterface
xjm’s picture

Title: Convert forum_page() to a Controller » [META] Convert page callbacks to controllers
Issue tags: -WSCCI-conversion +WSCCI, +FormInterface

Oops. :)

xjm’s picture

@webchick, #1848648: [meta] Convert menu system to new routing system is about the menu system itself, whereas this issue is just about simple callbacks and form builders.

hawkeye.twolf’s picture

Toolbar and Translation modules have been checked (one issue each, added to the list).
EDIT:
Additional detail:

  • translation_menu() contained page callback "toolbar_subtrees_jsonp"
  • toolbar_menu() contained page callback "translation_node_overview "
  • no form builder callbacks were found.
  • The corresponding test modules (translation_test and toolbar_test) were also checked for hook_menu() implementations.
hawkeye.twolf’s picture

Component: routing system » forum.module

Add the following views issues to meta:

views: tests/views_test_data/views_test_data.module

views: views.module

views_ui: views_ui/views_ui.module

catch’s picture

Component: routing system » menu system
Issue tags: -WSCCI, -FormInterface +WSCCI-conversion

The other issue is about the part-by-part re-implementation of old menu system features to the new one.

I'm not sure about shipping with some menu callbacks as functions, but it's not technically possible to do that at the moment. The bc-layer in the new router relies entirely on passing everything through to the old system which has to be removed, so it'd have to be added in there before that's even an option. Additionally all those items would still need to move into routing.yml anyway which could be tracked here.

xjm’s picture

Thanks @kgoel, @mitchellshelton, @vijaycs85, @derek.deraps, @cconrad, @shanethehat, and @sidharthap for your help checking for callbacks during core mentoring today!

I'm currently in the process of checking over the new issues that have been filed.

xjm’s picture

Issue tags: +WSCCI, +FormInterface

tag monster--

effulgentsia’s picture

I can see us shipping with a few legacy page callbacks in .module files if we don't get them all done by release.

I think less than 15% of Drupal core's page callback functions are in .module files. The vast majority are in .inc files. If we don't convert all of the ones in .inc files, then in addition to making sure that functions work as controller values in the new routing system, we'll need to also add a 'file' option to the routing.yml files to support loading those .inc files. Ugly as that may be, it may be preferable to delaying a D8 release if all other criticals get solved before this one. But I still think it makes sense to track this as critical in the meantime, with the understanding that the scope of the issue can change as we get closer to release.

For anyone who cares about us not resorting to some ugly and random mix of PSR-0 controllers with old-style include files, please help us get this issue 100% resolved :)

catch’s picture

We could also move the functions back to .module (although again I don't think this is a good idea in general).

webchick’s picture

Right, just trying to work out a reasonable "plan B." Thanks to everyone for working on these patches. Keep 'em coming! :D

xjm’s picture

Some of the issues added today are actually AJAX callbacks, and will probably need to be postponed on #1944472: Add generic content handler for returning dialogs. I'll update those issues.

andypost’s picture

There's a bunch of issues like #1872870: Implement a RoleListController and RoleFormController - what is a suggested way to convert them?
it seems no need to implement 2 issues - one to implement controllers and second to make another controller with routes
For config entities it seems better to make Generic implementation a-la #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController) and implement special controller to inject config storage to it.

Any feedback welcome.

tim.plunkett’s picture

Crell’s picture

I checked last week and verified that it would be possible to change the BC listener to allow for function controllers if needed. However, as others have noted I'd much rather not do so for the time being and try to get everything converted instead. Moving remaining page callbacks to the .module file and leaving them as-is would be plan Z IMO. It's technically possible, I just think it would be a really crappy end result.

Also, zOMG thanks to everyone who's been working on this and the linked issues while I was on vacation. :-)

xjm’s picture

FileSize
15.49 KB
29.45 KB

So last week I believe we added issues for:

  • comment
  • help
  • field and field UI
  • file
  • forum
  • shortcut
  • toolbar
  • tour
  • statistics
  • translation
  • views and views UI
  • xmlrpc

The two attached files contain all the non-form page callbacks in core, and then a rough grep that should get most or all of the form builders. We don't have anywhere near all of these filed. Let's make a table listing all of them so that we can keep track of issues that have different titles or target multiple callbacks, like the forum ones. I'll add a stub table to the summary.

xjm’s picture

I added two stub tables. They won't include rows for callbacks that were removed before this morning's git pull. If you want you can add rows for fixed issues, but at least leave the current rows there, even if they are redundant. Fill in an issue in each row that removes the callback.

cconrad’s picture

Issue summary: View changes

Removed all issues from the list below the tables which existed in one of the tables.
Also removed modules from the list which were marked as having no "hook_menu" or callbacks otherwise.

xjm’s picture

@larowlan re: AJAX callbacks:

no I don't believe those need to be postponed, we have a generic ajax callback (system/ajax), those ones don't use it in D7, so will need their own controllers that return an AjaxResponse, good example is user/autocomplete or the entity reference autocomplete

xjm’s picture

I removed the confirm_form() conversions from the old list at the bottom of the summary if they were already listed on #1945406: [meta] Convert all of confirm_form() to ConfirmFormBase.

Crell’s picture

On the WSCCI meeting this morning we decided that we need to break this list up. Specifically, we need to separate module page callbacks from test module page callbacks; effort needs to be focused on the former, as the latter is easier to do post-1 July. Can someone help with that?

tim.plunkett’s picture

I did the split after the meeting already. We now have 4 tables. (form/nonform, test/nontest)

valthebald’s picture

Is there documentation of advantages of controller based routing vs traditional, hook_menu() based?
I think that if there is justification for tremendous effort like this, it
a) should be documented somewhere, and this issue should contain link to this "somewhere"
b) hook_menu() should be claimed deprecated

xjm’s picture

Thanks @valthebald. This is a critical part of the web services initiative. Check out http://www.garfieldtech.com/blog/refocusing-wscci for an overview. :)

Also, these conversions do not entirely replace hook_menu() implementations, just the page callbacks. Note that the converted page callbacks still have hook_menu() entries for their paths. So we're not at a point where hook_menu() should be deprecated.

Edit: Also see http://drupal.org/node/1800686.

Maybe we could add these resources to the issue summary. :)

valthebald’s picture

@xjm: thanks for clarification!

oenie’s picture

I'm trying my best to help out on the controller business, but i'm running into confusing problems.
The fact is that the controllers that we have now are done in a wide variety of ways.

Their location: Controllers being dumped into the lib\Drupal\ directory , others into another subdir Controller
Implementation:

I'm being redirected to the WSCCI Conversion Guide, but it feels like this is incomplete.
I'm finding myself having to search for possible solutions to specific problems, but it's not always clear what/where to look for.

On top of that, issues are introduced that have impact on code that i find in my searches and use as a reference
(I submit a patch which uses the module_handler service, and then come across the issue
Replace calls to module_handler service with Drupal::moduleHandler())

I would really like to help in trying to get the WSCCI Conversion guide on par as a clear help and with reference to good, useful examples.
The book example is a start but it doesn't touch less trivial cases.
It would also be possible to replace a specific best practice, when something (like the replace issue above) changes.
This way, there would be a single point of information.

Am i too utopian here or is there anyone who can shed some light on best practices ?
I'm thinking specifically of available services to use, where to put files, ...

Crell’s picture

Hi Oenie.

Yes, the Conversion Guide is not yet complete. Mostly it's missing form handling and placeholders. Sorry about that. I want to get placeholders in there soon; forms are probably better written by Tim.

Strictly speaking, Controller classes can be named anything. By convention we've been trying to always put them in the Drupal\$module\Controller directory/namespace. If you see issues where that's not happening, please mark them needs work and/or fix them. :-)

The issue you reference above is a bit misleading. It's specifically about replacing drupal_container()->get('module_handler') with Drupal::moduleHandler(). Injecting the module handler service via create/__construct is still preferred over both, so please continue to do that.

You can also stop by #drupal-wscci in IRC most times and someone (usually Tim Plunkett, Daniel Werner, or myself) can try to help you out. (This week will be a bit crazy due to DrupalCon, of course.)

oenie’s picture

Hi Crell,

Daniel has been of real assistance to me yesterday.
As for the docs, i know you are all soooooo busy, just wanted to show it from my personal perspective.
I tried chipping in last year at Drupal Munich, but that turned out a bit too difficult at the time.
Now i feel like being able to do some helpful work.

Thanks for the clarification on the module_handler & Controller, it sounds like something that we might want to update in the docs ?
Especially the Controller location issue, since with all these ongoing conversions, the less extra work they create in the future the better.

berliner’s picture

@Crell: With reference to #1988802: [META] Rewrite test modules in system to provide better unit testing. and the note at the top of the page: wouldn't it be less confusing to replace the section "Test module form builders" with a link to the meta issue? I had just finished to convert form_test_alter and was about to post a patch, when I realized that there is a the mentioned meta issue and consequently no need for individual route conversions of test modules in the system module.

Crell’s picture

That looks a bit different. This is specifically for moving from hook_menu to the new routing system. That issue seems like a broader attempt to make test modules unit testable... which seems rather redundant since test module only exist to support tests of other modules. :-)

xjm’s picture

We cannot wontfix issues like #1987832: Convert system_test callbacks to a new style controller. This issue is critical; we shouldn't block criticals on cleanups. Let's please reopen them so that work can continue? If someone wants to work on rewriting tests, fine, but that cleanup will take a lot longer than we can afford to wait to get rid of the old routing system.

xjm’s picture

The core maintainers have agreed that these conversions can and should continue to happen after July 1. Declaring routes via hook_menu() in the D7 fashion is deprecated and will almost certainly be removed before release.

xjm’s picture

Issue tags: +Approved API change

tag monster--

tstoeckler’s picture

Opened #2074155: Convert user/%user/edit to a route but couldn't find it in the issue summary yet.

xjm’s picture

Thanks everyone for awesome work on these conversions so far!

In order to expedite the removal of the old routing system, we're changing the conversion process on these issues a bit. See: A faster process for Drupal 8 routing conversions.

What this means for existing issues is that any conversion patch that is committable by September 14 can continue on as it has been. After that, we'll ask you to split your work into two patches -- one that adds the route definition and controller class/method, but does not touch the old page or form callbacks; and then a separate, postponed followup issue with any other work from your conversion to date. (Also, feel free to start using this process on issues assigned to you now if you prefer.)

webchick’s picture

Ok, https://drupal.org/project/issues/search/drupal?status[]=14&issue_tags=W... is now an empty queue, and it's also September 15 in about 7 minutes. So this officially kick off the "just convert routes as fast as possible" methodology.

Thinking about it, though, I wonder if we could do this as one humungous patch for all remaining routes (ideally, this could be scripted, if not, then hacked on in a sandbox among 2-3 people perhaps), because:

a) It is much less effort-intensive this way. ONE patch to create/review/commit, not 50+.

b) We wouldn't need to open another 50+ issues for the "step B" patches, and we could preserve all of the existing comments in those issues that are about taking the "boiling the ocean" approach and let them continue doing that once the routes are moved.

c) Frankly, we don't have another 6 months to figure this out in individual little issues. We need to get this done. If we get it done before Prague, so much the better, because we'll be able to go in there knowing the really hairy menu router problems we still need to figure out. I don't see any way this gets committed before Prague unless it's one patch.

webchick’s picture

./core/includes/menu.inc:      'page callback' => '',
./core/modules/aggregator/aggregator.module:    'page callback' => 'aggregator_page_opml',
./core/modules/block/block.module:      'page callback' => 'block_admin_demo',
./core/modules/block/custom_block/tests/modules/custom_block_test/custom_block_test.module:    'page callback' => 'entity_view',
./core/modules/book/book.module:    'page callback' => 'drupal_get_form',
./core/modules/comment/comment.module:    'page callback' => 'comment_admin',
./core/modules/contact/contact.module:    'page callback' => 'contact_site_page',
./core/modules/contact/contact.module:    'page callback' => 'contact_site_page',
./core/modules/contact/contact.module:    'page callback' => 'contact_personal_page',
./core/modules/content_translation/content_translation.module:        'page callback' => 'content_translation_overview',
./core/modules/content_translation/content_translation.module:        'page callback' => 'content_translation_add_page',
./core/modules/content_translation/content_translation.module:        'page callback' => 'content_translation_edit_page',
./core/modules/content_translation/content_translation.module:        'page callback' => 'drupal_get_form',
./core/modules/dblog/dblog.module:    'page callback' => 'dblog_top',
./core/modules/dblog/dblog.module:    'page callback' => 'dblog_top',
./core/modules/dblog/dblog.module:      'page callback' => 'dblog_top',
./core/modules/field/tests/modules/field_test/field_test.module:    'page callback' => 'drupal_get_form',
./core/modules/file/tests/file_module_test.module:    'page callback' => 'drupal_get_form',
./core/modules/language/language.module:    'page callback' => 'drupal_get_form',
./core/modules/language/language.module:    'page callback' => 'language_content_settings_page',
./core/modules/language/tests/language_elements_test/language_elements_test.module:    'page callback' => 'drupal_get_form',
./core/modules/language/tests/language_elements_test/language_elements_test.module:    'page callback' => 'drupal_get_form',
./core/modules/locale/locale.module:    'page callback' => 'drupal_get_form',
./core/modules/locale/locale.module:    'page callback' => 'drupal_get_form',
./core/modules/locale/locale.module:    'page callback' => 'drupal_get_form',
./core/modules/node/node.module:    'page callback' => 'node_admin_nodes',
./core/modules/node/node.module:    'page callback' => 'node_add_page',
./core/modules/node/node.module:    'page callback' => 'node_add',
./core/modules/node/node.module:    'page callback' => 'node_page_view',
./core/modules/node/node.module:    'page callback' => 'node_revision_overview',
./core/modules/node/node.module:    'page callback' => 'node_show',
./core/modules/path/path.module:    'page callback' => 'path_admin_overview',
./core/modules/path/path.module:    'page callback' => 'path_admin_edit',
./core/modules/path/path.module:    'page callback' => 'path_admin_edit',
./core/modules/search/search.module:    'page callback' => 'search_view',
./core/modules/search/search.module:        'page callback' => 'search_view',
./core/modules/search/search.module:        'page callback' => 'search_view',
./core/modules/search/tests/modules/search_embedded_form/search_embedded_form.module:    'page callback' => 'drupal_get_form',
./core/modules/shortcut/shortcut.module:    'page callback' => 'drupal_get_form',
./core/modules/shortcut/shortcut.module:    'page callback' => 'drupal_get_form',
./core/modules/shortcut/shortcut.module:    'page callback' => 'drupal_get_form',
./core/modules/system/system.api.php: *       'page callback' => 'mymodule_abc_view',
./core/modules/system/system.api.php: *       'page callback' => 'mymodule_abc_view',
./core/modules/system/system.api.php: *     'page callback' => 'mymodule_abc_edit',
./core/modules/system/system.api.php: *     'page callback' => 'mymodule_abc_edit',
./core/modules/system/system.api.php:    'page callback' => 'example_page',
./core/modules/system/system.api.php:    'page callback' => 'example_feed',
./core/modules/system/system.module:    'page callback' => 'file_download',
./core/modules/system/system.module:    'page callback' => 'system_themes_page',
./core/modules/system/system.module:    'page callback' => 'system_theme_default',
./core/modules/system/system.module:      'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/ajax_test/ajax_test.module:    'page callback' => 'ajax_test_render',
./core/modules/system/tests/modules/ajax_test/ajax_test.module:    'page callback' => 'ajax_test_order',
./core/modules/system/tests/modules/ajax_test/ajax_test.module:    'page callback' => 'ajax_test_error',
./core/modules/system/tests/modules/ajax_test/ajax_test.module:    'page callback' => 'ajax_test_dialog',
./core/modules/system/tests/modules/ajax_test/ajax_test.module:    'page callback' => 'ajax_test_dialog_close',
./core/modules/system/tests/modules/batch_test/batch_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/batch_test/batch_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/batch_test/batch_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/database_test/database_test.module:    'page callback' => 'database_test_db_query_temporary',
./core/modules/system/tests/modules/database_test/database_test.module:    'page callback' => 'database_test_even_pager_query',
./core/modules/system/tests/modules/database_test/database_test.module:    'page callback' => 'database_test_odd_pager_query',
./core/modules/system/tests/modules/database_test/database_test.module:    'page callback' => 'database_test_tablesort',
./core/modules/system/tests/modules/database_test/database_test.module:    'page callback' => 'database_test_tablesort_first',
./core/modules/system/tests/modules/database_test/database_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/design_test/design_test.module:      'page callback' => $page_callback,
./core/modules/system/tests/modules/entity_test/entity_test.module:      'page callback' => 'entity_test_add',
./core/modules/system/tests/modules/entity_test/entity_test.module:      'page callback' => 'entity_test_edit',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'NOT_USED',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'form_test_wrapper_callback',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'form_test_double_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/form_test/form_test.module:    'page callback' => 'drupal_get_form',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_theme_page_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_theme_page_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_theme_page_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'test_page_test_page',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_menu_trail_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_menu_trail_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_custom_403_404_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_custom_403_404_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:   'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:   'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:   'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:   'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/menu_test/menu_test.module:    'page callback' => 'menu_test_callback',
./core/modules/system/tests/modules/module_test/module_test.module:    'page callback' => 'module_test_hook_dynamic_loading_invoke',
./core/modules/system/tests/modules/module_test/module_test.module:    'page callback' => 'module_test_hook_dynamic_loading_invoke_all',
./core/modules/system/tests/modules/module_test/module_test.module:    'page callback' => 'module_test_hook_dynamic_loading_invoke_all_during_load',
./core/modules/system/tests/modules/module_test/module_test.module:    'page callback' => 'module_test_class_loading',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_basic_auth_page',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_authorize_init_page',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_set_header',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'variable_get',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_lock_acquire',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_lock_exit',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_main_content_fallback',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_main_content_fallback',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_main_content_fallback',
./core/modules/system/tests/modules/system_test/system_test.module:    'page callback' => 'system_test_page_shutdown_functions',
./core/modules/system/tests/modules/test_page_test/test_page_test.module:    'page callback' => 'test_page_test_page',
./core/modules/taxonomy/taxonomy.module:    'page callback' => 'taxonomy_term_page',
./core/modules/taxonomy/taxonomy.module:    'page callback' => 'taxonomy_term_feed',
./core/modules/tracker/tracker.module:    'page callback' => 'tracker_page',
./core/modules/tracker/tracker.module:    'page callback' => 'tracker_page',
./core/modules/tracker/tracker.module:    'page callback' => 'tracker_page',
./core/modules/translation/translation.module:    'page callback' => 'translation_node_overview',
./core/modules/update/tests/modules/update_test/update_test.module:    'page callback' => 'update_test_mock_page',
./core/modules/update/update.module:    'page callback' => 'update_manual_status',
./core/modules/update/update.module:      'page callback' => 'drupal_get_form',
./core/modules/update/update.module:      'page callback' => 'drupal_get_form',
./core/modules/update/update.module:    'page callback' => 'drupal_get_form',
./core/modules/user/user.module:    'page callback' => 'drupal_get_form',
./core/modules/user/user.module:    'page callback' => 'user_admin_account',
./core/modules/user/user.module:    'page callback' => 'user_view_page',
./core/modules/user/user.module:    'page callback' => 'user_cancel_confirm',
./core/modules/xmlrpc/xmlrpc.module:    'page callback' => 'xmlrpc_server_page',

That's what's left according to grep.

webchick’s picture

Please note that #1981368: Convert all routes to 'module_name.foo_bar' naming convention just got committed. I've updated the existing change notices, and the WSCCI conversion guide accordingly.

effulgentsia’s picture

I wonder if we could do this as one humungous patch for all remaining routes

If I were one of the people trying to roll the patch for this (which I probably won't be, so feel free to ignore), I would split it into 4 patches:
- test module non-forms
- test module forms
- non-test module non-forms
- non-test module forms

And then, if any specific ones required a feature that does not yet exist in HEAD (e.g., a node_access() access checker compatible with the new routing system), then I would split those off into separate patches (possibly lumping all conversions that are blocked on the same missing feature into the same patch).

tim.plunkett’s picture

I'm taking a crack at #2089635: Convert non-test non-form page callbacks to routes and controllers, the other 3 issues need to be opened.

disasm’s picture

webchick’s picture

Also note that #2051097: Change "pattern" to "path" in *.routing.yml files just got committed as well.

tim.plunkett’s picture

#2091691: Convert test non-form page callbacks to routes and controllers is the 3rd of the 4 issues. Remaining is the form callbacks in test modules.

disasm’s picture

webchick’s picture

#2102653: Convert test form callbacks to new form controller was just committed! :)

webchicks-MacBook-Pro:8.x webchick$ grep -r "'page callback'" .
./core/includes/ajax.inc: *   // Menu 'page callback' and #ajax['callback'] functions are supposed to
./core/includes/menu.inc:      $router_item['page callback'] = 'USES_ROUTE';
./core/includes/menu.inc:        if (!isset($item['page callback']) && isset($parent['page callback'])) {
./core/includes/menu.inc:          $item['page callback'] = $parent['page callback'];
./core/includes/menu.inc:    if (!isset($item['access callback']) || empty($item['page callback'])) {
./core/includes/menu.inc:      'page callback' => '',
./core/includes/menu.inc:      'page_callback' => $item['page callback'],
./core/modules/node/node.module:    'page callback' => 'node_admin_nodes',
./core/modules/system/system.api.php: *       'page callback' => 'mymodule_abc_view',
./core/modules/system/system.api.php: *       'page callback' => 'mymodule_abc_view',
./core/modules/system/system.api.php: *     'page callback' => 'mymodule_abc_edit',
./core/modules/system/system.api.php: *     'page callback' => 'mymodule_abc_edit',
./core/modules/system/system.api.php:    'page callback' => 'example_page',
./core/modules/system/system.api.php:    'page callback' => 'example_feed',
./core/modules/system/tests/modules/design_test/design_test.module:      'page callback' => $page_callback,
./core/modules/tracker/tracker.module:    'page callback' => 'tracker_page',
./core/modules/tracker/tracker.module:    'page callback' => 'tracker_page',
./core/modules/tracker/tracker.module:    'page callback' => 'tracker_page',
./core/modules/user/user.module:    'page callback' => 'user_admin_account',

We're getting there now. :)

I still see:

#1972990: Convert tracker_page() to a Controller
#1938884: Replace the fallback user listing with a list controller
#2029569: Convert node_admin_nodes to a new-style Controller

Looks like system.api.php needs some clean-up, and then not sure what's happening in design_test.module.

tim.plunkett’s picture

#2027115: Allow views to override existing routing items replaces the user_admin_account one, and #2021161: Replace the fallback node listing with a list controller replaces the node_admin_nodes one which is blocked on the views one.

design_test module is 100% unused, see #2037569: Remove design_test module

webchick’s picture

Something that keeps coming up when I talk to people about D8 "in the field" is the current mishmash we have where some controllers are in classes with OO methods, and others are in classes with dumb wrappers around procedural code is creating a really confusing environment for learning the new routing system. :\ I think it'd be great to have one more "mega patch" that moves the body of those functions into the right places (and drop the old procedural functions) without also trying to boil the ocean of Dependency Injection blah blah blah.

dawehner’s picture

Something that keeps coming up when I talk to people about D8 "in the field" is the current mishmash we have where some controllers are in classes with OO methods, and others are in classes with dumb wrappers around procedural code is creating a really confusing environment for learning the new routing system. :\ I think it'd be great to have one more "mega patch" that moves the body of those functions into the right places (and drop the old procedural functions) without also trying to boil the ocean of Dependency Injection blah blah blah.

I am not sure whether it is worth to break all the conversion patches again.

vijaycs85’s picture

vijaycs85’s picture

Issue summary: View changes

Cleaning up sub-task list with completed & remaining session

vijaycs85’s picture

Issue summary: View changes

Updating more items into completed list.

webchick’s picture

Note that there was a change notice for this at https://drupal.org/node/2119699 with a before/after table which was being populated by the functions within individual conversion issues such as #1984702: Convert menu.module's page callbacks to Controllers which seemed like sheer, unadulterated madness to me.

I adjusted it to instead show a sample conversion of a D7 page callback => D8 controller that I blatantly stole from http://effulgentsia.drupalgardens.com/content/drupal-8-hello-oop-hello-w.... Someone probably should check my work, though: https://drupal.org/node/2119699/revisions/view/6890847/6891155

xjm’s picture

Priority: Critical » Major

Thanks everyone for your work on resolving these issues!

Now that we've removed the BC layer entirely in #2107533: Remove {menu_router}, the remaining work is to improve these various controllers to remove dependencies on procedural code, use DI where appropriate, etc. This remaining cleanup work doesn't block release, so downgrading to major. We also are probably not going to treat individual controllers as public API, so the changes in these issues can likely continue to go in after beta1, rc1, etc. (at maintainer discretion of course).

What we need most now for these issues is good architectural review for the patches. Many of them have been being rerolled for a long time (and probably need to be rerolled again for PSR-4). I'd suggest any interested reviewers start with the non-test, non-view issues.

jackbravo’s picture

I was checking #2078867: Convert _form_test_* functions to classes which is linked here, but seems like I said on [comment 23](https://drupal.org/node/2078867#comment-8856823) seems like just changing the tests to the new code is not enough because they are not being run with drush test-run or displayed in the Testing module.

xjm’s picture

Issue summary: View changes

Removing the completed list from the summary, since it's a bit overwhelming.

tim.plunkett’s picture

Issue summary: View changes
tim.plunkett’s picture

Mile23’s picture

Issue summary: View changes

Added user_cancel_confirm().

Mile23’s picture

Issue summary: View changes

Check whether database_test_theme_tablesort() still needs an issue
Check whether file_module_test_form(), form_label_test_form() need issues.

No, they don't, since they no longer exist.

aspilicious’s picture

Issue summary: View changes
aspilicious’s picture

Issue summary: View changes
aspilicious’s picture

Issue summary: View changes
cosmicdreams’s picture

removed #1979040: Rewrite XML RPC module to services and plugins because it's assigned to a contrib project now.

aspilicious’s picture

Issue summary: View changes
alexpott’s picture

Status: Active » Fixed

All the child issues have been committed. Time for this one to be set to fixed!

Crell’s picture

OMG WTF BBQ zOMG!

Amazing work everyone!

webchick’s picture

Wow! Fantastic! Great work, all!

jibran’s picture

Can we revert back to old massive issue summary? Yay!!! for the fix.

kim.pepper’s picture

/me weeps gently

vijaycs85’s picture

yay!

jibran’s picture

vijaycs85’s picture

#88: :) there were only 2 issues.
1. xmlrpc is out of core.
2. comment admin page to views isn't really part of this meta(i.e. converting callback to controller).

I think we are good with the status of this meta issue.

Status: Fixed » Closed (fixed)

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