Refactor: Enforce type safety, fix close deadlock, implement assignee restrictions and notify admin fallbacks

app/Filament/Resources/Feedback/Pages/EditFeedback.php

@@ -177,6 +177,9 @@ class EditFeedback extends EditRecord
 
         $newStatus = $data['status'] ?? null;
         if ($newStatus === 'closed' && $this->getRecord()->status !== 'closed') {
+            // Save pending attachments first so ClosingService validation sees them
+            $this->savePendingAttachments();
+
             $closingService = App::make(ClosingService::class);
             $closingService->close($this->getRecord(), auth()->user());
         }
@@ -184,7 +187,7 @@ class EditFeedback extends EditRecord
         return $data;
     }
 
-    protected function afterSave(): void
+    protected function savePendingAttachments(): void
     {
         if (empty($this->pendingAttachments)) {
             return;
@@ -213,6 +216,13 @@ class EditFeedback extends EditRecord
                 'size' => $this->safeFileSize($disk, $path),
             ]);
         }
+
+        $this->pendingAttachments = []; // Clear to prevent double saving in afterSave
+    }
+
+    protected function afterSave(): void
+    {
+        $this->savePendingAttachments();
     }
 
     protected function safeMimeType($disk, string $path): ?string

app/Filament/Resources/Feedback/Schemas/FeedbackForm.php

@@ -59,7 +59,25 @@ class FeedbackForm
                             )
                             ->visible(fn ($get): bool => ! $get('is_general'))
                             ->nullable()
-                            ->live(),
+                            ->live()
+                            ->afterStateUpdated(function ($state, $set): void {
+                                if (empty($state)) {
+                                    $set('contract_id', null);
+                                    return;
+                                }
+                                $cp = CustomerProduct::find($state);
+                                if (! $cp) {
+                                    return;
+                                }
+                                $activeContract = \App\Models\Contract::where('product_id', $cp->product_id)
+                                    ->where('status', 'active')
+                                    ->first();
+                                if ($activeContract) {
+                                    $set('contract_id', $activeContract->id);
+                                } else {
+                                    $set('contract_id', null);
+                                }
+                            }),
 
                         Select::make('contract_id')
                             ->label(__('app.contract'))

app/Models/Contract.php

@@ -7,6 +7,9 @@ use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Database\Eloquent\Relations\HasMany;
 
+use App\Enums\ContractType;
+use App\Enums\ContractStatus;
+
 #[Fillable(['product_id', 'customer_id', 'type', 'start_date', 'end_date', 'status', 'printed_at', 'signed_status'])]
 class Contract extends Model
 {
@@ -16,6 +19,8 @@ class Contract extends Model
             'start_date' => 'date',
             'end_date' => 'date',
             'printed_at' => 'date',
+            'type' => ContractType::class,
+            'status' => ContractStatus::class,
         ];
     }
 

app/Models/Department.php

@@ -15,6 +15,11 @@ class Department extends Model
         return $this->belongsTo(User::class, 'manager_id');
     }
 
