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
StatusFileSize
new3.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

StatusFileSize
new2.28 KB
new3.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

StatusFileSize
new3.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
StatusFileSize
new1.63 KB
new4.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
StatusFileSize
new4.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 :)