Block PM self-finalization and preserve approval audit trails.

When program-manager approval is off, PM-owned submits stay pending for admin review; notify admins instead of self, align reject messaging, and keep approval history readable after user deletion.

Co-authored-by: Cursor <cursoragent@cursor.com>

app/Filament/Resources/TimesheetResource.php

@@ -432,7 +432,11 @@ class TimesheetResource extends Resource
         }
 
         if ($user && $record->project?->isManagedBy($user) && $user->canApproveAsPm()) {
-            return 'Submit this timesheet for program manager approval? You will not be able to edit it until it is rejected or reverted to draft.';
+            if (Setting::programManagerApprovalRequired()) {
+                return 'Submit this timesheet for program manager approval? You will not be able to edit it until it is rejected or reverted to draft.';
+            }
+
+            return 'Submit this timesheet for admin approval? You cannot approve your own hours. You will not be able to edit it until it is rejected or reverted to draft.';
         }
 
         return 'Submit this timesheet for project manager approval? You will not be able to edit it until it is rejected or reverted to draft.';
@@ -487,13 +491,17 @@ class TimesheetResource extends Resource
             ]);
 
             if ($project && $user->canApproveAsPm() && $project->isManagedBy($user)) {
-                $record->approvalLogs()->create([
-                    'user_id' => $user->id,
-                    'action' => 'approved_pm',
-                    'comment' => 'Auto-approved as submitting project manager.',
-                ]);
-
+                // Skip the PM queue for the submitting PM (they cannot approve
+                // their own hours via the Approve action). With program-manager
+                // approval on, the next human reviewer is the program manager.
+                // With it off, leave pending_pm for an admin — never self-finalize.
                 if ($requireProgramManager) {
+                    $record->approvalLogs()->create([
+                        'user_id' => $user->id,
+                        'action' => 'approved_pm',
+                        'comment' => 'Auto-approved as submitting project manager.',
+                    ]);
+
                     $record->update(['status' => 'pending_program_manager']);
 
                     AuditLogger::log('Timesheet submitted by PM, pending Program Manager', $record, [
@@ -506,15 +514,14 @@ class TimesheetResource extends Resource
                     return;
                 }
 
-                $record->update(['status' => 'approved']);
+                $record->update(['status' => 'pending_pm']);
 
-                TimesheetNotifier::notifyApproved($record->fresh(['user', 'project']), $user);
-
-                AuditLogger::log('Timesheet submitted and approved by PM', $record, [
-                    'action' => 'approved_pm',
-                    'status' => 'approved',
+                AuditLogger::log('Timesheet submitted for admin approval (PM cannot self-approve)', $record, [
+                    'status' => 'pending_pm',
                 ]);
 
+                TimesheetNotifier::notifySubmitted($record->fresh(['user', 'project']));
+
                 return;
             }
 
@@ -645,6 +652,12 @@ class TimesheetResource extends Resource
             $record = Timesheet::query()->whereKey($record->getKey())->lockForUpdate()->firstOrFail();
             $user = auth()->user();
 
+            if ($record->isFutureWeek()) {
+                throw ValidationException::withMessages([
+                    'week_start' => 'This week has not started yet; it cannot be rejected until the week has begun.',
+                ]);
+            }
+
             if (! $user || ! $record->canBeRejectedBy($user)) {
                 abort(403, 'You are not allowed to reject this timesheet.');
             }

app/Filament/Resources/TimesheetResource/Pages/ViewTimesheet.php

@@ -104,9 +104,9 @@ class ViewTimesheet extends ViewRecord
                             ->schema([
                                 \Filament\Infolists\Components\RepeatableEntry::make('approvalLogs')
                                     ->schema([
-                                        TextEntry::make('user.name')
+                                        TextEntry::make('approver_name')
                                             ->label('By')
-                                            ->formatStateUsing(fn (?string $state): string => $state ?: '(deleted user)'),
+                                            ->getStateUsing(fn ($record): string => $record->user?->name ?? '(deleted user)'),
                                         TextEntry::make('action')
                                             ->label('Action')
                                             ->badge()

app/Support/TimesheetNotifier.php

@@ -109,7 +109,8 @@ class TimesheetNotifier
     {
         $manager = $timesheet->project?->projectManager;
 
-        if ($manager) {
+        // Segregation of duties: if the only PM is the submitter, notify admins.
+        if ($manager && $manager->id !== $timesheet->user_id) {
             return collect([$manager]);
         }
 
@@ -123,7 +124,7 @@ class TimesheetNotifier
     {
         $programManager = $timesheet->project?->programManager;
 
-        if ($programManager) {
+        if ($programManager && $programManager->id !== $timesheet->user_id) {
             return collect([$programManager]);
         }
 

tests/Feature/ApprovalLogRetentionTest.php

@@ -0,0 +1,68 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Filament\Resources\TimesheetResource;
+use App\Models\ApprovalLog;
+use App\Models\Project;
+use App\Models\Timesheet;
+use App\Models\User;
+use Carbon\Carbon;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Tests\TestCase;
+
+class ApprovalLogRetentionTest extends TestCase
+{
+    use RefreshDatabase;
+
+    public function test_approval_logs_survive_approver_user_deletion(): void
+    {
+        $employee = User::factory()->create(['role' => 'employee']);
+        $pm = User::factory()->projectManager()->create(['name' => 'Former PM']);
+        $admin = User::factory()->admin()->create();
+        $project = Project::create([
+            'code' => 'RET-01',
+            'name' => 'Retention Project',
+            'project_manager_id' => $pm->id,
+            'project_type_id' => 1,
+        ]);
+
+        $timesheet = Timesheet::create([
+            'user_id' => $employee->id,
+            'project_id' => $project->id,
+            'week_start' => Carbon::now()->startOfWeek(Carbon::MONDAY),
+            'hours' => [8, 8, 8, 8, 8, 0, 0],
+            'overtime_hours' => [0, 0, 0, 0, 0, 0, 0],
+            'status' => 'approved',
+        ]);
+
+        $log = ApprovalLog::create([
+            'timesheet_id' => $timesheet->id,
+            'user_id' => $pm->id,
+            'action' => 'approved_pm',
+            'comment' => 'Looks good',
+        ]);
+
+        $pmId = $pm->id;
+        $pm->delete();
+
+        $this->assertDatabaseMissing('users', ['id' => $pmId]);
+        $this->assertDatabaseHas('approval_logs', [
+            'id' => $log->id,
+            'timesheet_id' => $timesheet->id,
+            'user_id' => null,
+            'action' => 'approved_pm',
+            'comment' => 'Looks good',
+        ]);
+
+        $this->assertNull($log->fresh()->user_id);
+        $this->assertNull($log->fresh()->user);
+
+        $response = $this->actingAs($admin)
+            ->get(TimesheetResource::getUrl('view', ['record' => $timesheet]));
+
+        $response->assertOk();
+        $response->assertSee('Looks good', false);
+        $response->assertSee('(deleted user)', false);
+    }
+}

tests/Feature/TimesheetPmSubmissionTest.php

@@ -224,4 +224,98 @@ class TimesheetPmSubmissionTest extends TestCase
 
         $this->assertSame('pending_pm', $timesheet->fresh()->status);
     }
+
+    public function test_pm_own_submit_without_program_manager_goes_to_pending_pm_not_approved(): void
+    {
+        Setting::setValue('requireProgramManagerApproval', false);
+
+        $timesheet = Timesheet::create([
+            'user_id' => $this->pm->id,
+            'project_id' => $this->project->id,
+            'project_role' => 'Project Manager',
+            'week_start' => $this->monday,
+            'hours' => [8, 8, 8, 8, 8, 0, 0],
+            'overtime_hours' => [0, 0, 0, 0, 0, 0, 0],
+            'status' => 'draft',
+        ]);
+
+        $this->actingAs($this->pm);
+
+        $this->assertStringContainsString(
+            'admin approval',
+            TimesheetResource::submitConfirmationMessage($this->pm, $timesheet),
+        );
+
+        TimesheetResource::submitTimesheet($timesheet);
+
+        $timesheet->refresh();
+
+        $this->assertSame('pending_pm', $timesheet->status);
+        $this->assertFalse($timesheet->canBeApprovedBy($this->pm));
+        $this->assertDatabaseHas('approval_logs', [
+            'timesheet_id' => $timesheet->id,
+            'user_id' => $this->pm->id,
+            'action' => 'submitted',
+        ]);
+        $this->assertDatabaseMissing('approval_logs', [
+            'timesheet_id' => $timesheet->id,
+            'action' => 'approved_pm',
+        ]);
+    }
+
+    public function test_admin_can_approve_pm_own_timesheet_when_program_manager_off(): void
+    {
+        Setting::setValue('requireProgramManagerApproval', false);
+        $admin = User::factory()->admin()->create();
+
+        $timesheet = Timesheet::create([
+            'user_id' => $this->pm->id,
+            'project_id' => $this->project->id,
+            'project_role' => 'Project Manager',
+            'week_start' => $this->monday,
+            'hours' => [8, 8, 8, 8, 8, 0, 0],
+            'overtime_hours' => [0, 0, 0, 0, 0, 0, 0],
+            'status' => 'draft',
+        ]);
+
+        $this->actingAs($this->pm);
+        TimesheetResource::submitTimesheet($timesheet);
+
+        $this->assertSame('pending_pm', $timesheet->fresh()->status);
+
+        $this->actingAs($admin);
+        TimesheetResource::handleApprove($timesheet->fresh(), 'Admin review');
+
+        $this->assertSame('approved', $timesheet->fresh()->status);
+        $this->assertDatabaseHas('approval_logs', [
+            'timesheet_id' => $timesheet->id,
+            'user_id' => $admin->id,
+            'action' => 'approved_pm',
+        ]);
+    }
+
+    public function test_pm_own_submit_notifies_admins_not_self_when_program_manager_off(): void
+    {
+        Setting::setValue('requireProgramManagerApproval', false);
+        Setting::setValue('emailNotifications', true);
+        $admin = User::factory()->admin()->create();
+
+        $timesheet = Timesheet::create([
+            'user_id' => $this->pm->id,
+            'project_id' => $this->project->id,
+            'project_role' => 'Project Manager',
+            'week_start' => $this->monday,
+            'hours' => [8, 8, 8, 8, 8, 0, 0],
+            'overtime_hours' => [0, 0, 0, 0, 0, 0, 0],
+            'status' => 'draft',
+        ]);
+
+        $this->actingAs($this->pm);
+        Notification::fake();
+
+        TimesheetResource::submitTimesheet($timesheet);
+
+        Notification::assertSentTo($admin, TimesheetSubmittedNotification::class);
+        Notification::assertNotSentTo($this->pm, TimesheetSubmittedNotification::class);
+    }
 }