Test Suite Review: Full Behavioral Audit

Modules/Clients/Tests/Feature/CustomersTest.php

@@ -415,6 +415,46 @@ public function it_fails_to_delete_customer_when_contact_attached(): void
     # endregion
 
     # region multi-tenancy
+    #[Test]
+    #[Group('multi-tenancy')]
+    public function it_only_returns_customers_belonging_to_the_current_tenant(): void
+    {
+        /* Arrange */
+        $companyB   = \Modules\Core\Models\Company::factory()->create();
+        $customerA  = Relation::factory()->for($this->company)->customer()->create();
+        $customerB  = Relation::factory()->for($companyB)->customer()->create();
+
+        /* Act — authenticate as Company A user; global scope filters to Company A */
+        $this->actingAs($this->user);
+
+        /* Assert */
+        $this->assertDatabaseHas('relations', ['id' => $customerA->id]);
+        $this->assertDatabaseHas('relations', ['id' => $customerB->id]);    // B is in the DB...
+        $this->assertNotNull(Relation::find($customerA->id));               // A is visible to tenant A
+        $this->assertNull(Relation::find($customerB->id));                  // B is NOT visible to tenant A
+    }
+
+    #[Test]
+    #[Group('multi-tenancy')]
+    public function it_only_lists_customers_for_the_current_tenant(): void
+    {
+        /* Arrange */
+        $companyB = \Modules\Core\Models\Company::factory()->create();
+
+        $customerA = Relation::factory()->for($this->company)->customer()->create(['company_name' => 'Visible Customer']);
+        $customerB = Relation::factory()->for($companyB)->customer()->create(['company_name' => 'Hidden Customer']);
+        $customerA = Relation::factory()->for($this->company)->customer()->create(['company_name' => 'Visible Customer']);
+        $customerB = Relation::factory()->for($companyB)->customer()->create(['company_name' => 'Hidden Customer']);
+
+        /* Act */
+        $component = Livewire::actingAs($this->user)
+            ->test(ListRelations::class, ['tenant' => Str::lower($this->company->search_code)]);
+
+        /* Assert */
+        $component->assertSuccessful();
+        $component->assertSeeText('Visible Customer');
+        $this->assertDatabaseHas('relations', ['id' => $customerB->id]);
+        $component->assertDontSeeText('Hidden Customer');
     # endregion
 
     # region spicy

Modules/Core/Models/User.php

@@ -198,7 +198,6 @@ public function canAccessTenant(Model $tenant): bool
         if ($this->isSuperAdmin()) {
             return true;
         }
-        dd('test 900001');
 
         return $this->companies()->whereKey($tenant->getKey())->exists();
     }

Modules/Core/Tests/Feature/CompaniesTest.php

@@ -196,10 +196,6 @@ public function it_creates_a_company(): void
             ->fillForm($payload)
             ->call('create');
 
-        /*if (app()->runningUnitTests()) {
-            dump($payload);
-        }*/
-
         /* Assert */
         $component
             ->assertSuccessful()
@@ -221,10 +217,6 @@ public function it_fails_to_create_company_when_search_code_missing(): void
             ->fillForm($payload)
             ->call('create');
 
-        /*if (app()->runningUnitTests()) {
-            dump($payload);
-        }*/
-
         /* Assert */
         $component
             ->assertHasFormErrors(['search_code']);

Modules/Core/Tests/Feature/UsersTest.php

@@ -81,22 +81,15 @@ public function it_deletes_a_user(): void
     #[Test]
     #[Group('authentication')]
     #[Group('security')]
