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
#9 drupal8.system-module.1987594-9.patch3.02 KBdisasm
PASSED: [[SimpleTest]]: [MySQL] 59,020 pass(es).
[ View ]
#9 interdiff.txt1.41 KBdisasm
#4 system-theme_test-convert_to_controller-1987594-4.patch2.34 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 58,898 pass(es).
[ View ]

Comments

Status:Active» Closed (won't fix)

Need to rewrite the whole module to make test sync with current test implementation. For more details, please refer: #1988802: [META] Rewrite test modules in system to provide better unit testing.

Status:Closed (won't fix)» Active

Assigned:Unassigned» mparker17

I will help!

Assigned:mparker17» Unassigned
Status:Active» Needs review
StatusFileSize
new2.34 KB
PASSED: [[SimpleTest]]: [MySQL] 58,898 pass(es).
[ View ]

Try this...

Status:Needs review» Needs work

This is nearly perfect!

  1. +++ b/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    @@ -31,4 +31,10 @@ function functionTemplateOverridden() {
    +  function themeTestSuggestion() {
    +    return theme(array('theme_test__suggestion', 'theme_test'), array());

    Let's make the method public and return a render array.

  2. +++ b/core/modules/system/tests/modules/theme_test/theme_test.module
    @@ -47,8 +47,7 @@ function theme_test_system_theme_info() {
       $items['theme-test/suggestion'] = array(
         'title' => 'Suggestion',
    -    'page callback' => '_theme_test_suggestion',
    -    'access callback' => TRUE,
    +    'route_name' => 'theme_test_suggestion',
         'theme callback' => '_theme_custom_theme',
         'type' => MENU_CALLBACK,
       );

    Let's just drop the full menu item, as MENU_CALLBACK is not needed.

@dawehner, just two quick questions before I work on this...

1. I left the hook_menu item in because it had a 'theme callback'. Will returning a render array allow me to get around that?
2. I'm not entirely sure how to return a render array in this circumstance because of the unusual call to theme(). Could I trouble you to provide me with an example?

Thanks!

Ping...

Status:Needs work» Needs review

StatusFileSize
new1.41 KB
new3.02 KB
PASSED: [[SimpleTest]]: [MySQL] 59,020 pass(es).
[ View ]

Doing some git searches, I can't find any previous conversions that have:
a) removed a theme_callback
b) converted a theme() function with an array() as first parameter to a render array.

As such, my suggestion is to let this slide and get it committed where we can deal with refactoring a and b in a follow-up. The tricky thing with a is no one has addressed what theme_callback because in new routes that I know of. The tricky thing in b is the theme() function when it's an array uses the first available hook, and ignores the other parameter, so in the case of the following:

<?php
theme
(array('foo', 'bar'), array());
?>

If the foo theme exists in the registry, it is used. If the foo theme does not exist, but the bar one does it is used. I am an un-aware of a #theme equivalent to this logic, and probably the whole thing should be refactored anyways.

Attached patch makes the methods public.

+++ w/core/modules/system/tests/modules/theme_test/theme_test.module
@@ -47,8 +47,7 @@ function theme_test_system_theme_info() {
   $items['theme-test/suggestion'] = array(
     'title' => 'Suggestion',
+++ w/core/modules/system/tests/modules/theme_test/theme_test.routing.yml
@@ -4,3 +4,10 @@ function_template_override:
+theme_test_suggestion:
+  pattern: 'theme-test/suggestion'
+  defaults:
+    _content: '\Drupal\theme_test\ThemeTestController::themeTestSuggestion'

See title on the other review.

Status:Needs review» Closed (duplicate)