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
#11 1987728-language-test-subrequest-controller-11.patch4.74 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,575 pass(es). View
#8 1987728-language-test-subrequest-controller-8.patch4.79 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 56,371 pass(es). View
#8 1987728-diff-5-8.txt1.63 KBvijaycs85
#4 1987728-language-test-subrequest-controller-4.patch3.54 KBNiklas Fiekas
PASSED: [[SimpleTest]]: [MySQL] 57,351 pass(es). View
#4 1987728-language-test-subrequest-controller-4-interdiff.txt2.28 KBNiklas Fiekas
#5 1987728-language-test-subrequest-controller-5.patch3.53 KBNiklas Fiekas
PASSED: [[SimpleTest]]: [MySQL] 58,011 pass(es). View
#2 1987728-language-test-subrequest-controller-1.patch3.43 KBNiklas Fiekas
PASSED: [[SimpleTest]]: [MySQL] 55,702 pass(es). View

Comments

Niklas Fiekas’s picture

Assigned: Unassigned » Niklas Fiekas
Niklas Fiekas’s picture

Assigned: Niklas Fiekas » Unassigned
Status: Active » Needs review
FileSize
3.43 KB
PASSED: [[SimpleTest]]: [MySQL] 55,702 pass(es). View

Converted this one. Not sure if this should be added to the existing LanguageTestController or should have an extra controller, because this is the only one that needs the http_kernel service.

dawehner’s picture

+++ b/core/modules/language/tests/language_test/language_test.routing.ymlundefined
@@ -11,3 +11,10 @@ language_test_theme_link_active_class:
+    _content: '\Drupal\language_test\Controller\LanguageTestController::testSubRequest'

Just wondering whether it makes sense to use content when there is a subrequest anyway which will trigger the HtmlPageController anyway.

+++ b/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.phpundefined
@@ -16,10 +18,27 @@
+    $this->http_kernel = $http_kernel;

Should be better $this->httpKernel

+++ b/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.phpundefined
@@ -110,4 +129,11 @@ public function lActiveClass() {
+   * Uses a subrequest to retrieve the 'user' page.
...
+  public function testSubRequest() {

Needs @return

Niklas Fiekas’s picture

FileSize
2.28 KB
3.54 KB
PASSED: [[SimpleTest]]: [MySQL] 57,351 pass(es). View

Thanks for the review!

Just wondering whether it makes sense to use content when there is a subrequest anyway which will trigger the HtmlPageController anyway.

Right. I wasn't aware there is anything else than _content when I wrote the patch.

Also fixed the variable name and the missing @return.

Niklas Fiekas’s picture

FileSize
3.53 KB
PASSED: [[SimpleTest]]: [MySQL] 58,011 pass(es). View

Forgot this hunk:

-   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $http_kernel
+   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $httpKernel
dawehner’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.phpundefined
@@ -16,10 +18,27 @@
    * Implements \Drupal\Core\ControllerInterface::create().

Just in case there is a rerole needed, let's use {@inheritdoc}

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

curl https://drupal.org/files/1987728-language-test-subrequest-controller-5.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  3619  100  3619    0     0   4771      0 --:--:-- --:--:-- --:--:--  6218
error: patch failed: core/modules/language/tests/language_test/language_test.module:97
error: core/modules/language/tests/language_test/language_test.module: patch does not apply
vijaycs85’s picture

Status: Needs work » Needs review
FileSize
1.63 KB
4.79 KB
PASSED: [[SimpleTest]]: [MySQL] 56,371 pass(es). View

Conflict is because of #1888424: Make Drupal's URL generation logic available to HttpKernel, and minimize code repetition/divergence landed. updated the code from callback to method.

Crell’s picture

Status: Needs review » Reviewed & tested by the community

Seems straightforward enough to me.

alexpott’s picture

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

And again...

curl https://drupal.org/files/1987728-language-test-subrequest-controller-8.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4908  100  4908    0     0   1910      0  0:00:02  0:00:02 --:--:--  2075
error: patch failed: core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.php:9
error: core/modules/language/tests/language_test/lib/Drupal/language_test/Controller/LanguageTestController.php: patch does not apply
vijaycs85’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
4.74 KB
PASSED: [[SimpleTest]]: [MySQL] 57,575 pass(es). View

Re-rolling...

Crell’s picture

Status: Needs review » Reviewed & tested by the community

And again...

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed dec3e5c and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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

Gábor Hojtsy’s picture

Retroactive tags :)