+    public function members(): HasMany
+    {
+        return $this->hasMany(User::class);
+    }
+
     public function feedbacks(): HasMany
     {
         return $this->hasMany(Feedback::class, 'current_department_id');

app/Models/Handover.php

@@ -6,9 +6,18 @@ use Illuminate\Database\Eloquent\Attributes\Fillable;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 
+use App\Enums\HandoverStatus;
+
 #[Fillable(['product_id', 'customer_id', 'handover_date', 'status', 'remark'])]
 class Handover extends Model
 {
+    protected function casts(): array
+    {
+        return [
+            'status' => HandoverStatus::class,
+            'handover_date' => 'date',
+        ];
+    }
     public function product(): BelongsTo
     {
         return $this->belongsTo(Product::class);

app/Models/ProductService.php

@@ -6,11 +6,23 @@ use Illuminate\Database\Eloquent\Attributes\Fillable;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 
+use App\Enums\ServiceType;
+use App\Enums\ContractStatus;
+
 #[Fillable(['product_id', 'service_type', 'status', 'actual_collection_date', 'remark'])]
 class ProductService extends Model
 {
     protected $table = 'product_services';
 
+    protected function casts(): array
+    {
+        return [
+            'service_type' => ServiceType::class,
+            'status' => ContractStatus::class,
+            'actual_collection_date' => 'date',
+        ];
+    }
+
     public function product(): BelongsTo
     {
         return $this->belongsTo(Product::class);

app/Models/User.php

@@ -7,6 +7,7 @@ use Illuminate\Database\Eloquent\Attributes\Fillable;
 use Illuminate\Database\Eloquent\Attributes\Hidden;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Relations\HasMany;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Foundation\Auth\User as Authenticatable;
 use Illuminate\Notifications\Notifiable;
 use Spatie\Permission\Traits\HasRoles;
@@ -14,7 +15,7 @@ use Illuminate\Contracts\Auth\Access\Authorizable;
 use Filament\Models\Contracts\FilamentUser;
 use Filament\Panel;
 
-#[Fillable(['name', 'email', 'password', 'role'])]
+#[Fillable(['name', 'email', 'password', 'role', 'department_id'])]
 #[Hidden(['password', 'remember_token'])]
 class User extends Authenticatable implements FilamentUser
 {
@@ -29,6 +30,11 @@ class User extends Authenticatable implements FilamentUser
         ];
     }
 
+    public function department(): BelongsTo
+    {
+        return $this->belongsTo(Department::class);
+    }
+
     public function canAccessPanel(Panel $panel): bool
     {
         return true;

app/Services/ClosingService.php

@@ -23,9 +23,12 @@ class ClosingService
             throw new HttpException(403, __('feedback.close_permission_denied'));
         }
 
-        if ($actor->hasRole('manager') && ! $actor->hasRole('admin')) {
-            $managedDeptIds = Department::where('manager_id', $actor->id)->pluck('id');
-            if (! $managedDeptIds->contains($feedback->current_department_id)) {
+        if (! $actor->hasRole('admin')) {
+            $managedDeptIds = Department::where('manager_id', $actor->id)->pluck('id')->toArray();
+            $actorDeptId = $actor->department_id;
+            $allowedDeptIds = array_filter(array_merge([$actorDeptId], $managedDeptIds));
+
+            if (! in_array($feedback->current_department_id, $allowedDeptIds)) {
                 throw new HttpException(403, __('feedback.close_department_only'));
             }
         }
@@ -68,13 +71,26 @@ class ClosingService
 
     protected function notifyStakeholders(Feedback $feedback, User $actor): void
     {
+        $notifiedUserIds = [];
+
         if ($feedback->assignedTo) {
             $feedback->assignedTo->notify(new \App\Notifications\TicketClosed($feedback, $actor));
+            $notifiedUserIds[] = $feedback->assignedTo->id;
         }
 
         $department = $feedback->currentDepartment;
-        if ($department && $department->manager && $department->manager->id !== ($feedback->assignedTo?->id ?? null)) {
-            $department->manager->notify(new \App\Notifications\TicketClosed($feedback, $actor));
+        if ($department && $department->manager) {
+            if (! in_array($department->manager->id, $notifiedUserIds)) {
+                $department->manager->notify(new \App\Notifications\TicketClosed($feedback, $actor));
+            }
+        } else {
+            // No manager! Notify admins
+            $admins = User::whereHas('roles', fn ($q) => $q->where('name', 'admin'))->get();
+            foreach ($admins as $admin) {
+                if (! in_array($admin->id, $notifiedUserIds)) {
+                    $admin->notify(new \App\Notifications\TicketClosed($feedback, $actor));
+                }
+            }
         }
     }
 }

app/Services/TransferService.php

@@ -24,6 +24,20 @@ class TransferService
             ]);
         }
 
+        if ($newHandlerId !== null) {
+            $handler = User::find($newHandlerId);
+            if (! $handler) {
+                throw ValidationException::withMessages([
+                    'new_assignee_id' => __('feedback.user_not_found'),
+                ]);
+            }
+            if ($handler->department_id !== $toDepartment->id && $toDepartment->manager_id !== $handler->id) {
+                throw ValidationException::withMessages([
+                    'new_assignee_id' => __('feedback.handler_not_in_department'),
+                ]);
+            }
+        }
+
         $fromDepartmentId = $feedback->current_department_id;
         $fromDepartmentName = $feedback->currentDepartment?->name ?? 'N/A';
 
@@ -67,6 +81,12 @@ class TransferService
 
         if ($manager) {
             $manager->notify(new \App\Notifications\TicketTransferred($feedback, $log, $sender));
+        } else {
+            // No manager! Notify admins
+            $admins = User::whereHas('roles', fn ($q) => $q->where('name', 'admin'))->get();
+            foreach ($admins as $admin) {
+                $admin->notify(new \App\Notifications\TicketTransferred($feedback, $log, $sender));
+            }
         }
     }
 }

database/migrations/2026_05_20_140000_add_department_id_to_users_table.php

@@ -0,0 +1,27 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+return new class extends Migration
+{
+    public function up(): void
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->foreignId('department_id')
+                ->nullable()
+                ->after('role')
+                ->constrained('departments')
+                ->nullOnDelete();
+        });
+    }
+
+    public function down(): void
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->dropForeign(['department_id']);
+            $table->dropColumn('department_id');
+        });
+    }
+};

