Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.6] Update AuthenticatesUsers.php #24717

Merged
merged 1 commit into from Jul 3, 2018
Merged

[5.6] Update AuthenticatesUsers.php #24717

merged 1 commit into from Jul 3, 2018

Conversation

guizoxxv
Copy link
Contributor

@guizoxxv guizoxxv commented Jul 1, 2018

Added function loggedOut based on registered and authenticated methods from Laravel authentication scaffold.

Added function loggedOut based on registered and authenticated methods from Laravel authentication scaffold.
@jmarcher
Copy link
Contributor

jmarcher commented Jul 2, 2018

I do not undestand what this method is soposed to do...

We have something like

auth()->check()

@devcircus
Copy link
Contributor

He just didn't explain it well. It's just like the "registered" method in the "RegistersUsers" trait. It gives you an option to do something other than "redirect()", after you log-out.

In my opinion, it is unnecessary, as you can simply override the "logout" method and run your custom code.

@tillkruss tillkruss changed the title Update AuthenticatesUsers.php [5.6] Update AuthenticatesUsers.php Jul 2, 2018
@guizoxxv
Copy link
Contributor Author

guizoxxv commented Jul 3, 2018

Yes, the idea is to be able to do something after logging out. Yes, you can overwrite the logout method, but the idea is to make the code inside /app/Http/Controllers/Auth controllers cleaner. I believe that is the point of having the authenticated() and registered() methods. Without the suggested implementation I would have to overwrite the logout() method like this:

    public function logout(Request $request)
    {
        $this->guard()->logout();

        $request->session()->invalidate();
        
       // Do something

        return redirect('/');
    }

$this->guard() is referencing a method only visible in the vendor folder so it would be cleaner if it didn't show. Also the logout method is running behind the scenes like an abstraction so it should only appear outside the vendor folder when the user wants to change the code. That's not the case, I'm only interested in adding a functionality after logging out which is rather normal.

PS: I don't get why continuous-integration/styleci/pr failed. I see no message specifying the error.

@devcircus
Copy link
Contributor

The style error is an issue with the blank line, or lack thereof, between the end of the method and the next docblock.

    //
    return $this->loggedOut($request) ?: redirect('/');     
}
-
+
     /**      
      * The user has logged out.
      *
--

@taylorotwell taylorotwell merged commit e4a00e8 into laravel:5.6 Jul 3, 2018
@guizoxxv guizoxxv deleted the patch-1 branch July 4, 2018 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants