diff --git a/app/Filament/Resources/Feedback/Pages/EditFeedback.php b/app/Filament/Resources/Feedback/Pages/EditFeedback.php index 02a0c882..5c845c83 100644 --- a/app/Filament/Resources/Feedback/Pages/EditFeedback.php +++ b/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 diff --git a/app/Filament/Resources/Feedback/Schemas/FeedbackForm.php b/app/Filament/Resources/Feedback/Schemas/FeedbackForm.php index dff43580..48fb79a0 100644 --- a/app/Filament/Resources/Feedback/Schemas/FeedbackForm.php +++ b/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')) diff --git a/app/Models/Contract.php b/app/Models/Contract.php index 059eb768..67e32638 100644 --- a/app/Models/Contract.php +++ b/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, ]; } diff --git a/app/Models/Department.php b/app/Models/Department.php index db48d411..d3aff8e5 100644 --- a/app/Models/Department.php +++ b/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'); diff --git a/app/Models/Handover.php b/app/Models/Handover.php index 2b58bab3..999faa05 100644 --- a/app/Models/Handover.php +++ b/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); diff --git a/app/Models/ProductService.php b/app/Models/ProductService.php index 5b00d560..2e6f7b2f 100644 --- a/app/Models/ProductService.php +++ b/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); diff --git a/app/Models/User.php b/app/Models/User.php index 9d328d64..dbd9b32f 100644 --- a/app/Models/User.php +++ b/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; diff --git a/app/Services/ClosingService.php b/app/Services/ClosingService.php index 7e1ba335..bb03aba2 100644 --- a/app/Services/ClosingService.php +++ b/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)); + } + } } } } diff --git a/app/Services/TransferService.php b/app/Services/TransferService.php index 6fb12c50..a739c356 100644 --- a/app/Services/TransferService.php +++ b/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)); + } } } } diff --git a/database/migrations/2026_05_20_140000_add_department_id_to_users_table.php b/database/migrations/2026_05_20_140000_add_department_id_to_users_table.php new file mode 100644 index 00000000..7827ec7f --- /dev/null +++ b/database/migrations/2026_05_20_140000_add_department_id_to_users_table.php @@ -0,0 +1,27 @@ +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'); + }); + } +}; diff --git a/lang/en/feedback.php b/lang/en/feedback.php index ba1df163..274dd469 100644 --- a/lang/en/feedback.php +++ b/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.', diff --git a/lang/vi/feedback.php b/lang/vi/feedback.php index 8ad13a38..41987678 100644 --- a/lang/vi/feedback.php +++ b/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.', diff --git a/tests/Feature/ClosingPermissionTest.php b/tests/Feature/ClosingPermissionTest.php index c0c95d8b..49d41f9f 100644 --- a/tests/Feature/ClosingPermissionTest.php +++ b/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; + } + ); +}); diff --git a/tests/Feature/EditFeedbackTest.php b/tests/Feature/EditFeedbackTest.php new file mode 100644 index 00000000..4ad78758 --- /dev/null +++ b/tests/Feature/EditFeedbackTest.php @@ -0,0 +1,72 @@ +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(); +}); diff --git a/tests/Feature/ImportCskhTest.php b/tests/Feature/ImportCskhTest.php index ccf7e87b..693c22c4 100644 --- a/tests/Feature/ImportCskhTest.php +++ b/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); diff --git a/tests/Feature/TransferFlowTest.php b/tests/Feature/TransferFlowTest.php index 17b3be02..c62a7c06 100644 --- a/tests/Feature/TransferFlowTest.php +++ b/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); +});