Fixed bugs in current organization; Add database consistency checks; Add foreign key

app/Console/Commands/SelfHost/SelfHostDatabaseConsistency.php

@@ -0,0 +1,123 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Console\Commands\SelfHost;
+
+use Illuminate\Console\Command;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Database\Query\JoinClause;
+use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
+
+class SelfHostDatabaseConsistency extends Command
+{
+    /**
+     * The name and signature of the console command.
+     *
+     * @var string
+     */
+    protected $signature = 'self-host:database-consistency';
+
+    /**
+     * The console command description.
+     *
+     * @var string
+     */
+    protected $description = '';
+
+    /**
+     * Execute the console command.
+     */
+    public function handle(): int
+    {
+        $hadAProblem = false;
+
+        // Task need to be part of project in time entries
+        $problems = DB::table('time_entries')
+            ->select(['time_entries.id as id'])
+            ->join('tasks', 'time_entries.task_id', '=', 'tasks.id')
+            ->where('tasks.project_id', '!=', DB::raw('time_entries.project_id'))
+            ->get();
+        $this->logProblems($problems, 'Time entries have a task that does not belong to the project of the time entry', $hadAProblem);
+
+        // Client id is the client id of the project
+        $problems = DB::table('time_entries')
+            ->select(['time_entries.id as id'])
+            ->join('projects', 'time_entries.project_id', '=', 'projects.id')
+            ->where(DB::raw('coalesce(projects.client_id::varchar, \'\')'), '!=', DB::raw('coalesce(time_entries.client_id::varchar, \'\')'))
+            ->get();
+        $this->logProblems($problems, 'Time entries have a client that does not match the client of the project', $hadAProblem);
+
+        // Client id can only be not null if the project id is not null
+        $problems = DB::table('time_entries')
+            ->select(['time_entries.id as id'])
+            ->whereNotNull('client_id')
+            ->whereNull('project_id')
+            ->get();
+        $this->logProblems($problems, 'Time entries have a client but no project', $hadAProblem);
+
+        // Every user needs to be a member of at least one organization
+        $problems = DB::table('users')
+            ->select(['users.id as id'])
+            ->leftJoin('members', 'users.id', '=', 'members.user_id')
+            ->whereNull('members.id')
+            ->get();
+        $this->logProblems($problems, 'Users are not member of any organization', $hadAProblem);
+
+        // Every organization needs at least an owner
+        $problems = DB::table('organizations')
+            ->select(['organizations.id as id'])
+            ->leftJoin('members', function (JoinClause $join): void {
+                $join->on('organizations.id', '=', 'members.organization_id')
+                    ->where('members.role', '=', 'owner');
+            })
+            ->whereNull('members.id')
+            ->get();
+        $this->logProblems($problems, 'Organizations without an owner', $hadAProblem);
+
+        // Every member can only have one running time entry
+        $problems = DB::table('time_entries')
+            ->select(['user_id as id'])
+            ->whereNull('end')
+            ->groupBy('user_id')
+            ->havingRaw('count(*) > 1')
+            ->get(['user_id', DB::raw('count(*) as count')]);
+        $this->logProblems($problems, 'Users with more than one running time entry', $hadAProblem);
+
+        // Users have a current organization that they are not a member of
+        $problems = DB::table('users')
+            ->select(['users.id as id'])
+            ->whereNotNull('current_team_id')
+            ->whereNotIn('current_team_id', function (Builder $query): void {
+                $query->select('organization_id')
+                    ->from('members')
+                    ->whereColumn('members.user_id', 'users.id');
+            })->get();
+        $this->logProblems($problems, 'Users have a current organization that they are not a member of', $hadAProblem);
+
+        return $hadAProblem ? self::FAILURE : self::SUCCESS;
+    }
+
+    /**
+     * @param  Collection<int, \stdClass>  $problems
+     */
+    private function logProblems(Collection $problems, string $message, bool &$hadAProblem): void
+    {
+        $message = 'Consistency problem: '.$message;
+        if ($problems->isNotEmpty()) {
+            $ids = $problems->pluck('id');
+            $hadAProblem = true;
+            Log::error($message, [
+                'ids' => $ids,
+            ]);
+
+            $error = $message;
+            foreach ($ids as $id) {
+                $error .= "\n  - ".$id;
+            }
+            $this->error($error);
+        }
+    }
+}

app/Console/Kernel.php

@@ -25,6 +25,10 @@ class Kernel extends ConsoleKernel
         $schedule->command('self-host:telemetry')
             ->when(fn (): bool => config('scheduling.tasks.self_hosting_telemetry'))
             ->twiceDaily();
