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
#21 common_test-1987644-21.patch3.09 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,194 pass(es).
[ View ]
#18 drupal8.system-module.1987644-18.patch3.12 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 59,077 pass(es).
[ View ]
#14 drupal8.system-module.1987644-14.patch3 KBmparker17
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.system-module.1987644-14.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#14 interdiff.txt817 bytesmparker17
#11 drupal8.system-module.1987644-11.patch3 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 58,685 pass(es).
[ View ]
#11 interdiff.txt1.39 KBmparker17
#7 drupal8.system-module.1987644-7.patch2.96 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 58,403 pass(es).
[ View ]
#7 interdiff.txt1.15 KBmparker17
#4 system-common_test_destination_controller-1987644-4.patch2.8 KBmparker17
PASSED: [[SimpleTest]]: [MySQL] 58,137 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'll help!

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

Try this...

  1. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestTextController.php
    @@ -0,0 +1,26 @@
    +   * Prints a destination query parameter.

    Let's also put a @return statement in there.

  2. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestTextController.php
    @@ -0,0 +1,26 @@
    +    $destination = drupal_get_destination();
    +    $output = "The destination: " . check_plain($destination['destination']);

    check_plain can be replaced by String::checkPlain()

Status:Needs review» Needs work
  1. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestTextController.php
    @@ -0,0 +1,26 @@
    +   * Prints a destination query parameter.

    Let's also put a @return statement in there.

  2. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestTextController.php
    @@ -0,0 +1,26 @@
    +    $destination = drupal_get_destination();
    +    $output = "The destination: " . check_plain($destination['destination']);

    check_plain can be replaced by String::checkPlain()

Status:Needs work» Needs review
StatusFileSize
new1.15 KB
new2.96 KB
PASSED: [[SimpleTest]]: [MySQL] 58,403 pass(es).
[ View ]

Try this...

  1. +++ b/core/modules/system/tests/modules/common_test/common_test.routing.yml
    @@ -4,3 +4,9 @@ common_test_l_active_class:
    +  pattern: 'common-test/destination'

    Sorry, but pattern should start with "/"

  2. +++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestTextController.php
    @@ -0,0 +1,31 @@
    +   * @return Response

    ... these parameters should be absolute, so \Symfony\Component\HttpFoundation\Response ...

Status:Needs review» Needs work

K

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.

Status:Needs work» Needs review
StatusFileSize
new1.39 KB
new3 KB
PASSED: [[SimpleTest]]: [MySQL] 58,685 pass(es).
[ View ]

Try this...

+++ b/core/modules/system/tests/modules/common_test/lib/Drupal/common_test/Controller/CommonTestTextController.php
@@ -0,0 +1,31 @@
+   * @return Symfony\Component\HttpFoundation\Response

Missing trailing "\" in the @return statement.

The rest looks perfect!

Trailing slash or leading slash? :)

StatusFileSize
new817 bytes
new3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.system-module.1987644-14.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Try this instead...

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

The last submitted patch, drupal8.system-module.1987644-14.patch, failed testing.

Status:Needs work» Needs review

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

The last submitted patch, drupal8.system-module.1987644-14.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.12 KB
PASSED: [[SimpleTest]]: [MySQL] 59,077 pass(es).
[ View ]

Straight re-roll of the patch...

Status:Needs review» Reviewed & tested by the community

That is fine ... even we should rather convert drupal_get_destination() to something else, but yeah this is a follow up.

Status:Reviewed & tested by the community» Needs work

Sorry, no longer applies.

Status:Needs work» Needs review
StatusFileSize
new3.09 KB
PASSED: [[SimpleTest]]: [MySQL] 59,194 pass(es).
[ View ]

Let's move the function into the existing controller.

Status:Needs review» Reviewed & tested by the community

Hey @dawehner,

I had put this into a different controller based on a conversation with @Crell at the Midwest Drupal Developer Summit, where we decided it would be helpful to have separate controller objects for different return types.

However, I don't think it's worth holding up the patch for that... I really want to see the old routing system gone!

The code looks good.
The tests pass.
Manual testing works.

Looks RTBC! Thanks for all your help!

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

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