Implement EnsureUserHasWorkspace Trait and Integrate into Controllers… (#741)

* Implement EnsureUserHasWorkspace Trait and Integrate into Controllers and Jobs

- Introduced the EnsureUserHasWorkspace trait to ensure users have at least one workspace when they are detached from a workspace.
- Integrated the trait into WorkspaceUserController to enforce workspace checks during user detachment.
- Updated RemoveWorkspaceGuests job to utilize the new trait for ensuring users have a workspace after detachment.
- Modified UserWorkspace model to call the ensureUserHasWorkspace method upon deletion, maintaining workspace integrity.

These changes enhance user management by ensuring that users always have a workspace, improving overall application stability.

* Add test

---------

Co-authored-by: Julien Nahum <julien@nahum.net>

api/app/Http/Controllers/WorkspaceUserController.php

@@ -4,6 +4,7 @@ namespace App\Http\Controllers;
 
 use App\Jobs\Billing\WorkspaceUsersUpdated;
 use App\Models\UserInvite;
+use App\Traits\EnsureUserHasWorkspace;
 use Illuminate\Http\Request;
 use App\Models\Workspace;
 use App\Models\User;
@@ -11,6 +12,8 @@ use App\Service\WorkspaceHelper;
 
 class WorkspaceUserController extends Controller
 {
+    use EnsureUserHasWorkspace;
+
     public function __construct()
     {
         $this->middleware('auth');
@@ -106,7 +109,9 @@ class WorkspaceUserController extends Controller
         $workspace = Workspace::findOrFail($workspaceId);
         $this->authorize('adminAction', $workspace);
 
+        $user = User::findOrFail($userId);
         $workspace->users()->detach($userId);
+        $this->ensureUserHasWorkspace($user);
         WorkspaceUsersUpdated::dispatch($workspace);
 
         return $this->success([
@@ -119,7 +124,9 @@ class WorkspaceUserController extends Controller
         $workspace = Workspace::findOrFail($workspaceId);
         $this->authorize('view', $workspace);
 
-        $workspace->users()->detach($request->user()->id);
+        $user = $request->user();
+        $workspace->users()->detach($user->id);
+        $this->ensureUserHasWorkspace($user);
 
         return $this->success([
             'message' => 'You have left the workspace successfully.'

api/app/Jobs/Billing/RemoveWorkspaceGuests.php

@@ -4,11 +4,13 @@ namespace App\Jobs\Billing;
 
 use App\Models\User;
 use App\Models\Workspace;
+use App\Traits\EnsureUserHasWorkspace;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
+use Illuminate\Support\Facades\Log;
 
 class RemoveWorkspaceGuests implements ShouldQueue
 {
@@ -16,6 +18,7 @@ class RemoveWorkspaceGuests implements ShouldQueue
     use InteractsWithQueue;
     use Queueable;
     use SerializesModels;
+    use EnsureUserHasWorkspace;
 
     /**
      * Create a new job instance.
@@ -50,13 +53,14 @@ class RemoveWorkspaceGuests implements ShouldQueue
 
             // Detach all users from the workspace (except the owner)
             foreach ($workspace->users()->where('users.id', '!=', $this->user->id)->get() as $user) {
-                \Log::info('Detaching user from workspace', [
+                Log::info('Detaching user from workspace', [
                     'workspace_id' => $workspace->id,
                     'workspace_name' => $workspace->name,
                     'user_id' => $user->id,
                     'user_email' => $user->email,
                 ]);
                 $workspace->users()->detach($user);
+                $this->ensureUserHasWorkspace($user);
             }
         });
     }

api/app/Models/UserWorkspace.php

@@ -2,12 +2,15 @@
 
 namespace App\Models;
 
+use App\Traits\EnsureUserHasWorkspace;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 
 class UserWorkspace extends Model
 {
     use HasFactory;
+    use EnsureUserHasWorkspace;
+
     protected $table = 'user_workspace';
 
     protected $fillable = [
@@ -25,4 +28,13 @@ class UserWorkspace extends Model
     {
         return $this->belongsTo(Workspace::class);
     }
+
+    protected static function boot()
+    {
+        parent::boot();
+
+        static::deleted(function (UserWorkspace $userWorkspace) {
+            $userWorkspace->ensureUserHasWorkspace($userWorkspace->user);
+        });
+    }
 }

api/app/Traits/EnsureUserHasWorkspace.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace App\Traits;
+
+use App\Models\User;
+use App\Models\Workspace;
+use Illuminate\Support\Facades\Log;
+
+trait EnsureUserHasWorkspace
+{
+    /**
+     * Ensure a user has at least one workspace.
+     * If they don't, create a new workspace for them.
+     */
+    protected function ensureUserHasWorkspace(User $user): void
+    {
+        if ($user->workspaces()->count() === 0) {
+            Log::info('Creating new workspace for user with no workspaces', [
+                'user_id' => $user->id,
+                'user_email' => $user->email,
+            ]);
+
+            $newWorkspace = Workspace::create([
+                'name' => 'My Workspace',
+            ]);
+
+            $user->workspaces()->sync([
+                $newWorkspace->id => [
+                    'role' => 'admin',
+                ],
+            ], false);
+        }
+    }
+}

api/tests/Feature/EnsureUserHasWorkspaceTest.php

@@ -0,0 +1,211 @@
+<?php
+
+use App\Models\User;
+use App\Models\Workspace;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use App\Jobs\Billing\RemoveWorkspaceGuests;
+use Illuminate\Support\Facades\DB;
+
+uses(RefreshDatabase::class);
+
+test('detaching user from last workspace creates a default one via controller', function () {
+    // Arrange: User with one workspace, acting user is admin
+    $admin = User::factory()->create();
+    $workspace = Workspace::factory()->create();
+    $userToDetach = User::factory()->create();
+    $workspace->users()->attach($admin, ['role' => 'admin']);
+    $workspace->users()->attach($userToDetach, ['role' => 'member']);
+
+    expect($userToDetach->workspaces()->count())->toBe(1);
+
+    // Act: Admin detaches the user
+    $this->actingAs($admin)
+        ->deleteJson(route('open.workspaces.users.remove', ['workspaceId' => $workspace->id, 'userId' => $userToDetach->id]))
+        ->assertOk();
+
+    // Assert: User now has 1 workspace, named 'My Workspace'
+    $userToDetach->refresh();
+    expect($userToDetach->workspaces()->count())->toBe(1);
+
+    // Check workspace name
+    $newWorkspace = $userToDetach->workspaces()->first();
+    expect($newWorkspace->name)->toBe('My Workspace');
+
+    // Check role by querying the pivot table directly
+    $pivotData = DB::table('user_workspace')
+        ->where('user_id', $userToDetach->id)
+        ->where('workspace_id', $newWorkspace->id)
+        ->first();
+    expect($pivotData->role)->toBe('admin');
+});
+
+test('detaching user from one workspace when they have others does not create default one via controller', function () {
+    // Arrange: User with two workspaces, acting user is admin
+    $admin = User::factory()->create();
+    $workspace1 = Workspace::factory()->create();
+    $workspace2 = Workspace::factory()->create();
+    $userToDetach = User::factory()->create();
+
+    $workspace1->users()->attach($admin, ['role' => 'admin']);
+    $workspace1->users()->attach($userToDetach, ['role' => 'member']);
+    $workspace2->users()->attach($userToDetach, ['role' => 'member']); // User belongs to a second workspace
+
+    expect($userToDetach->workspaces()->count())->toBe(2);
+    $initialWorkspaceIds = $userToDetach->workspaces()->pluck('workspaces.id')->toArray();
+
+    // Act: Admin detaches the user from workspace1
+    $this->actingAs($admin)
+        ->deleteJson(route('open.workspaces.users.remove', ['workspaceId' => $workspace1->id, 'userId' => $userToDetach->id]))
+        ->assertOk();
+
+    // Assert: User now has 1 workspace (workspace2), no default created
+    $userToDetach->refresh();
+    expect($userToDetach->workspaces()->count())->toBe(1);
+    $remainingWorkspace = $userToDetach->workspaces()->first();
+    expect($remainingWorkspace->id)->toBe($workspace2->id);
+    expect($userToDetach->workspaces()->where('name', 'My Workspace')->exists())->toBeFalse();
+    expect($userToDetach->workspaces()->pluck('workspaces.id')->toArray())->not->toContain($workspace1->id);
+});
+
+test('user leaving last workspace creates a default one', function () {
+    // Arrange: User with one workspace
+    $workspace = Workspace::factory()->create();
+    $user = User::factory()->create();
+    $workspace->users()->attach($user, ['role' => 'member']); // Can be any role to leave
+
+    expect($user->workspaces()->count())->toBe(1);
+
+    // Act: User leaves the workspace
+    $this->actingAs($user)
+        ->postJson(route('open.workspaces.leave', ['workspaceId' => $workspace->id]))
+        ->assertOk();
+
+    // Assert: User now has 1 workspace, named 'My Workspace'
+    $user->refresh();
+    expect($user->workspaces()->count())->toBe(1);
+
+    // Check workspace name
+    $newWorkspace = $user->workspaces()->first();
+    expect($newWorkspace->name)->toBe('My Workspace');
+
+    // Check role by querying the pivot table directly
+    $pivotData = DB::table('user_workspace')
+        ->where('user_id', $user->id)
+        ->where('workspace_id', $newWorkspace->id)
+        ->first();
+    expect($pivotData->role)->toBe('admin');
+});
+
+test('user leaving one workspace when they have others does not create default one', function () {
+    // Arrange: User with two workspaces
+    $workspace1 = Workspace::factory()->create();
+    $workspace2 = Workspace::factory()->create();
+    $user = User::factory()->create();
+
+    $workspace1->users()->attach($user, ['role' => 'member']);
+    $workspace2->users()->attach($user, ['role' => 'member']);
+
+    expect($user->workspaces()->count())->toBe(2);
+
+    // Act: User leaves workspace1
+    $this->actingAs($user)
+        ->postJson(route('open.workspaces.leave', ['workspaceId' => $workspace1->id]))
+        ->assertOk();
+
+    // Assert: User now has 1 workspace (workspace2), no default created
+    $user->refresh();
+    expect($user->workspaces()->count())->toBe(1);
+    $remainingWorkspace = $user->workspaces()->first();
+    expect($remainingWorkspace->id)->toBe($workspace2->id);
+    expect($user->workspaces()->where('name', 'My Workspace')->exists())->toBeFalse();
+});
+
+// Tests for RemoveWorkspaceGuests job
+test('removing guest from last workspace via job creates a default one', function () {
+    // Arrange: Guest user with one workspace (the one they'll be removed from), Admin user
+    $admin = User::factory()->create();
+    $workspace = Workspace::factory()->create();
+    $guest = User::factory()->create();
+    $workspace->users()->attach($admin, ['role' => 'admin']);
+    $workspace->users()->attach($guest, ['role' => 'guest']);
+
+    expect($guest->workspaces()->count())->toBe(1);
+
+    // Act: Dispatch the job to remove guests from the workspace
+    // Pass the admin user as required by the job constructor
+    RemoveWorkspaceGuests::dispatchSync($admin, $workspace); // Fixed: Job expects User first, then additional param
+
+    // Assert: Guest user now has 1 workspace, named 'My Workspace'
+    $guest->refresh();
+    expect($guest->workspaces()->count())->toBe(1);
+
+    // Check workspace name
+    $newWorkspace = $guest->workspaces()->first();
+    expect($newWorkspace->name)->toBe('My Workspace');
+
+    // Check role by querying the pivot table directly
+    $pivotData = DB::table('user_workspace')
+        ->where('user_id', $guest->id)
+        ->where('workspace_id', $newWorkspace->id)
+        ->first();
+    expect($pivotData->role)->toBe('admin');
+});
+
+test('removing guest from one workspace when they have others via job does not create default one', function () {
+    // Arrange: Guest user with two workspaces, Admin user
+    $admin = User::factory()->create();
+    $workspaceToRemoveFrom = Workspace::factory()->create();
+    $otherWorkspace = Workspace::factory()->create();
+    $guest = User::factory()->create();
+
+    $workspaceToRemoveFrom->users()->attach($admin, ['role' => 'admin']);
+    $workspaceToRemoveFrom->users()->attach($guest, ['role' => 'guest']);
+    $otherWorkspace->users()->attach($guest, ['role' => 'member']); // Belongs to another workspace
+
+    expect($guest->workspaces()->count())->toBe(2);
+
+    // Act: Dispatch the job to remove guests from the first workspace
+    // Pass the admin user as required by the job constructor
+    RemoveWorkspaceGuests::dispatchSync($admin, $workspaceToRemoveFrom); // Fixed: Job expects User first, then additional param
+
+    // Assert: Guest user now has 1 workspace (the other one), no default created
+    $guest->refresh();
+    expect($guest->workspaces()->count())->toBe(1);
+    $remainingWorkspace = $guest->workspaces()->first();
+    expect($remainingWorkspace->id)->toBe($otherWorkspace->id);
+    expect($guest->workspaces()->where('name', 'My Workspace')->exists())->toBeFalse();
+});
+
+test('ensures user with no workspaces gets a default one', function () {
+    // Arrange: Create user without any workspaces
+    $user = User::factory()->create();
+    expect($user->workspaces()->count())->toBe(0);
+
+    // Create a controller that uses the trait
+    $controller = new class () {
+        use \App\Traits\EnsureUserHasWorkspace;
+
+        public function ensureWorkspace(User $user)
+        {
+            $this->ensureUserHasWorkspace($user);
+        }
+    };
+
+    // Act: Call the trait method
+    $controller->ensureWorkspace($user);
+
+    // Assert: User now has a workspace
+    $user->refresh();
+    expect($user->workspaces()->count())->toBe(1);
+
+    // Check workspace properties
+    $newWorkspace = $user->workspaces()->first();
+    expect($newWorkspace->name)->toBe('My Workspace');
+
+    // Check role
+    $pivotData = DB::table('user_workspace')
+        ->where('user_id', $user->id)
+        ->where('workspace_id', $newWorkspace->id)
+        ->first();
+    expect($pivotData->role)->toBe('admin');
+});