fix(loans): promuovi la testa della coda quando le prenotazioni saturano le copie (#157)#199
Open
fabiodalez-dev wants to merge 1 commit into
Open
fix(loans): promuovi la testa della coda quando le prenotazioni saturano le copie (#157)#199fabiodalez-dev wants to merge 1 commit into
fabiodalez-dev wants to merge 1 commit into
Conversation
…be the copies (#157) The reservation-maintenance cron failed to convert any reservation into a pending loan when a fully-subscribed waitlist queued for a title — e.g. 1 copy + 2 active reservations promoted 0 instead of 1. Root cause: the promotion gate (ReservationManager::processBookAvailability → isDateRangeAvailable → CapacityService::hasFreeCapacity) counted EVERY active reservation overlapping the period as occupying a capacity unit, excluding only the one being promoted. With 1 copy and 2 queued reservations, promoting the head (queue position 1) still counted the trailing entry (position 2) as occupying the only copy, so occ == total and the head was never promoted. Both runs converted 0; F.21b (#157) saw 0 pending instead of 1. Fix: the promotion path now passes the promoted reservation's queue_position so the capacity gate ignores active reservations with a known queue_position strictly greater than it — lower-priority waitlist entries promoted in later runs as copies free up. Reservations AHEAD in the queue (lower position) still count, and a legacy NULL queue_position still counts (conservative — never overbook). The new parameter defaults to null, so the admin-creation gate and overbooked auditor sharing CapacityService are unchanged. Verified: PHPStan level 5 clean; cron repro now promotes exactly one head (run 1 → 1 pending, run 2 → still 1); E2E loan-reservation-complete 26/26 (F.21b passes), loan-overlap 35/35 (overbook prevention intact), loan-state-model 4/4.
📝 WalkthroughWalkthroughLa promozione di una prenotazione ora calcola la posizione di coda della testa e la passa al controllo di capacità. Il calcolo della disponibilità esclude le prenotazioni con ChangesPromozione prenotazione e gate di capacità
Effort di revisione🎯 3 (Moderate) | ⏱️ ~20 minuti 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Controllers/ReservationManager.php`:
- Around line 196-199: In ReservationManager::promoteNextReservation, the queue
reordering path still uses nextReservation['queue_position'] directly, which
turns NULL into 0 and corrupts positions. Reuse the resolved queue position
already stored in $headQueuePos, and when it is null, treat the
promotion/reorder as starting from 1 instead of passing a zero value into
updateQueuePositions. Make the fix in the promotion flow around
isDateRangeAvailable and updateQueuePositions so the same validated queue
position is used consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb211fa2-61d6-42d6-8158-d11bc58d2920
📒 Files selected for processing (2)
app/Controllers/ReservationManager.phpapp/Services/CapacityService.php
Comment on lines
+196
to
+199
| $headQueuePos = isset($nextReservation['queue_position']) | ||
| ? (int) $nextReservation['queue_position'] | ||
| : null; | ||
| if ($this->isDateRangeAvailable($bookId, $startDate, $endDate, (int) $nextReservation['id'], $headQueuePos)) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Gestisci anche il riordino quando queue_position è NULL.
Qui $headQueuePos può diventare null e la promozione può proseguire; più avanti però updateQueuePositions($bookId, (int) $nextReservation['queue_position']) trasforma NULL in 0, decrementando tutte le posizioni > 0 e creando code con posizione 0. Usa lo stesso valore risolto e riordina da 1 quando manca una posizione valida.
🐛 Fix proposto
- $headQueuePos = isset($nextReservation['queue_position'])
+ $headQueuePos = isset($nextReservation['queue_position'])
? (int) $nextReservation['queue_position']
: null;
if ($this->isDateRangeAvailable($bookId, $startDate, $endDate, (int) $nextReservation['id'], $headQueuePos)) {
@@
- $this->updateQueuePositions($bookId, (int) $nextReservation['queue_position']);
+ if ($headQueuePos !== null) {
+ $this->updateQueuePositions($bookId, $headQueuePos);
+ } else {
+ $this->reorderQueuePositions($bookId);
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Controllers/ReservationManager.php` around lines 196 - 199, In
ReservationManager::promoteNextReservation, the queue reordering path still uses
nextReservation['queue_position'] directly, which turns NULL into 0 and corrupts
positions. Reuse the resolved queue position already stored in $headQueuePos,
and when it is null, treat the promotion/reorder as starting from 1 instead of
passing a zero value into updateQueuePositions. Make the fix in the promotion
flow around isDateRangeAvailable and updateQueuePositions so the same validated
queue position is used consistently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problema
La manutenzione prenotazioni (
cron/full-maintenance.php→MaintenanceService::processScheduledReservations) non promuoveva nessuna prenotazione in prestito pendente quando una waitlist saturava il titolo: 1 copia + 2 prenotazioni attive → 0 promosse invece di 1.Causa
Il gate di promozione (
ReservationManager::processBookAvailability→isDateRangeAvailable→CapacityService::hasFreeCapacity) contava ogni prenotazione attiva sovrapposta come occupante una unità di capacità, escludendo solo quella in promozione. Con 1 copia e 2 prenotazioni, promuovendo la testa (queue_position=1) la seconda (queue_position=2) veniva comunque contata come occupante l'unica copia →occ == total→ testa mai promossa. Entrambi i run convertivano 0; il test F.21b (#157) vedeva 0 pending invece di 1.Fix
Il path di promozione ora passa la
queue_positiondella prenotazione promossa, così il capacity gate ignora le prenotazioni attive conqueue_positionstrettamente maggiore (le entry di coda a priorità inferiore, promosse nei run successivi man mano che le copie si liberano).queue_positionNULL (legacy) conta ancora (conservativo — mai overbook).default null): admin-creation gate e overbooked-auditor, che condividonoCapacityService, restano invariati.Verifica
Emerso eseguendo l'intera suite E2E; pre-esistente su
main, indipendente dal PR Swagger #197.Summary by CodeRabbit