+
+        $schedule->command('self-host:database-consistency')
+            ->when(fn (): bool => config('scheduling.tasks.self_hosting_database_consistency'))
+            ->twiceDaily();
     }
 
     /**

app/Events/DatabaseSeederAfterSeed.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Events;
+
+use Illuminate\Foundation\Events\Dispatchable;
+
+class DatabaseSeederAfterSeed
+{
+    use Dispatchable;
+
+    public function __construct() {}
+}

app/Events/DatabaseSeederBeforeDelete.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Events;
+
+use Illuminate\Foundation\Events\Dispatchable;
+
+class DatabaseSeederBeforeDelete
+{
+    use Dispatchable;
+
+    public function __construct() {}
+}

app/Service/DeletionService.php

@@ -100,12 +100,18 @@ class DeletionService
 
         // Make sure all users have at least one organization and delete placeholders
         foreach ($users as $user) {
+            /** @var User $user */
             if ($ignoreUser !== null && $user->is($ignoreUser)) {
                 continue;
             }
             if ($user->is_placeholder) {
                 $user->delete();
             } else {
+                if ($user->current_team_id === $organization->getKey()) {
+                    $user->currentOrganization()->disassociate();
+                    $user->save();
+                }
+
                 $this->userService->makeSureUserHasAtLeastOneOrganization($user);
                 $this->userService->makeSureUserHasCurrentOrganization($user);
             }

app/Service/MemberService.php

@@ -164,6 +164,11 @@ class MemberService
     public function makeMemberToPlaceholder(Member $member, bool $makeSureUserHasAtLeastOneOrganization = true): void
     {
         $user = $member->user;
+        if ($user->current_team_id === $member->organization_id) {
+            $user->currentTeam()->disassociate();
+            $user->save();
+        }
+
         $placeholderUser = $user->replicate();
         $placeholderUser->is_placeholder = true;
         $placeholderUser->save();
@@ -175,6 +180,7 @@ class MemberService
         $this->userService->assignOrganizationEntitiesToDifferentUser($member->organization, $user, $placeholderUser);
         if ($makeSureUserHasAtLeastOneOrganization) {
             $this->userService->makeSureUserHasAtLeastOneOrganization($user);
+            $this->userService->makeSureUserHasCurrentOrganization($user);
         }
     }
 }

app/Service/UserService.php

@@ -114,13 +114,15 @@ class UserService
 
     public function makeSureUserHasCurrentOrganization(User $user): void
     {
-        if ($user->currentOrganization !== null) {
+        if ($user->current_team_id !== null) {
             return;
         }
 
         $organization = $user->organizations()->first();
-        $user->currentOrganization()->associate($organization);
+        if ($organization !== null) {
-        $user->save();
+            $user->currentOrganization()->associate($organization);
+            $user->save();
+        }
     }
 
     /**

config/scheduling.php

@@ -8,5 +8,6 @@ return [
         'time_entry_send_still_running_mails' => (bool) env('SCHEDULING_TASK_TIME_ENTRY_SEND_STILL_RUNNING_MAILS', true),
         'self_hosting_check_for_update' => (bool) env('SCHEDULING_TASK_SELF_HOSTING_CHECK_FOR_UPDATE', true),
         'self_hosting_telemetry' => (bool) env('SCHEDULING_TASK_SELF_HOSTING_TELEMETRY', true),
+        'self_hosting_database_consistency' => (bool) env('SCHEDULING_TASK_SELF_HOSTING_DATABASE_CONSISTENCY', false),
     ],
 ];

database/migrations/2025_05_16_075757_add_foreign_key_for_current_team_id_in_users_table.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+return new class extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        DB::statement('
+            update users
+            set current_team_id = null
+            where id in (
+                select users.id from users
+                left join organizations on users.current_team_id = organizations.id
+                where users.current_team_id is not null and organizations.id is null
+            )
+        ');
+        Schema::table('users', function (Blueprint $table): void {
+            $table->foreign('current_team_id', 'organizations_current_organization_id_foreign')
+                ->references('id')
+                ->on('organizations')
+                ->onDelete('restrict')
+                ->onUpdate('cascade');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     */
+    public function down(): void
+    {
+        Schema::table('users', function (Blueprint $table): void {
+            $table->dropForeign('organizations_current_organization_id_foreign');
+        });
+    }
+};

database/seeders/DatabaseSeeder.php

