laravel / jetstream

Tailwind scaffolding for the Laravel framework.
https://jetstream.laravel.com
MIT License
3.97k stars 822 forks source link

option to disable personal teams #117

Closed rroethof closed 4 years ago

rroethof commented 4 years ago

As for a lot of my projects I will be using teams as roles, i dislike personal teams Having 250 users on a project for my employer,, it kinda sucks to have 250 'personal' teams .. i know you can check if a team is personal or not, but be honest.. it's not the cleanest way for i.e. database etc.

so an option to disable personal themes would be grately appreciated

rama-adi commented 4 years ago

bump, this will be very useful as I not need the personal team feature.

mallardduck commented 4 years ago

EDIT: Just do it yourself by adjusting the Actions that Jetstream publishes. When it's installed Jetstream will install a CreateNewUser action class into your app folder. This code is your responsibilty to maintain, so you can simply edit the create method:

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

Into:

            return User::create([
                'name' => $input['name'],
                'email' => $input['email'],
                'password' => Hash::make($input['password']),
            ]);
rroethof commented 4 years ago

What happens when one of your users wants share access to something they "own" to another User?

That question is a invalid question for 99.9% of the projects I am working on for my employer. In these projects, there is a small group who are 'admins', the rest only requires read and/or write access to 'modules' each module would be coresponding to a team.. i.e. a user that needs to access the CMDB would be member of the CMDB team..

They have no personal anything other then a username/password on these systems, therefore this question would be invalid in these.

mallardduck commented 4 years ago

@rroethof It sounds like your use case is very specific to your employers needs and doesn't strictly fit the SaaS model that jetstream is intended for.

I would also say that based on what you described it sounds like you can fairly easily work around the issue too. If users only need access to resources through the respective teams they are assigned, then just don't create them a personal team?

Unless something changed from the pre-release version - which I started working with - I had to specifically create an Eloquent listener on my user model to create the personal team. So in theory you don't need to create personal teams at all. So maybe instead you create a default team of sorts for a user to be in before they are given access to specific modules and add them to that.

However, I don't know how much sense it makes to implement all that directly in Jetstream - it seems pretty specific to an individual use case. I believe that if you gave it a shot there would be a way for you to extend Jetstream - as is - and get the results you're seeking.

Taylor already built it in a manner where the Models it uses are 'owned' by your App, so most the control already lies in your hands. Further the Jetstream service is extensible and you can update your app's JetstreamServiceProvider easily to use this extended version.

mallardduck commented 4 years ago

After looking over things further I realized that I was wrong earlier about Personal Teams needing to be created with an Eloquent listener.

I see now that within the App itself - if you enable teams - the CreateNewUser action will in fact create a new user. This just was something I overlooked since my seeded user wasn't created with this action.

Given that Jetstream publishes this action class to your App this means it's trivially easy to disable teams. Simply remove the create team action from the tap call.

https://github.com/laravel/jetstream/blob/cf06bbf91dd739a31a9e3f52d0d18475c5053d11/stubs/app/Actions/Fortify/CreateNewUserWithTeams.php#L35

imClement commented 4 years ago

