Skip to content

Default team will not work if primary key for user changes from id() to uuid() #218

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

Closed
AngelinCalu opened this issue Sep 17, 2020 · 8 comments

Comments

@AngelinCalu
Copy link

  • Jetstream Version: 1.2.1
  • Jetstream Stack: Livewire
  • Laravel Version: 8.4.0
  • PHP Version: 7.4.6
  • Database Driver & Version: Ver 15.1 Distrib 10.5.4-MariaDB, for Win64 (AMD64)

Description:

When user registers through the form everything works perfectly.
When I'm trying to log in with a user created with:

User::factory(50)->create();

I'm getting an ErrorException Trying to get property 'id' of non-object on currentTeam() function from
...\vendor\laravel\jetstream\src\HasTeams.php:28

Steps To Reproduce:

  • Create a fresh laravel instalation with the jetstream package (livewire stack)
  • Change the the primary key of the user from 'id' to 'uuid'
  • Update all the relevant foreign fields to match the change.
  • Create a new user through form registration -> This works normally.
  • Create a new user through model factory / seeder -> This will produce the error.
@driesvints
Copy link
Member

I think we only support regular id's but need confirmation by @taylorotwell.

/cc @crynobone

@elbaylot
Copy link

@AngelinCalu you can read this PR : #22

use numeric ids for users

@AngelinCalu
Copy link
Author

AngelinCalu commented Sep 17, 2020

The fact that this actually works when creating the user though the registration form, and not when creating it from the model factory, it's an alarm signal that the problem is not about using uuid's or not, but about creating a new account workflow.

From what I can see that's because in app/Actions/Fortify/CreateNewUser.php there is a default team created with the user, and when creating the user through the factory I would have to manually create a default team for each user to bypass the error.

Is there something obvious that I'm maybe missing here?

return DB::transaction(function () use ($input) {
            return tap(User::create([
                'name' => $input['name'],
                'email' => $input['email'],
                'password' => Hash::make($input['password']),
            ]), function (User $user) {
                $this->createTeam($user);   // <- this part here
            });
        });

LE: after all this is only an inconvenience when writing tests, as in production everybody will eventually use the registration form to create a new account, thus creating that default team as shown above ☝️

@AngelinCalu
Copy link
Author

For anyone else facing the same problem, my workaround was to manually create a personal team for each user created through the factory in the "seed-ing" process:

User::factory(50)->create();

        foreach (User::all() as $user) {
            $user->ownedTeams()->save(Team::forceCreate([
                'user_id' => $user->id,
                'name' => explode(' ', $user->name, 2)[0]."'s Team",
                'personal_team' => true,
            ]));
        }

The downsides remaining:

  • You have to make sure you're seeding the database before running tests that include users
  • Users created inside the tests would face the same problem so only the previously created ones work

@taylorotwell
Copy link
Member

We don't support UUIDs out of the box.

@AngelinCalu
Copy link
Author

@driesvints, @taylorotwell any changes now that this got merged?

@SimonMacIntyre
Copy link

This is a massive bummer. It means you cannot use Sanctum with uuidMorphs with Jetstream, unless many methods are overridden. I am pretty far down that path now, but not sure how painful it will be to complete or maintain.

@GringoDotDev
Copy link

GringoDotDev commented Nov 30, 2023

For any future travelers who stumble onto this issue, here is how I solved it (I'm using nano id, but uuid should work almost the same):

  1. Override Team::getRouteKeyName
    public function getRouteKeyName(): string
    {
        return 'nano_id';
    }
  1. Override the teams.show route from Jetstream (Livewire in my case, but again, similar approach applies to Inertia)
use App\Models\Team;
use Illuminate\Http\Request;
use Laravel\Jetstream\Http\Controllers\Livewire\TeamController;

Route::group(['middleware' => config('jetstream.middleware', ['web'])], function () {
    $authMiddleware = config('jetstream.guard')
        ? 'auth:'.config('jetstream.guard')
        : 'auth';

    $authSessionMiddleware = config('jetstream.auth_session', false)
        ? config('jetstream.auth_session')
        : null;

    Route::group(['middleware' => array_values(array_filter([$authMiddleware, $authSessionMiddleware]))], function () {
        Route::group(['middleware' => 'verified'], function () {
            Route::get('/teams/{team}', function (Request $request, Team $team) {
                return app()->make(TeamController::class)->show($request, $team->id);
            })->name('teams.show');
        });
    });

All the middleware, etc. is essentially a direct copy from the vendor file included in Jetstream. Then we just override the team-id dependent route, dereference the id after dependency injection (which will be resolved how we want thanks to the getRouteKeyName override), and pass it to the vendor controller.

We're using Laravel's service container here to resolve the TeamController class, which is a little more idiomatic than instantiating the controller directly.

Janky? Yes. Jankier than the alternatives? Meh, it's a matter of taste, but this feels ok to me.

Hope it helps someone out there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants