fix: allow organiser to approve shift assignments when shift is full
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) <noreply@anthropic.com>
This commit is contained in:
@@ -168,18 +168,26 @@ final class ShiftAssignmentService
|
|||||||
$this->validateStatusTransition($assignment, ShiftAssignmentStatus::APPROVED);
|
$this->validateStatusTransition($assignment, ShiftAssignmentStatus::APPROVED);
|
||||||
|
|
||||||
$shift = $assignment->shift;
|
$shift = $assignment->shift;
|
||||||
|
$oldStatus = $assignment->status;
|
||||||
|
|
||||||
|
// Log overbooking for audit trail (organisers may intentionally overbook)
|
||||||
$approvedCount = $shift->shiftAssignments()
|
$approvedCount = $shift->shiftAssignments()
|
||||||
->where('status', ShiftAssignmentStatus::APPROVED)
|
->where('status', ShiftAssignmentStatus::APPROVED)
|
||||||
->count();
|
->count();
|
||||||
|
|
||||||
if ($approvedCount >= $shift->slots_total) {
|
if ($approvedCount >= $shift->slots_total) {
|
||||||
throw ValidationException::withMessages([
|
activity('shift_assignment')
|
||||||
'shift' => ['Shift is vol — alle slots zijn bezet.'],
|
->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->status = ShiftAssignmentStatus::APPROVED;
|
||||||
$assignment->approved_by = $approvedBy->id;
|
$assignment->approved_by = $approvedBy->id;
|
||||||
$assignment->approved_at = now();
|
$assignment->approved_at = now();
|
||||||
@@ -280,16 +288,22 @@ final class ShiftAssignmentService
|
|||||||
}
|
}
|
||||||
|
|
||||||
$shift = $assignment->shift;
|
$shift = $assignment->shift;
|
||||||
|
|
||||||
|
// Log overbooking for audit trail
|
||||||
$approvedCount = $shift->shiftAssignments()
|
$approvedCount = $shift->shiftAssignments()
|
||||||
->where('status', ShiftAssignmentStatus::APPROVED)
|
->where('status', ShiftAssignmentStatus::APPROVED)
|
||||||
->count();
|
->count();
|
||||||
|
|
||||||
if ($approvedCount >= $shift->slots_total) {
|
if ($approvedCount >= $shift->slots_total) {
|
||||||
return [
|
activity('shift_assignment')
|
||||||
'assignment_id' => $assignment->id,
|
->causedBy($approvedBy)
|
||||||
'status' => 'skipped',
|
->performedOn($shift)
|
||||||
'reason' => 'Shift is vol.',
|
->withProperties([
|
||||||
];
|
'filled_slots' => $approvedCount,
|
||||||
|
'slots_total' => $shift->slots_total,
|
||||||
|
'assignment_id' => $assignment->id,
|
||||||
|
])
|
||||||
|
->log('shift.overbooked_approval');
|
||||||
}
|
}
|
||||||
|
|
||||||
$assignment->status = ShiftAssignmentStatus::APPROVED;
|
$assignment->status = ShiftAssignmentStatus::APPROVED;
|
||||||
|
|||||||
@@ -406,6 +406,44 @@ class ShiftAssignmentWorkflowTest extends TestCase
|
|||||||
->assertJsonPath('data.rejection_reason', 'Onvoldoende ervaring voor deze rol.');
|
->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
|
public function test_cannot_approve_already_approved(): void
|
||||||
{
|
{
|
||||||
$shift = $this->createOpenShift();
|
$shift = $this->createOpenShift();
|
||||||
|
|||||||
@@ -197,11 +197,38 @@ function flashFeedback(message: string, variant: 'success' | 'error' = 'success'
|
|||||||
|
|
||||||
// --- Actions ---
|
// --- Actions ---
|
||||||
|
|
||||||
|
// Overbook confirmation
|
||||||
|
const isOverbookDialogOpen = ref(false)
|
||||||
|
const overbookingAssignment = ref<ShiftAssignment | null>(null)
|
||||||
|
|
||||||
function onApprove(assignment: ShiftAssignment) {
|
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, {
|
approveAssignment(assignment.id, {
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
flashFeedback(`${assignment.person?.full_name ?? 'Toewijzing'} goedgekeurd`, 'success')
|
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
|
cancellingAssignment.value = null
|
||||||
flashFeedback(`${name} geannuleerd`, 'success')
|
flashFeedback(`${name} geannuleerd`, 'success')
|
||||||
},
|
},
|
||||||
|
onError: (err: unknown) => {
|
||||||
|
flashFeedback(getApiErrorMessage(err, 'Fout bij annuleren'), 'error')
|
||||||
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -250,6 +280,9 @@ function onRejectExecute() {
|
|||||||
rejectReason.value = ''
|
rejectReason.value = ''
|
||||||
flashFeedback(`${name} afgewezen`, 'success')
|
flashFeedback(`${name} afgewezen`, 'success')
|
||||||
},
|
},
|
||||||
|
onError: (err: unknown) => {
|
||||||
|
flashFeedback(getApiErrorMessage(err, 'Fout bij afwijzen'), 'error')
|
||||||
|
},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -261,6 +294,13 @@ const cancellingAssignment = ref<ShiftAssignment | null>(null)
|
|||||||
// Bulk approve dialog
|
// Bulk approve dialog
|
||||||
const isBulkApproveDialogOpen = ref(false)
|
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() {
|
function onBulkApprove() {
|
||||||
isBulkApproveDialogOpen.value = true
|
isBulkApproveDialogOpen.value = true
|
||||||
}
|
}
|
||||||
@@ -274,6 +314,9 @@ function onBulkApproveExecute() {
|
|||||||
flashFeedback(`${store.selectedAssignmentIds.length} toewijzingen goedgekeurd`, 'success')
|
flashFeedback(`${store.selectedAssignmentIds.length} toewijzingen goedgekeurd`, 'success')
|
||||||
store.clearSelection()
|
store.clearSelection()
|
||||||
},
|
},
|
||||||
|
onError: (err: unknown) => {
|
||||||
|
flashFeedback(getApiErrorMessage(err, 'Fout bij bulk goedkeuren'), 'error')
|
||||||
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -856,6 +899,17 @@ function fillRateColor(): string {
|
|||||||
<strong>{{ store.selectedAssignmentIds.length }}</strong>
|
<strong>{{ store.selectedAssignmentIds.length }}</strong>
|
||||||
{{ store.selectedAssignmentIds.length === 1 ? 'toewijzing' : 'toewijzingen' }}
|
{{ store.selectedAssignmentIds.length === 1 ? 'toewijzing' : 'toewijzingen' }}
|
||||||
wilt goedkeuren?
|
wilt goedkeuren?
|
||||||
|
|
||||||
|
<VAlert
|
||||||
|
v-if="bulkApproveWillOverbook"
|
||||||
|
type="warning"
|
||||||
|
variant="tonal"
|
||||||
|
class="mt-3"
|
||||||
|
density="compact"
|
||||||
|
>
|
||||||
|
Deze actie zal de shift overbezetten
|
||||||
|
({{ shift?.filled_slots }}/{{ shift?.slots_total }} plekken bezet).
|
||||||
|
</VAlert>
|
||||||
</VCardText>
|
</VCardText>
|
||||||
<VCardActions>
|
<VCardActions>
|
||||||
<VSpacer />
|
<VSpacer />
|
||||||
@@ -866,11 +920,47 @@ function fillRateColor(): string {
|
|||||||
Annuleren
|
Annuleren
|
||||||
</VBtn>
|
</VBtn>
|
||||||
<VBtn
|
<VBtn
|
||||||
color="success"
|
:color="bulkApproveWillOverbook ? 'warning' : 'success'"
|
||||||
:loading="isBulkApproving"
|
:loading="isBulkApproving"
|
||||||
@click="onBulkApproveExecute"
|
@click="onBulkApproveExecute"
|
||||||
>
|
>
|
||||||
Goedkeuren
|
{{ bulkApproveWillOverbook ? 'Toch goedkeuren' : 'Goedkeuren' }}
|
||||||
|
</VBtn>
|
||||||
|
</VCardActions>
|
||||||
|
</VCard>
|
||||||
|
</VDialog>
|
||||||
|
|
||||||
|
<!-- Overbook confirmation dialog (single approve) -->
|
||||||
|
<VDialog
|
||||||
|
v-model="isOverbookDialogOpen"
|
||||||
|
max-width="440"
|
||||||
|
>
|
||||||
|
<VCard>
|
||||||
|
<VCardTitle class="text-h6 pt-5 px-5">
|
||||||
|
Shift overbezetten?
|
||||||
|
</VCardTitle>
|
||||||
|
<VCardText class="px-5">
|
||||||
|
Deze shift is al vol
|
||||||
|
({{ shift?.filled_slots }}/{{ shift?.slots_total }} plekken bezet).
|
||||||
|
Wil je de aanmelding van
|
||||||
|
<strong>{{ overbookingAssignment?.person?.full_name ?? 'deze persoon' }}</strong>
|
||||||
|
toch goedkeuren?
|
||||||
|
</VCardText>
|
||||||
|
<VCardActions class="px-5 pb-5">
|
||||||
|
<VSpacer />
|
||||||
|
<VBtn
|
||||||
|
variant="tonal"
|
||||||
|
@click="isOverbookDialogOpen = false"
|
||||||
|
>
|
||||||
|
Annuleren
|
||||||
|
</VBtn>
|
||||||
|
<VBtn
|
||||||
|
color="warning"
|
||||||
|
variant="flat"
|
||||||
|
:loading="isApproving"
|
||||||
|
@click="onOverbookConfirm"
|
||||||
|
>
|
||||||
|
Toch goedkeuren
|
||||||
</VBtn>
|
</VBtn>
|
||||||
</VCardActions>
|
</VCardActions>
|
||||||
</VCard>
|
</VCard>
|
||||||
|
|||||||
@@ -62,7 +62,11 @@ apiClient.interceptors.response.use(
|
|||||||
notificationStore.show('The requested item was not found.', 'warning')
|
notificationStore.show('The requested item was not found.', 'warning')
|
||||||
}
|
}
|
||||||
else if (status === 422) {
|
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) {
|
else if (status === 503) {
|
||||||
notificationStore.show('Service temporarily unavailable. Please try again later.', 'error')
|
notificationStore.show('Service temporarily unavailable. Please try again later.', 'error')
|
||||||
|
|||||||
Reference in New Issue
Block a user