From ed1eddd486fe2307c2d96c91ac025ea85e745af3 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Tue, 14 Apr 2026 17:42:04 +0200 Subject: [PATCH] fix: allow organiser to approve shift assignments when shift is full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The approve() and bulkApprove() methods in ShiftAssignmentService hard-blocked with a 422 when all slots were filled. This was incorrect for organiser actions — only volunteer claims (portal self-service) should enforce capacity limits. Organiser assign() already allowed overbooking, making the approve block inconsistent. Changes: - Remove capacity hard-block from approve() and bulkApprove(), replace with audit log entry (shift.overbooked_approval) - Add overbook confirmation dialog in ShiftDetailPanel before approving a full shift (single + bulk approve) - Add onError handlers to all mutations in ShiftDetailPanel (approve, reject, cancel, bulk-approve) so errors display in the snackbar - Add global 422 validation error display in axios interceptor via useNotificationStore as safety net for all components - Add PHPUnit test for approve-when-full scenario Co-Authored-By: Claude Opus 4.6 (1M context) --- api/app/Services/ShiftAssignmentService.php | 34 +++++-- .../Api/V1/ShiftAssignmentWorkflowTest.php | 38 ++++++++ .../components/shifts/ShiftDetailPanel.vue | 94 ++++++++++++++++++- apps/app/src/lib/axios.ts | 6 +- 4 files changed, 159 insertions(+), 13 deletions(-) diff --git a/api/app/Services/ShiftAssignmentService.php b/api/app/Services/ShiftAssignmentService.php index 84557ffd..5b790da0 100644 --- a/api/app/Services/ShiftAssignmentService.php +++ b/api/app/Services/ShiftAssignmentService.php @@ -168,18 +168,26 @@ final class ShiftAssignmentService $this->validateStatusTransition($assignment, ShiftAssignmentStatus::APPROVED); $shift = $assignment->shift; + $oldStatus = $assignment->status; + + // Log overbooking for audit trail (organisers may intentionally overbook) $approvedCount = $shift->shiftAssignments() ->where('status', ShiftAssignmentStatus::APPROVED) ->count(); if ($approvedCount >= $shift->slots_total) { - throw ValidationException::withMessages([ - 'shift' => ['Shift is vol — alle slots zijn bezet.'], - ]); + activity('shift_assignment') + ->causedBy($approvedBy) + ->performedOn($shift) + ->withProperties([ + 'filled_slots' => $approvedCount, + 'slots_total' => $shift->slots_total, + 'assignment_id' => $assignment->id, + 'person_id' => $assignment->person_id, + ]) + ->log('shift.overbooked_approval'); } - $oldStatus = $assignment->status; - $assignment->status = ShiftAssignmentStatus::APPROVED; $assignment->approved_by = $approvedBy->id; $assignment->approved_at = now(); @@ -280,16 +288,22 @@ final class ShiftAssignmentService } $shift = $assignment->shift; + + // Log overbooking for audit trail $approvedCount = $shift->shiftAssignments() ->where('status', ShiftAssignmentStatus::APPROVED) ->count(); if ($approvedCount >= $shift->slots_total) { - return [ - 'assignment_id' => $assignment->id, - 'status' => 'skipped', - 'reason' => 'Shift is vol.', - ]; + activity('shift_assignment') + ->causedBy($approvedBy) + ->performedOn($shift) + ->withProperties([ + 'filled_slots' => $approvedCount, + 'slots_total' => $shift->slots_total, + 'assignment_id' => $assignment->id, + ]) + ->log('shift.overbooked_approval'); } $assignment->status = ShiftAssignmentStatus::APPROVED; diff --git a/api/tests/Feature/Api/V1/ShiftAssignmentWorkflowTest.php b/api/tests/Feature/Api/V1/ShiftAssignmentWorkflowTest.php index 102cd1c2..46ce0495 100644 --- a/api/tests/Feature/Api/V1/ShiftAssignmentWorkflowTest.php +++ b/api/tests/Feature/Api/V1/ShiftAssignmentWorkflowTest.php @@ -406,6 +406,44 @@ class ShiftAssignmentWorkflowTest extends TestCase ->assertJsonPath('data.rejection_reason', 'Onvoldoende ervaring voor deze rol.'); } + public function test_approve_allows_overbooking_when_shift_is_full(): void + { + $shift = $this->createOpenShift(['slots_total' => 1]); + + // Fill the slot + ShiftAssignment::factory()->approved()->create([ + 'shift_id' => $shift->id, + 'person_id' => Person::factory()->approved()->create([ + 'event_id' => $this->event->id, + 'crowd_type_id' => $this->crowdType->id, + ])->id, + 'time_slot_id' => $this->timeSlot->id, + ]); + + // Pending assignment to approve + $assignment = ShiftAssignment::factory()->create([ + 'shift_id' => $shift->id, + 'person_id' => $this->person->id, + 'time_slot_id' => $this->timeSlot->id, + 'status' => ShiftAssignmentStatus::PENDING_APPROVAL, + ]); + + Sanctum::actingAs($this->orgAdmin); + + $response = $this->postJson( + "/api/v1/organisations/{$this->organisation->id}/events/{$this->event->id}/shift-assignments/{$assignment->id}/approve", + ); + + $response->assertOk() + ->assertJsonPath('data.status', 'approved'); + + $this->assertDatabaseHas('shift_assignments', [ + 'id' => $assignment->id, + 'status' => ShiftAssignmentStatus::APPROVED->value, + 'approved_by' => $this->orgAdmin->id, + ]); + } + public function test_cannot_approve_already_approved(): void { $shift = $this->createOpenShift(); diff --git a/apps/app/src/components/shifts/ShiftDetailPanel.vue b/apps/app/src/components/shifts/ShiftDetailPanel.vue index 79fca85d..5655d55d 100644 --- a/apps/app/src/components/shifts/ShiftDetailPanel.vue +++ b/apps/app/src/components/shifts/ShiftDetailPanel.vue @@ -197,11 +197,38 @@ function flashFeedback(message: string, variant: 'success' | 'error' = 'success' // --- Actions --- +// Overbook confirmation +const isOverbookDialogOpen = ref(false) +const overbookingAssignment = ref(null) + function onApprove(assignment: ShiftAssignment) { + const filledSlots = props.shift?.filled_slots ?? 0 + const totalSlots = props.shift?.slots_total ?? 0 + + if (filledSlots >= totalSlots) { + overbookingAssignment.value = assignment + isOverbookDialogOpen.value = true + return + } + + executeApprove(assignment) +} + +function onOverbookConfirm() { + if (!overbookingAssignment.value) return + executeApprove(overbookingAssignment.value) + isOverbookDialogOpen.value = false + overbookingAssignment.value = null +} + +function executeApprove(assignment: ShiftAssignment) { approveAssignment(assignment.id, { onSuccess: () => { flashFeedback(`${assignment.person?.full_name ?? 'Toewijzing'} goedgekeurd`, 'success') }, + onError: (err: unknown) => { + flashFeedback(getApiErrorMessage(err, 'Fout bij goedkeuren'), 'error') + }, }) } @@ -220,6 +247,9 @@ function onCancelExecute() { cancellingAssignment.value = null flashFeedback(`${name} geannuleerd`, 'success') }, + onError: (err: unknown) => { + flashFeedback(getApiErrorMessage(err, 'Fout bij annuleren'), 'error') + }, }) } @@ -250,6 +280,9 @@ function onRejectExecute() { rejectReason.value = '' flashFeedback(`${name} afgewezen`, 'success') }, + onError: (err: unknown) => { + flashFeedback(getApiErrorMessage(err, 'Fout bij afwijzen'), 'error') + }, }, ) } @@ -261,6 +294,13 @@ const cancellingAssignment = ref(null) // Bulk approve dialog const isBulkApproveDialogOpen = ref(false) +const bulkApproveWillOverbook = computed(() => { + const filledSlots = props.shift?.filled_slots ?? 0 + const totalSlots = props.shift?.slots_total ?? 0 + const selectedCount = store.selectedAssignmentIds.length + return filledSlots + selectedCount > totalSlots +}) + function onBulkApprove() { isBulkApproveDialogOpen.value = true } @@ -274,6 +314,9 @@ function onBulkApproveExecute() { flashFeedback(`${store.selectedAssignmentIds.length} toewijzingen goedgekeurd`, 'success') store.clearSelection() }, + onError: (err: unknown) => { + flashFeedback(getApiErrorMessage(err, 'Fout bij bulk goedkeuren'), 'error') + }, }) } @@ -856,6 +899,17 @@ function fillRateColor(): string { {{ store.selectedAssignmentIds.length }} {{ store.selectedAssignmentIds.length === 1 ? 'toewijzing' : 'toewijzingen' }} wilt goedkeuren? + + + Deze actie zal de shift overbezetten + ({{ shift?.filled_slots }}/{{ shift?.slots_total }} plekken bezet). + @@ -866,11 +920,47 @@ function fillRateColor(): string { Annuleren - Goedkeuren + {{ bulkApproveWillOverbook ? 'Toch goedkeuren' : 'Goedkeuren' }} + + + + + + + + + + Shift overbezetten? + + + Deze shift is al vol + ({{ shift?.filled_slots }}/{{ shift?.slots_total }} plekken bezet). + Wil je de aanmelding van + {{ overbookingAssignment?.person?.full_name ?? 'deze persoon' }} + toch goedkeuren? + + + + + Annuleren + + + Toch goedkeuren diff --git a/apps/app/src/lib/axios.ts b/apps/app/src/lib/axios.ts index cb27dfb5..4ab70cee 100644 --- a/apps/app/src/lib/axios.ts +++ b/apps/app/src/lib/axios.ts @@ -62,7 +62,11 @@ apiClient.interceptors.response.use( notificationStore.show('The requested item was not found.', 'warning') } else if (status === 422) { - // Validation errors — pass through to calling component + // Show validation message to user; still reject so component onError handlers can react + const message = error.response?.data?.message + if (message && typeof message === 'string') { + notificationStore.show(message, 'error') + } } else if (status === 503) { notificationStore.show('Service temporarily unavailable. Please try again later.', 'error')