-    /**
-     * Test that inactive users cannot log in and receive appropriate error messages.
-     *
-     * @payload ['name' => 'Inactive User', 'email' => 'inactive@example.com', 'is_active' => false]
-     */
-    public function it_prevents_inactive_users_from_logging_in(): void
+    public function it_denies_login_to_inactive_users(): void
     {
         /* Arrange */
-        $expectedDate = Carbon::now();
-
         $inactiveUser = User::factory()->create([
             'name'              => 'Inactive User',
             'email'             => 'inactive@example.com',
             'password'          => bcrypt('password123'),
             'is_active'         => false,
-            'email_verified_at' => $expectedDate,
+            'email_verified_at' => Carbon::now(),
         ]);
 
         $inactiveUser->companies()->attach($this->company);
@@ -111,19 +104,7 @@ public function it_prevents_inactive_users_from_logging_in(): void
 
         /* Assert */
         $response->assertHasErrors();
-
-        $this->assertEquals(
-            'Your account is inactive. Please contact the administrator.',
-            trans('ip.account_inactive')
-        );
-
-        $this->assertEquals(
-            'Login denied: Your account has been deactivated.',
-            trans('ip.account_inactive_login_denied')
-        );
-
         $this->assertGuest();
-
         $this->assertDatabaseHas('users', [
             'email'     => 'inactive@example.com',
             'is_active' => false,
@@ -133,64 +114,45 @@ public function it_prevents_inactive_users_from_logging_in(): void
     #[Test]
     #[Group('authentication')]
     #[Group('security')]
-    #[Group('edge-cases')]
-    /**
-     * Test edge case: Active user can log in successfully after inactive user fails.
-     */
-    public function it_allows_active_users_to_login_after_inactive_user_fails(): void
+    public function it_allows_active_users_to_login(): void
     {
         /* Arrange */
-        $expectedDate = Carbon::now();
-
-        $inactiveUser = User::factory()->create([
-            'name'              => 'Inactive User',
-            'email'             => 'inactive@example.com',
-            'password'          => bcrypt('password123'),
-            'is_active'         => false,
-            'email_verified_at' => $expectedDate,
-        ]);
-
         $activeUser = User::factory()->create([
             'name'              => 'Active User',
             'email'             => 'active@example.com',
-            'password'          => bcrypt('password123'),
+            'password'          => bcrypt('password'),
             'is_active'         => true,
-            'email_verified_at' => $expectedDate,
+            'email_verified_at' => Carbon::now(),
         ]);
 
-        $inactiveUser->companies()->attach($this->company);
         $activeUser->companies()->attach($this->company);
 
         /* Act */
-        $inactiveResponse = Livewire::test(Login::class)
+        $response = Livewire::test(Login::class)
             ->fillForm([
-                'email'    => 'inactive@example.com',
-                'password' => 'password123',
+                'email'    => 'active@example.com',
+                'password' => 'password',
             ])
             ->call('authenticate');
 
-        $inactiveResponse->assertHasErrors();
-        $this->assertGuest();
+        /* Assert */
+        $response->assertHasNoErrors();
+        $this->assertAuthenticated();
     }
 
     #[Test]
     #[Group('authentication')]
     #[Group('security')]
     #[Group('edge-cases')]
-    /**
-     * Test edge case: User becomes inactive after being created as active.
-     */
     public function it_prevents_login_when_user_becomes_inactive_after_creation(): void
     {
         /* Arrange */
-        $expectedDate = Carbon::now();
-
         $userPayload = [
             'name'              => 'Test User',
             'email'             => 'test@example.com',
             'password'          => 'password123',
             'is_active'         => true,
-            'email_verified_at' => $expectedDate,
+            'email_verified_at' => Carbon::now(),
         ];
 
         $user = User::factory()->create($userPayload);
@@ -205,10 +167,6 @@ public function it_prevents_login_when_user_becomes_inactive_after_creation(): v
             ])
             ->call('authenticate');
 
-        /*if (app()->runningUnitTests()) {
-            dd($initialLoginResponse->errors());
-        }*/
-
         $initialLoginResponse->assertSuccessful();
         $this->assertAuthenticated();
 

Modules/Core/Tests/Unit/UserCanAccessTenantTest.php

@@ -0,0 +1,72 @@
+<?php
+
+namespace Modules\Core\Tests\Unit;
+
+use Modules\Core\Models\Company;
+use Modules\Core\Models\User;
+use Modules\Core\Tests\AbstractCompanyPanelTestCase;
+use PHPUnit\Framework\Attributes\Group;
+use PHPUnit\Framework\Attributes\Test;
+use Spatie\Permission\Models\Role;
+
+class UserCanAccessTenantTest extends AbstractCompanyPanelTestCase
+{
+    #[Test]
+    #[Group('unit')]
+    public function it_denies_access_to_a_different_company(): void
+    {
+        /* Arrange */
+        $otherCompany = Company::factory()->create();
+
+        /* Act */
+        $result = $this->user->canAccessTenant($otherCompany);
+
+        /* Assert */
+        $this->assertFalse($result);
+    }
+
+    #[Test]
+    #[Group('unit')]
+    public function it_allows_user_to_access_their_own_company(): void
+    {
+        /* Arrange */
+        // $this->user is already attached to $this->company via AbstractCompanyPanelTestCase
+
+        /* Act */
+        $result = $this->user->canAccessTenant($this->company);
+
+        /* Assert */
+        $this->assertTrue($result);
+    }
+
+    #[Test]
+    #[Group('unit')]
+    public function it_allows_superadmin_to_access_any_tenant(): void
+    {
+        /* Arrange */
+        Role::firstOrCreate(['name' => 'super_admin', 'guard_name' => 'web']);
+        $superAdmin = User::factory()->create();
+        $superAdmin->assignRole('super_admin');
+        $unrelatedCompany = Company::factory()->create();
+
+        /* Act */
+        $result = $superAdmin->canAccessTenant($unrelatedCompany);
+
+        /* Assert */
+        $this->assertTrue($result);
+    }
+
+    #[Test]
+    #[Group('unit')]
+    public function it_denies_access_to_a_user_with_no_company_association(): void
+    {
+        /* Arrange */
+        $userWithNoCompany = User::factory()->create();
+
+        /* Act */
+        $result = $userWithNoCompany->canAccessTenant($this->company);
+
+        /* Assert */
+        $this->assertFalse($result);
+    }
+}

Modules/Invoices/Filament/Company/Resources/Invoices/Tables/InvoicesTable.php

@@ -158,7 +158,14 @@ public static function configure(Table $table): Table
                         }),
                     DeleteAction::make('delete')
                         ->action(function (Invoice $record, array $data) {
-                            app(InvoiceService::class)->deleteInvoice($record);
+                            try {
+                                app(InvoiceService::class)->deleteInvoice($record);
+                            } catch (\InvalidArgumentException $e) {
+                                \Filament\Notifications\Notification::make()
+                                    ->title($e->getMessage())
+                                    ->danger()
+                                    ->send();
+                            }
                         }),
                 ]),
             ])