It is not enough to remove the create team action from the tap call because jetstream/src/HasTeams.php will try to forceFill a null current_team_id

    public function currentTeam()
    {
        if (is_null($this->current_team_id)) {
            $this->forceFill([
                'current_team_id' => $this->personalTeam()->id,
            ])->save();
        }

and that will cause a Trying to get property 'id' of non-object error after the registration.

rroethof commented 4 years ago

And if you deleted a team using the top menu, when you do not have a personal team.. same Trying to get property 'id' of non-object

taylorotwell commented 4 years ago

This is a feature request and not an issue report. To help us keep issues to only bug reports will close this for now. Hopefully I'll be able to enable Discussions on this GitHub repository soon.

taylorotwell commented 4 years ago

Also as a note so it isn't PR'd... I don't plan to pursue this. I think it is better for a user to always have at least 1 team.

JGamboa commented 4 years ago

@taylorotwell On my case i have a default team for everyone, and i want to change the HasTeam Logic, so copy that trait to an own trait, it should be enought right?

NickStarlight commented 4 years ago

For anyone who is following a SaaS model, you probably don't wan't pollute your database with thousands of teams that will mostly make your permission and modularity harder to maintain, you also probably want your users to request team access or even choose sometimes. Although @taylorotwell has stated that this will not be pursed, i will, for the sake of this being the top result on Google, offer my take on this.

1- Remove the personal team creation method on app/Actions/Fortify/CreateNewUser.php

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

2- Create a new Middleware php artisan make:middleware HasTeam with the following handler:

    public function handle(Request $request, Closure $next)
    {
        if (Auth::user()->currentTeam->count() === 0) {
            return Response(view('no-team'));
        }

        return $next($request);
    }

3- Create a new view php artisan make:view no-team, the content is whatever looks good on your use case.

4- The original currentTeam method returns an instance of App\Model\Team, however, the currentTeam method, if null, will call the personalTeam() method in order to retrieve the 'default' team, this will fail since we didn't create any personal teams for the user, in order to fix this, we need to customize the HasTeams trait:

4.1 - Create a new folder: App/Traits/Jetstream 4.2 - Copy the contents of vendor/laravel/jetstream/src/HasTeams.php inside this new folder. 4.3 - Change the structure as following:

- namespace Laravel\Jetstream;
+namespace App\Traits\Jetstream;

+use Laravel\Jetstream\Jetstream;

-    /**
-     * Get the current team of the user's context.
-     */
-    public function currentTeam()
-    {
-        if (is_null($this->current_team_id) && $this->id) {
-            $this->switchTeam($this->personalTeam());
-        }
-
-        return $this->belongsTo(Jetstream::teamModel(), 'current_team_id');
-    }

+    /**
+     * Get the current team of the user's context.
+     */
+    public function currentTeam()
+    {
+        if (is_null($this->current_team_id) && $this->id) {
+            $team = $this->allTeams()->first();
+
+            if ($team) {
+                $this->switchTeam($team);
+            } else {
+                /** The return MUST be a Team Model */
+                return $this->ownedTeams();
+            }
+            
+        }
+
+       return $this->belongsTo(Jetstream::teamModel(), 'current_team_id');
+    }

What i'm basically doing here is instead of setting the team to personal, i'm fetching the first team available, if there are no teams available, i'm returning ownedTeams() that is an empty collection, that can be checked at step 2 with Auth::user()->currentTeam->count() === 0.

5- Update your app/Models/User.php to use App\Traits\Jetstream\HasTeams;

6- Finally, register the middleware on Kernel.php and add it to your routes.

This should suffice, however, all scaffolding from this point will break, so it' important that you intercept this with a middleware before the request reaches any Blade files, you can do a form for requesting a team, or anything that fits your use case. This is a hacky solution, since the whole Jetstream scaffold was designed with a Team in mind(when enabled), so be careful.

wivaku commented 3 years ago

Created a workaround similar to @NickStarlight

The difference: only overwrite specific trait methods from vendor/laravel/jetstream/src/HasTeams.php They simply make the references to $team optional: optional($team)

In short:

The complete HasNoPersonalTeams.php:

<?php

namespace App\Traits\JetStream;

use Laravel\Jetstream\Jetstream;

trait HasNoPersonalTeams
{

    /**
     * Determine if the user owns the given team.
     *
     * @param  mixed  $team
     * @return bool
     */
    public function ownsTeam($team)
    {
        // return $this->id == $team->user_id;
        return $this->id == optional($team)->user_id;
    }

    /**
     * Determine if the given team is the current team.
     *
     * @param  mixed  $team
     * @return bool
     */
    public function isCurrentTeam($team)
    {
        // return $team->id === $this->currentTeam->id;
        return optional($team)->id === $this->currentTeam->id;
    }

}

Tests of all the methods in HasTeams.php: (prerequisite: at least one team with owner)

$team = Team::first()
$owner = $team->owner

$user = \App\Models\User::factory()->create()
$userTeam = $user->currentTeam // null

$owner->belongsToTeam($team) // true

$user->ownsTeam($team) // false
$user->ownsTeam($userTeam) // false
$user->belongsToTeam($team) // false
$user->belongsToTeam($userTeam) // false
$user->currentTeam // null
$user->allTeams() // []
$user->ownedTeams()->get() // []
$user->teams()->get() // []
$user->personalTeam() // null
$user->teamRole($team) // null

$user->delete()
marijoo commented 3 years ago

Thanks @wivaku – this seems to work out quite well. I also added a method to the HasNoPersonalTeams trait to determine if the user has any teams which is a nice helper for conditionally show the team options in the navigation-dropdown.blade.php for example.

/**
 * Determine if the user is assigned to any teams.
 *
 * @return bool
 */
public function belongsToAnyTeam()
{
    return (bool) optional($this->teams)->isNotEmpty();
}
scc-agnitio commented 3 years ago

Also posted a workaround, by overriding a few methods of the HasTeams trait.

https://laracasts.com/discuss/channels/code-review/proper-way-to-disable-personal-teams-in-jetstream?page=1#reply=668759

driesvints commented 3 years ago

Hey everyone,

I'm locking this issue because it either has gone off-topic, become a dumping ground for things which shouldn't be in an issue tracker or is just too old. Please try to discuss things further on one of the below channels: