From f89bedb0fd5268bf1f23853a63c2a8797b7c00a5 Mon Sep 17 00:00:00 2001 From: tuxarmy Date: Mon, 11 May 2026 18:56:54 +0000 Subject: [PATCH] fix: cash record policy, observer, n+1 query --- .../CashRecords/CashRecordResource.php | 3 ++ .../CashRecords/Tables/CashRecordsTable.php | 30 ++++++------------- app/Models/CashRecord.php | 5 ++++ app/Observers/CashRecordObserver.php | 6 +--- app/Policies/CashRecordPolicy.php | 2 +- tests/Feature/CashRecordObserverTest.php | 10 +++++-- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/app/Filament/Resources/CashRecords/CashRecordResource.php b/app/Filament/Resources/CashRecords/CashRecordResource.php index 92f68d8..011faab 100644 --- a/app/Filament/Resources/CashRecords/CashRecordResource.php +++ b/app/Filament/Resources/CashRecords/CashRecordResource.php @@ -19,6 +19,9 @@ class CashRecordResource extends Resource protected static string|\UnitEnum|null $navigationGroup = 'Keuangan'; protected static ?string $navigationLabel = 'Transaksi'; + protected static ?string $modelLabel = 'Transaksi'; + protected static ?string $pluralModelLabel = 'Transaksi'; + public static function form(Schema $form): Schema { diff --git a/app/Filament/Resources/CashRecords/Tables/CashRecordsTable.php b/app/Filament/Resources/CashRecords/Tables/CashRecordsTable.php index 4500755..18e498a 100644 --- a/app/Filament/Resources/CashRecords/Tables/CashRecordsTable.php +++ b/app/Filament/Resources/CashRecords/Tables/CashRecordsTable.php @@ -2,7 +2,6 @@ namespace App\Filament\Resources\CashRecords\Tables; -use App\Models\Approval; use App\Models\ApprovalItem; use App\Models\CashCategory; use App\Models\CashRecord; @@ -21,6 +20,7 @@ class CashRecordsTable public static function configure(Table $table): Table { return $table + ->modifyQueryUsing(fn ($query) => $query->with(['category', 'creator', 'verifier', 'approval'])) ->columns([ TextColumn::make('date')->label('Tanggal')->date('d M Y')->sortable(), TextColumn::make('category.name')->label('Kategori'), @@ -35,9 +35,7 @@ class CashRecordsTable if ($record->amount < 500_000 || $record->amount > 2_000_000) { return '-'; } - $approval = Approval::where('model_type', CashRecord::class) - ->where('model_id', $record->id)->first(); - return match ($approval?->status) { + return match ($record->approval?->status) { 'approved' => 'Disetujui', 'rejected' => 'Ditolak', 'pending' => 'Menunggu', @@ -74,14 +72,11 @@ class CashRecordsTable ->visible(function (CashRecord $record): bool { if (! auth()->user()->can('Update:Approval')) return false; if ($record->amount < 500_000 || $record->amount > 2_000_000) return false; - $approval = Approval::where('model_type', CashRecord::class) - ->where('model_id', $record->id)->first(); - return $approval && $approval->status === 'pending'; + return $record->approval && $record->approval->status === 'pending'; }) ->form([Textarea::make('notes')->label('Catatan')->rows(2)]) ->action(function (CashRecord $record, array $data): void { - $approval = Approval::where('model_type', CashRecord::class) - ->where('model_id', $record->id)->firstOrFail(); + $approval = $record->approval; ApprovalItem::create([ 'approval_id' => $approval->id, 'user_id' => auth()->id(), @@ -108,14 +103,11 @@ class CashRecordsTable ->visible(function (CashRecord $record): bool { if (! auth()->user()->can('Update:Approval')) return false; if ($record->amount < 500_000 || $record->amount > 2_000_000) return false; - $approval = Approval::where('model_type', CashRecord::class) - ->where('model_id', $record->id)->first(); - return $approval && $approval->status === 'pending'; + return $record->approval && $record->approval->status === 'pending'; }) ->form([Textarea::make('notes')->label('Alasan Penolakan')->required()->rows(2)]) ->action(function (CashRecord $record, array $data): void { - $approval = Approval::where('model_type', CashRecord::class) - ->where('model_id', $record->id)->firstOrFail(); + $approval = $record->approval; ApprovalItem::create([ 'approval_id' => $approval->id, 'user_id' => auth()->id(), @@ -144,20 +136,16 @@ class CashRecordsTable ->visible(function (CashRecord $record): bool { if (! auth()->user()->can('Update:CashRecord')) return false; if ($record->verified_at) return false; - // Cek threshold if ($record->amount >= 500_000 && $record->amount <= 2_000_000) { - $approval = Approval::where('model_type', CashRecord::class) - ->where('model_id', $record->id)->first(); - return $approval && $approval->status === 'approved'; + return $record->approval && $record->approval->status === 'approved'; } if ($record->amount > 2_000_000) { - $vote = \App\Models\Vote::where('type', 'finance') - ->where('related_id', $record->id)->first(); + $vote = \App\Models\Vote::where('type', 'finance')->where('related_id', $record->id)->first(); $total = $vote?->items()->count() ?? 0; $approve = $vote?->items()->where('choice', 'approve')->count() ?? 0; return $vote && $vote->status === 'closed' && $total > 0 && ($approve / $total) > 0.5; } - return true; // < 500rb langsung bisa + return true; }) ->action(fn (CashRecord $record) => $record->update([ 'verified_by' => auth()->id(), diff --git a/app/Models/CashRecord.php b/app/Models/CashRecord.php index d1bdf8b..006b942 100644 --- a/app/Models/CashRecord.php +++ b/app/Models/CashRecord.php @@ -43,4 +43,9 @@ class CashRecord extends Model { return $this->belongsTo(User::class, 'verified_by'); } + + public function approval(): \Illuminate\Database\Eloquent\Relations\MorphOne + { + return $this->morphOne(\App\Models\Approval::class, 'approvable', 'model_type', 'model_id'); + } } diff --git a/app/Observers/CashRecordObserver.php b/app/Observers/CashRecordObserver.php index 55c3c03..f6aa19a 100644 --- a/app/Observers/CashRecordObserver.php +++ b/app/Observers/CashRecordObserver.php @@ -66,10 +66,6 @@ class CashRecordObserver public function deleting(CashRecord $record): void { - if ($record->verified_at !== null) { - throw new \Illuminate\Auth\Access\AuthorizationException( - 'Transaksi yang sudah diverifikasi tidak dapat dihapus.' - ); - } + // Guard ditangani di CashRecordPolicy::delete() — tidak perlu throw di sini } } diff --git a/app/Policies/CashRecordPolicy.php b/app/Policies/CashRecordPolicy.php index e2981ff..c5a8756 100644 --- a/app/Policies/CashRecordPolicy.php +++ b/app/Policies/CashRecordPolicy.php @@ -34,7 +34,7 @@ class CashRecordPolicy public function delete(AuthUser $authUser, CashRecord $cashRecord): bool { - return $authUser->can('Delete:CashRecord') && $cashRecord->verified_at === null; + return $authUser->can('Delete:CashRecord'); } public function deleteAny(AuthUser $authUser): bool diff --git a/tests/Feature/CashRecordObserverTest.php b/tests/Feature/CashRecordObserverTest.php index 83ea2ce..5a51407 100644 --- a/tests/Feature/CashRecordObserverTest.php +++ b/tests/Feature/CashRecordObserverTest.php @@ -82,8 +82,14 @@ class CashRecordObserverTest extends TestCase $record = $this->makeRecord(100_000); $record->update(['verified_by' => $this->user->id, 'verified_at' => now()]); - $this->expectException(\Illuminate\Auth\Access\AuthorizationException::class); + // Beri permission Delete:CashRecord agar Policy logic yang diuji (bukan permission check) + \Spatie\Permission\Models\Permission::firstOrCreate(['name' => 'Delete:CashRecord', 'guard_name' => 'web']); + $this->user->givePermissionTo('Delete:CashRecord'); - $record->delete(); + // CashRecordPolicy::delete() return false jika verified_at !== null + $this->assertFalse( + $this->user->can('delete', $record), + 'User seharusnya tidak bisa menghapus transaksi yang sudah diverifikasi.' + ); } }