@@ -5,6 +5,8 @@ declare(strict_types=1);
 namespace Database\Seeders;
 
 use App\Enums\Role;
+use App\Events\DatabaseSeederAfterSeed;
+use App\Events\DatabaseSeederBeforeDelete;
 use App\Models\Audit;
 use App\Models\Client;
 use App\Models\Member;
@@ -184,10 +186,13 @@ class DatabaseSeeder extends Seeder
             'email' => 'admin@example.com',
         ]);
 
+        DatabaseSeederAfterSeed::dispatch();
     }
 
     private function deleteAll(): void
     {
+        DatabaseSeederBeforeDelete::dispatch();
+
         // Laravel Passport tables
         DB::table((new RefreshToken)->getTable())->delete();
         DB::table((new Token)->getTable())->delete();
@@ -213,6 +218,9 @@ class DatabaseSeeder extends Seeder
         DB::table((new Client)->getTable())->delete();
         DB::table((new Member)->getTable())->delete();
         DB::table((new OrganizationInvitation)->getTable())->delete();
+        DB::table((new User)->getTable())->update([
+            'current_team_id' => null,
+        ]);
         DB::table((new Organization)->getTable())->delete();
         DB::table((new User)->getTable())->delete();
     }

tests/Unit/Console/Commands/SelfHost/SelfHostDatabaseConsistencyCommandTest.php