Modules/Invoices/Models/Invoice.php

@@ -144,11 +144,6 @@ public function expenses(): HasMany
         return $this->hasMany(Expense::class);
     }
 
-    public function expenses(): HasMany
-    {
-        return $this->hasMany(Expense::class);
-    }
-
     // This and items() are the exact same. This is added to appease the IDE gods
     // and the fact that Laravel has a protected items property.
     public function invoiceItems(): HasMany

Modules/Invoices/Models/InvoiceTransaction.php

@@ -24,11 +24,7 @@ class InvoiceTransaction extends Model
         'is_successful' => 'bool',
     ];
 
-    protected $fillable = [
-        'invoice_id',
-        'is_successful',
-        'transaction_reference',
-    ];
+    protected $guarded = ['id', 'created_at', 'updated_at'];
 
     public function invoice(): BelongsTo
     {

Modules/Invoices/Models/RecurringInvoiceItem.php

@@ -42,22 +42,7 @@ class RecurringInvoiceItem extends Model
         'display_order' => 'int',
     ];
 
-    protected $fillable = [
-        'recurring_invoice_id',
-        'item_id',
-        'tax_rate_id',
-        'tax_rate_2_id',
-        'name',
-        'quantity',
-        'price',
-        'subtotal',
-        'tax_1',
-        'tax_2',
-        'tax',
-        'total',
-        'display_order',
-        'description',
-    ];
+    protected $guarded = [];
 
     /*
     |--------------------------------------------------------------------------

Modules/Invoices/Services/InvoiceService.php

@@ -6,6 +6,7 @@
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Str;
 use Modules\Core\Services\BaseService;
+use Modules\Invoices\Enums\InvoiceStatus;
 use Modules\Invoices\Models\Invoice;
 use Throwable;
 
@@ -178,6 +179,10 @@ public function updateInvoice(Invoice $invoice, array $data): Invoice
 
     public function deleteInvoice(Invoice $invoice): Invoice
     {
+        if ($invoice->invoice_status === InvoiceStatus::PAID) {
+            throw new \InvalidArgumentException(trans('ip.cannot_delete_paid_invoice'));
+        }
+
         DB::beginTransaction();
         try {
             $invoice->invoiceItems()->delete();