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
#14 1987894-user-logout-route-14.patch6.75 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 56,507 pass(es). View
#10 1987894-user-logout-route-10.patch6.88 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 55,571 pass(es). View
#10 interdiff.txt2.34 KBParisLiakos
#9 1987894-user-logout-route-9.patch6.4 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,757 pass(es), 35 fail(s), and 4 exception(s). View
#9 interdiff.txt1.71 KBParisLiakos
#6 1987894-user-logout-route-6.patch7.33 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,572 pass(es), 39 fail(s), and 1 exception(s). View
#6 interdiff.txt1.26 KBParisLiakos
#5 1987894-user-logout-route-5.patch7.33 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,960 pass(es), 39 fail(s), and 1 exception(s). View
#5 interdiff.txt1001 bytesParisLiakos
#4 1987894-user-logout-route-4.patch7.04 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,844 pass(es), 39 fail(s), and 1 exception(s). View
#4 interdiff.txt1.4 KBParisLiakos
#3 1987894-user-logout-route-3.patch5.78 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 56,196 pass(es), 39 fail(s), and 1 exception(s). View
#1 1987894-user-logout-route-1.patch3.86 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 55,532 pass(es), 0 fail(s), and 418 exception(s). View

Comments

vijaycs85’s picture

Status: Active » Needs review
FileSize
3.86 KB
FAILED: [[SimpleTest]]: [MySQL] 55,532 pass(es), 0 fail(s), and 418 exception(s). View

Initial patch...

Status: Needs review » Needs work

The last submitted patch, 1987894-user-logout-route-1.patch, failed testing.

ParisLiakos’s picture

Assigned: Unassigned » ParisLiakos
Status: Needs work » Needs review
FileSize
5.78 KB
FAILED: [[SimpleTest]]: [MySQL] 56,196 pass(es), 39 fail(s), and 1 exception(s). View
ParisLiakos’s picture

FileSize
1.4 KB
7.04 KB
FAILED: [[SimpleTest]]: [MySQL] 55,844 pass(es), 39 fail(s), and 1 exception(s). View

also..

ParisLiakos’s picture

FileSize
1001 bytes
7.33 KB
FAILED: [[SimpleTest]]: [MySQL] 55,960 pass(es), 39 fail(s), and 1 exception(s). View

meh..this wont fire hooks..something a bit uglier then

ParisLiakos’s picture

FileSize
1.26 KB
7.33 KB
FAILED: [[SimpleTest]]: [MySQL] 55,572 pass(es), 39 fail(s), and 1 exception(s). View

humpff, wrong namespace

Crell’s picture

+++ b/core/modules/user/user.module
@@ -906,11 +907,9 @@ function user_menu() {
@@ -1056,8 +1055,7 @@ function user_menu_site_status_alter(&$menu_site_status, $path) {

@@ -1056,8 +1055,7 @@ function user_menu_site_status_alter(&$menu_site_status, $path) {
   if ($menu_site_status == MENU_SITE_OFFLINE) {
     // If the site is offline, log out unprivileged users.
     if (user_is_logged_in() && !user_access('access site in maintenance mode')) {
-      module_load_include('pages.inc', 'user', 'user');
-      user_logout();
+      UserController::create(Drupal::getContainer())->logout()->send();
     }

This seems very wrong... You should never send() the response yourself.

ParisLiakos’s picture

i know its ugly like i said in #5 but i am open to alternatives!

hmm maybe i should keep user_logout() and fix this in another issue

ParisLiakos’s picture

FileSize
1.71 KB
6.4 KB
FAILED: [[SimpleTest]]: [MySQL] 55,757 pass(es), 35 fail(s), and 4 exception(s). View

there, lets keep this issue moving, cause this access check here is needed to several other conversions and fixing openid for drupal_exit removal

ParisLiakos’s picture

FileSize
2.34 KB
6.88 KB
PASSED: [[SimpleTest]]: [MySQL] 55,571 pass(es). View

WebTestBase--

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

The last submitted patch, 1987894-user-logout-route-10.patch, failed testing.

ParisLiakos’s picture

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

cant reproduce locally
#10: 1987894-user-logout-route-10.patch queued for re-testing.

dawehner’s picture

+++ b/core/modules/user/lib/Drupal/user/Controller/UserController.phpundefined
@@ -0,0 +1,69 @@
+use Drupal\user\Plugin\Core\Entity\User;

Do we really need this statement? I can't see any usage.

+++ b/core/modules/user/lib/Drupal/user/Access/LoginStatusCheck.phpundefined
index 7e294f5..46745ec 100644
--- a/core/modules/user/lib/Drupal/user/UserAutocompleteController.php

--- a/core/modules/user/lib/Drupal/user/UserAutocompleteController.php
+++ b/core/modules/user/lib/Drupal/user/Controller/UserAutocompleteController.phpundefined

Renaming an existing file feels a bit unrelated though this could make sense.

ParisLiakos’s picture

FileSize
6.75 KB
PASSED: [[SimpleTest]]: [MySQL] 56,507 pass(es). View

thanks for the review!

this conflicted with roles commit, so no interdiff, sorry.

i removed the unneeded use statement

Renaming an existing file feels a bit unrelated though this could make sense.

yes i moved cause i just created the directory, i dont think we should have a seperate issue for moving this

Crell’s picture

Why exactly aren't we removing user_logout(), the function?

ParisLiakos’s picture

because it is needed by user_menu_site_status_alter
it does something hacky, and imo should be converted to an event listener. you cant redirect like that, but fixing this should happen in the RedirectResponse issue

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Great, thank you.

ParisLiakos’s picture

alexpott’s picture

Committed e21c1a1 and pushed to 8.x. Thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

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