@@ -0,0 +1,163 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\Console\Commands\SelfHost;
+
+use App\Console\Commands\SelfHost\SelfHostDatabaseConsistency;
+use App\Enums\Role;
+use App\Models\Client;
+use App\Models\Organization;
+use App\Models\Project;
+use App\Models\Task;
+use App\Models\TimeEntry;
+use App\Models\User;
+use Illuminate\Console\Command;
+use Illuminate\Support\Facades\Artisan;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+use Tests\TestCaseWithDatabase;
+
+#[CoversClass(SelfHostDatabaseConsistency::class)]
+#[UsesClass(SelfHostDatabaseConsistency::class)]
+class SelfHostDatabaseConsistencyCommandTest extends TestCaseWithDatabase
+{
+    public function test_checks_that_task_need_to_be_part_of_project_in_time_entries(): void
+    {
+        // Arrange
+        $user = $this->createUserWithRole(Role::Owner);
+        $project1 = Project::factory()->forOrganization($user->organization)->create();
+        $project2 = Project::factory()->forOrganization($user->organization)->create();
+        $task = Task::factory()->forOrganization($user->organization)->forProject($project1)->create();
+        $timeEntry = TimeEntry::factory()->forMember($user->member)->forTask($task)->forProject($project2)->create();
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Time entries have a task that does not belong to the project of the time entry\n  - ".$timeEntry->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_client_id_is_the_client_id_of_the_project(): void
+    {
+        // Arrange
+        $user = $this->createUserWithRole(Role::Owner);
+        $client1 = Client::factory()->forOrganization($user->organization)->create();
+        $client2 = Client::factory()->forOrganization($user->organization)->create();
+        $project = Project::factory()->forOrganization($user->organization)->forClient($client1)->create();
+        $timeEntry = TimeEntry::factory()->forMember($user->member)->forProject($project)->create([
+            'client_id' => $client2->id,
+        ]);
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Time entries have a client that does not match the client of the project\n  - ".$timeEntry->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_client_id_is_the_client_id_of_the_project_with_no_client_in_time_entry(): void
+    {
+        // Arrange
+        $user = $this->createUserWithRole(Role::Owner);
+        $client1 = Client::factory()->forOrganization($user->organization)->create();
+        $client2 = Client::factory()->forOrganization($user->organization)->create();
+        $project = Project::factory()->forOrganization($user->organization)->forClient($client1)->create();
+        $timeEntry = TimeEntry::factory()->forMember($user->member)->forProject($project)->create([
+            'client_id' => null,
+        ]);
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Time entries have a client that does not match the client of the project\n  - ".$timeEntry->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_client_id_is_only_null_if_project_is_also_null(): void
+    {
+        // Arrange
+        $user = $this->createUserWithRole(Role::Owner);
+        $client1 = Client::factory()->forOrganization($user->organization)->create();
+        $project = Project::factory()->forOrganization($user->organization)->forClient($client1)->create();
+        $timeEntry = TimeEntry::factory()->forMember($user->member)->create([
+            'client_id' => $client1->getKey(),
+        ]);
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Time entries have a client but no project\n  - ".$timeEntry->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_every_user_needs_to_be_a_member_of_at_least_one_organization(): void
+    {
+        // Arrange
+        $user = User::factory()->create();
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Users are not member of any organization\n  - ".$user->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_every_organization_needs_at_least_an_owner(): void
+    {
+        // Arrange
+        $user = $this->createUserWithRole(Role::Owner);
+        $organization = Organization::factory()->withOwner($user->user)->create();
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Organizations without an owner\n  - ".$organization->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_every_member_can_only_have_one_running_time_entry(): void
+    {
+        // Arrange
+        $user = $this->createUserWithRole(Role::Owner);
+        $timeEntry1 = TimeEntry::factory()->forMember($user->member)->active()->create();
+        $timeEntry2 = TimeEntry::factory()->forMember($user->member)->active()->create();
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Users with more than one running time entry\n  - ".$user->user->getKey()."\n", $output);
+    }
+
+    public function test_checks_that_users_have_a_current_organization_that_they_are_not_a_member_of(): void
+    {
+        // Arrange
+        $user1 = $this->createUserWithRole(Role::Owner);
+        $user2 = $this->createUserWithRole(Role::Owner);
+        $user1->user->currentOrganization()->associate($user2->organization);
+        $user1->user->save();
+
+        // Act
+        $exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
+
+        // Assert
+        $this->assertSame(Command::FAILURE, $exitCode);
+        $output = Artisan::output();
+        $this->assertSame("Consistency problem: Users have a current organization that they are not a member of\n  - ".$user1->user->getKey()."\n", $output);
+    }
+}

tests/Unit/Service/DeletionServiceTest.php

@@ -150,6 +150,24 @@ class DeletionServiceTest extends TestCaseWithDatabase
         $this->assertSame($specialCase ? 7 : 6, TimeEntry::query()->whereBelongsTo($organization, 'organization')->count());
     }
 
+    public function test_delete_organization_resets_the_current_organization_of_users_that_had_the_deleted_organization_as_current_organization(): void
+    {
+        // Arrange
+        $userOwner = User::factory()->create();
+        $organization = Organization::factory()->withOwner($userOwner)->create();
+        $userOwner->currentOrganization()->associate($organization);
+        $userOwner->save();
+
+        // Act
+        $this->deletionService->deleteOrganization($organization);
+
+        // Assert
+        $this->assertOrganizationDeleted($organization);
+        $userOwner->refresh();
+        $this->assertNull($userOwner->current_team_id);
+        $this->assertNotSame($organization->id, $userOwner->current_team_id);
+    }
+
     public function test_delete_organization_deletes_all_resources_of_the_organization_but_does_not_delete_other_resources(): void
     {
         // Arrange

tests/Unit/Service/MemberServiceTest.php

@@ -114,6 +114,41 @@ class MemberServiceTest extends TestCaseWithDatabase
         $this->assertSame(1, $otherUser->organizations()->count());
     }
 
+    public function test_make_member_to_placeholder_resets_current_organization_of_user_if_user_is_no_longer_member_to_newly_created_organization(): void
+    {
+        // Arrange
+        $organization = Organization::factory()->create();
+        $user = User::factory()->forCurrentOrganization($organization)->create();
+        $member = Member::factory()->forOrganization($organization)->forUser($user)->role(Role::Employee)->create();
+
+        // Act
+        $this->memberService->makeMemberToPlaceholder($member);
+
+        // Assert
+        $user->refresh();
+        $this->assertNotNull($user->current_team_id);
+        $this->assertNotSame($organization->id, $user->current_team_id);
+    }
+
+    public function test_make_member_to_placeholder_resets_current_organization_of_user_if_user_is_no_longer_member_to_already_existing_other_organization(): void
+    {
+        // Arrange
+        $organization = Organization::factory()->create();
+        $user = User::factory()->forCurrentOrganization($organization)->create();
+        $member = Member::factory()->forOrganization($organization)->forUser($user)->role(Role::Employee)->create();
+
+        $otherOrganization = Organization::factory()->create();
+        $otherMember = Member::factory()->forOrganization($otherOrganization)->forUser($user)->role(Role::Employee)->create();
+
+        // Act
+        $this->memberService->makeMemberToPlaceholder($member);
+
+        // Assert
+        $user->refresh();
+        $this->assertNotNull($user->current_team_id);
+        $this->assertSame($otherOrganization->id, $user->current_team_id);
+    }
+
     public function test_assign_organization_entities_to_different_member_without_any_entries(): void
     {
         // Arrange