[5.9] Extract registered event and login to registered method

[antonkomarev]

Resubmit of PR #27800 to Laravel 5.9

[lindyhopchris]

@antonkomarev the Laravel 6.0 upgrade guide states that you need to call the parent::registered() method. However this is not possible because RegistersUsers is a trait. So this is a breaking change unless the upgrade guide states that we need to rename the registered function from the trait when importing it.

cc @driesvints

[antonkomarev]

@lindyhopchris why can't you call registered method? It's protected, not private.

[lindyhopchris]

Because if I use a trait that has registered on it, then implement my own registered method, it means I can only call the one from the trait if I rename the method, i.e. import the trait as:

use RegistersUsers { registered as afterRegister; }

protected function registered($request, $user)
{
    $this->afterRegister($request, $user);
    return \response('my-custom response', 200);
}

That's fine if that's how we're meant to use it, but I thought registered was there so that you could return a different response to the standard redirect... which we do because we always check if the client wants JSON to know whether or not to return the redirect response or a JSON response.

For parent to work I'd have to be extending a controller which has the registered() method on it already, which is not the case on the default RegisterController in a brand new Laravel app.

Not raising this to necessarily say the approach is wrong... more to point out that what the upgrade guides currently say is wrong.

[antonkomarev]

Have you tried to implement it like this?

protected function registered($request, $user)
{
    parent::registered($request, $user);
    return \response('my-custom response', 200);
}

[antonkomarev]

And why did you return something in registered method? It's just like an event. In should be executed under the hood of register method.

Edit: I've removed that it's not designed to return anything because of return mixed on docblock. But IMHO if you want to return something - do it in register method instead.

[lindyhopchris]

Why does the register() method on the trait do this:

        return $this->registered($request, $user)
                        ?: redirect($this->redirectPath());

That totally implies it's to return something other than the redirect response?! So that's the reason we've been using it to return JSON responses.... and have been since at least Laravel 5.4 (as that's the first version our app was on).

If we're now meant to do it in the register() method... before we just returned the response we wanted, now we'll have to run validation, create the user, call registered() then return our custom response... doesn't seem like an improvement.

Currently we have this:

protected function registered($request, $user)
{
    if ($request->wantsJson()) {
        return response()->json($user);
    }
}

After your changes we'd need to do:

public function register($request)
{
    $this->validator($request->all())->validate();
    $user = $this->create($request->all());
    $this->registered($request, $user); // to login and fire event.

    if ($request->wantsJson()) {
        return response()->json($user);
    }

    return redirect($this->redirectPath());
}

As the parent register() method cannot be called for the same reason.

So from our view it's a big backwards step. And the upgrade guide would have to make that clear.