Updated: Comment #22

Note to committers: give mparker credit (he worked on two of the conversions):
Issue #1987870 by disasm, mparker | vijaycs85: Convert theme_test() callbacks to a new style controller.

Problem/Motivation

Convert all theme_test module callbacks to a new Controller

Proposed resolution

Remaining tasks

Reviews

#1987594: Convert _theme_test_suggestion() to a new style controller
#1987592: Convert _theme_test_alter() to a new style controller

Original report by @vijaycs85

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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vijaycs85’s picture

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.

ayelet_Cr’s picture

Status: Closed (won't fix) » Active
disasm’s picture

Title: Convert theme_test_hook_init_page_callback() to a new style controller » Convert theme_test callbacks to a new style controller
Assigned: Unassigned » disasm
disasm’s picture

Status: Active » Needs review
FileSize
3.44 KB

first pass. still needs to have drupal_add_css calls converted to a render array.

Status: Needs review » Needs work

The last submitted patch, drupal8.theme_test.1987870-4.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
+++ b/core/modules/system/tests/modules/theme_test/theme_test.module
@@ -64,16 +64,6 @@ function theme_test_menu() {
-    'page callback' => 'theme_test_template_test_page_callback',
...
-    'page callback' => 'theme_test_info_stylesheets',

You should also remove the previous code.

disasm’s picture

converting drupal_add_css to render array. removing code I forgot to delete.

Status: Needs review » Needs work

The last submitted patch, drupal8.theme_test.1987870-7.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
881 bytes
4.62 KB

Apparently arrays can't be instantiated as objects with the new keyword. In other words, I messed up ;-) Here's a fix.

disasm’s picture

renaming render_array -> build.

Status: Needs review » Needs work

The last submitted patch, drupal8.theme_test.1987870-10.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
2.03 KB
4.95 KB

actually returning the render array now instead of return some other variable that isn't set.

Status: Needs review » Needs work

The last submitted patch, drupal8.theme_test.1987870-12.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
7.33 KB

trying a combination of all three remaining theme_test callbacks.

Status: Needs review » Needs work

The last submitted patch, drupal8.theme_test.1987870-14.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
3.63 KB
7.58 KB

try again.

Status: Needs review » Needs work

The last submitted patch, drupal8.system-module.1987870-16.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
FileSize
711 bytes
7.56 KB

reverting testTemplate to return theme() to pass tests.

disasm’s picture

adding the last remaining callback in that module for good measure.

Status: Needs review » Needs work
Issue tags: -WSCCI-conversion

The last submitted patch, drupal8.system-module.1987870-19.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI-conversion
disasm’s picture

Issue summary: View changes

update

xjm’s picture

Thanks 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.

dawehner’s picture

  1. +++ w/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    @@ -7,28 +7,88 @@
    +      return theme('theme_test_template_test');
    

    I am sorry ... too many whitespaces.

  2. +++ w/core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php
    @@ -7,28 +7,88 @@
    +   * Tests themed output generated in a request listener.
    +   * @return string
    

    Let's put an empty line between there.

dawehner’s picture

Status: Needs review » Needs work

.

disasm’s picture

Status: Needs work » Needs review
FileSize
879 bytes
8.13 KB

changes requested in #23 completed.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you very much!

Status: Reviewed & tested by the community » Needs work
Issue tags: -WSCCI-conversion

The last submitted patch, drupal8.theme_test.1987870-25.patch, failed testing.

disasm’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI-conversion
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This was just a random failure.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary: View changes

adding related issues.