lang/en/feedback.php

@@ -10,6 +10,8 @@ return [
 
     // TransferService
     'transfer_reason_required' => 'Please enter a transfer reason.',
+    'handler_not_in_department' => 'The selected handler does not belong to the target department.',
+    'user_not_found' => 'The selected handler was not found.',
 
     // FileService
     'file_type_not_allowed' => 'File type :type is not allowed. Allowed: jpg, png, pdf, mp4, mov.',

lang/vi/feedback.php

@@ -10,6 +10,8 @@ return [
 
     // TransferService
     'transfer_reason_required' => 'Vui lòng nhập lý do chuyển tiếp.',
+    'handler_not_in_department' => 'Người xử lý được chọn không thuộc phòng ban đích.',
+    'user_not_found' => 'Không tìm thấy người xử lý được chọn.',
 
     // FileService
     'file_type_not_allowed' => 'Loại file :type không được phép. Cho phép: jpg, png, pdf, mp4, mov.',

tests/Feature/ClosingPermissionTest.php

@@ -187,3 +187,157 @@ test('manager cannot close ticket from department they do not manage', function
 
     expect($feedback->fresh()->status)->toBe('resolved');
 });
+
+// ─── Scenario 2d: Staff with close permission cannot close ticket from other department ──
+test('staff with close permission cannot close ticket from other department', function () {
+    $staffRole = Role::create(['name' => 'staff']);
+    $staffRole->givePermissionTo(['close-ticket', 'view-feedback', 'update-feedback']);
+
+    // Staff belongs to Department A
+    $deptA = Department::create(['name' => 'CSKH']);
+    $staff = User::factory()->create(['department_id' => $deptA->id]);
+    $staff->assignRole('staff');
+
+    // Ticket belongs to Department B
+    $deptB = Department::create(['name' => 'Ky Thuat']);
+
+    $channel = FeedbackChannel::create(['name' => 'Phone', 'slug' => 'phone-staff-close']);
+    $customer = Customer::create(['name' => 'Test Customer Staff', 'email' => 'staff@test.com']);
+
+    $feedback = Feedback::create([
+        'customer_id' => $customer->id,
+        'feedback_channel_id' => $channel->id,
+        'title' => 'Test Feedback Staff',
+        'content' => 'Test content',
+        'status' => 'resolved',
+        'current_department_id' => $deptB->id,
+    ]);
+
+    FeedbackAttachment::create([
+        'uuid' => 'test-uuid-staff-1',
+        'feedback_id' => $feedback->id,
+        'user_id' => $staff->id,
+        'name' => 'proof.pdf',
+        'path' => 'feedback-attachments/proof_staff.pdf',
+        'disk' => 'local',
+        'collection' => 'proof_of_resolution',
+        'mime_type' => 'application/pdf',
+        'size' => 1024,
+    ]);
+
+    $this->actingAs($staff);
+
+    $closingService = app(ClosingService::class);
+
+    try {
+        $closingService->close($feedback, $staff);
+        $this->fail('Expected HttpException (403) but none thrown.');
+    } catch (\Symfony\Component\HttpKernel\Exception\HttpException $e) {
+        expect($e->getStatusCode())->toBe(403);
+        expect($e->getMessage())->toBe('Bạn chỉ có thể đóng phiếu thuộc phòng ban mình quản lý.');
+    }
+
+    expect($feedback->fresh()->status)->toBe('resolved');
+});
+
+// ─── Scenario 2e: Staff with close permission can close ticket from same department ────
+test('staff with close permission can close ticket from same department', function () {
+    $staffRole = Role::create(['name' => 'staff']);
+    $staffRole->givePermissionTo(['close-ticket', 'view-feedback', 'update-feedback']);
+
+    // Staff belongs to Department A
+    $deptA = Department::create(['name' => 'CSKH']);
+    $staff = User::factory()->create(['department_id' => $deptA->id]);
+    $staff->assignRole('staff');
+
+    $channel = FeedbackChannel::create(['name' => 'Phone', 'slug' => 'phone-staff-close-success']);
+    $customer = Customer::create(['name' => 'Test Customer Staff Success', 'email' => 'staff_success@test.com']);
+
+    $feedback = Feedback::create([
+        'customer_id' => $customer->id,
+        'feedback_channel_id' => $channel->id,
+        'title' => 'Test Feedback Staff Success',
+        'content' => 'Test content',
+        'status' => 'resolved',
+        'current_department_id' => $deptA->id,
+    ]);
+
+    FeedbackAttachment::create([
+        'uuid' => 'test-uuid-staff-2',
+        'feedback_id' => $feedback->id,
+        'user_id' => $staff->id,
+        'name' => 'proof.pdf',
+        'path' => 'feedback-attachments/proof_staff2.pdf',
+        'disk' => 'local',
+        'collection' => 'proof_of_resolution',
+        'mime_type' => 'application/pdf',
+        'size' => 1024,
+    ]);
+
+    $this->actingAs($staff);
+
+    $closingService = app(ClosingService::class);
+    $closingService->close($feedback, $staff);
+
+    expect($feedback->fresh()->status)->toBe('closed');
+});
+
+// ─── Scenario 2f: Notify admin when closing a department ticket with no manager ────────
+test('admin notification is sent when closing department ticket with no manager', function () {
+    \Illuminate\Support\Facades\Notification::fake();
+
+    // Create admin role and user
+    $adminRole = Role::create(['name' => 'admin']);
+    $admin = User::factory()->create();
+    $admin->assignRole('admin');
+
+    $managerRole = Role::create(['name' => 'manager']);
+    $managerRole->givePermissionTo(['close-ticket', 'view-feedback', 'update-feedback']);
+
+    // Department with no manager
+    $dept = Department::create(['name' => 'Unmanaged Dept', 'manager_id' => null]);
+
+    $manager = User::factory()->create(['department_id' => $dept->id]);
+    $manager->assignRole('manager');
+
+    $channel = FeedbackChannel::create(['name' => 'Phone', 'slug' => 'phone-no-manager']);
+    $customer = Customer::create(['name' => 'Customer', 'email' => 'no_manager@test.com']);
+
+    $feedback = Feedback::create([
+        'customer_id' => $customer->id,
+        'feedback_channel_id' => $channel->id,
+        'title' => 'Ticket',
+        'content' => 'Content',
+        'status' => 'resolved',
+        'current_department_id' => $dept->id,
+    ]);
+
+    FeedbackAttachment::create([
+        'uuid' => 'test-uuid-no-manager',
+        'feedback_id' => $feedback->id,
+        'user_id' => $manager->id,
+        'name' => 'proof.pdf',
+        'path' => 'feedback-attachments/proof3.pdf',
+        'disk' => 'local',
+        'collection' => 'proof_of_resolution',
+        'mime_type' => 'application/pdf',
+        'size' => 1024,
+    ]);
+
+    $this->actingAs($manager);
+
+    $closingService = app(ClosingService::class);
+    $closingService->close($feedback, $manager);
+
+    expect($feedback->fresh()->status)->toBe('closed');
+
+    // Assert notification sent to admin
+    \Illuminate\Support\Facades\Notification::assertSentTo(
+        $admin,
+        \App\Notifications\TicketClosed::class,
+        function ($notification) use ($feedback, $manager) {
+            return $notification->feedback->id === $feedback->id
+                && $notification->actor->id === $manager->id;
+        }
+    );
+});

tests/Feature/EditFeedbackTest.php

@@ -0,0 +1,72 @@
+<?php
+
+use App\Models\Customer;
+use App\Models\Department;
+use App\Models\Feedback;
+use App\Models\FeedbackChannel;
+use App\Models\User;
+use App\Filament\Resources\Feedback\Pages\EditFeedback;
+use Spatie\Permission\Models\Permission;
+use Spatie\Permission\Models\Role;
+use Livewire\Livewire;
+use Illuminate\Support\Facades\Storage;
+
+uses(\Illuminate\Foundation\Testing\RefreshDatabase::class);
+
+beforeEach(function () {
+    app()[\Spatie\Permission\PermissionRegistrar::class]->forgetCachedPermissions();
+
+    Permission::firstOrCreate(['name' => 'close-ticket']);
+    Permission::firstOrCreate(['name' => 'view-feedback']);
+    Permission::firstOrCreate(['name' => 'update-feedback']);
+    Permission::firstOrCreate(['name' => 'transfer-department']);
+    Permission::firstOrCreate(['name' => 'delete-feedback']);
+    Permission::firstOrCreate(['name' => 'create-feedback']);
+});
+
+test('can upload proof and close ticket in one save operation', function () {
+    Storage::fake('public');
+
+    // Create manager who has close permission
+    $managerRole = Role::create(['name' => 'manager']);
+    $managerRole->givePermissionTo(['close-ticket', 'view-feedback', 'update-feedback']);
+
+    $manager = User::factory()->create();
+    $manager->assignRole('manager');
+
+    $dept = Department::create(['name' => 'CSKH', 'manager_id' => $manager->id]);
+
+    $channel = FeedbackChannel::create(['name' => 'Email', 'slug' => 'email']);
+    $customer = Customer::create(['name' => 'Test Customer', 'email' => 'test@test.com']);
+
+    $feedback = Feedback::create([
+        'customer_id' => $customer->id,
+        'feedback_channel_id' => $channel->id,
+        'title' => 'Test Feedback',
+        'content' => 'Test content',
+        'status' => 'resolved',
+        'current_department_id' => $dept->id,
+    ]);
+
+    // Mock an uploaded file on disk
+    $path = 'feedback-attachments/mock-proof.pdf';
+    Storage::disk('public')->put($path, 'dummy content');
+
+    $this->actingAs($manager);
+
+    // Run Filament form test
+    Livewire::test(EditFeedback::class, ['record' => $feedback->id])
+        ->fillForm([
+            'status' => 'closed',
+            'attachment_collection' => 'proof_of_resolution',
+            'attachments' => [$path],
+        ])
+        ->call('save')
+        ->assertHasNoFormErrors();
+
+    // Verify feedback is closed
+    expect($feedback->fresh()->status)->toBe('closed');
+    
+    // Verify attachment exists in database
+    expect($feedback->attachments()->where('collection', 'proof_of_resolution')->exists())->toBeTrue();
+});

tests/Feature/ImportCskhTest.php

@@ -116,11 +116,11 @@ test('cskh import command processes rows correctly in dry run and active modes',
     
     $handover = Handover::where('product_id', $productA->id)->first();
     expect($handover)->not->toBeNull();
-    expect($handover->handover_date)->toBe('2026-01-01');
+    expect($handover->handover_date->format('Y-m-d'))->toBe('2026-01-01');
     
     $contract = Contract::where('product_id', $productA->id)->first();
     expect($contract)->not->toBeNull();
-    expect($contract->type)->toBe('lease'); // type is a string 'lease'
+    expect($contract->type->value)->toBe('lease'); // type is ContractType enum
 
     // Clean up temporary file
     @unlink($csvPath);

tests/Feature/TransferFlowTest.php

@@ -119,3 +119,44 @@ test('transfer without reason throws validation error', function () {
 
     expect(TicketTransferLog::count())->toBe(0);
 });
+
+// ─── Transfer to invalid handler should fail ─────────────────────────
+test('transfer to handler not in target department throws validation error', function () {
+    $managerRole = Role::create(['name' => 'manager']);
+    $managerRole->givePermissionTo(['transfer-department', 'view-feedback', 'create-feedback', 'update-feedback']);
+
+    $sender = User::factory()->create();
+    $sender->assignRole('manager');
+
+    $fromDept = Department::create(['name' => 'CSKH']);
+    $toDept = Department::create(['name' => 'Ky Thuat']);
+
+    // Handler belongs to Finance department
+    $financeDept = Department::create(['name' => 'Tai Chinh']);
+    $handler = User::factory()->create(['department_id' => $financeDept->id]);
+
+    $channel = FeedbackChannel::create(['name' => 'Phone', 'slug' => 'phone-transfer']);
+    $customer = Customer::create(['name' => 'Test', 'email' => 't@test.com']);
+
+    $feedback = Feedback::create([
+        'customer_id' => $customer->id,
+        'feedback_channel_id' => $channel->id,
+        'title' => 'Transfer To Invalid Handler',
+        'content' => 'Test',
+        'status' => 'pending',
+        'current_department_id' => $fromDept->id,
+    ]);
+
+    $this->actingAs($sender);
+
+    $transferService = app(TransferService::class);
+
+    try {
+        $transferService->transfer($feedback, $toDept, 'Chuyen sang kiem tra', $sender, $handler->id);
+        $this->fail('Expected ValidationException but none thrown.');
+    } catch (\Illuminate\Validation\ValidationException $e) {
+        expect($e->errors()['new_assignee_id'][0])->toContain('Người xử lý được chọn không thuộc phòng ban đích');
+    }
+
+    expect(TicketTransferLog::count())->toBe(0);
+});