Current behavior is: LocalHoldsPriority is checked if enabled Item level holds are filled if possible Now we loop requests: - We check if there are items held at the pickup branch - We check if these items HoldsQueuePrioritizeBranch match the patron branch - Otherwise we check the transport cost matrix - continued checks The correct behavior would be Now we loop requests: - We check if there are items held at the pickup branch - We check if these items HoldsQueuePrioritizeBranch match the patron branch - We check the transport cost matrix - continued checks The difference being that if we can't fill a hold by match HoldsQueuePrioritizeBranch we should next check the matrix, not skip it. The matrix, if enabled, will select where the holding and pickup library match (as it is the least cost transfer to not transfer) and HoldsQueuePrioritize branch doesn't match, so we want this fallback
Created attachment 159335 [details] [review] Bug 35431: Unit tests
Created attachment 159336 [details] [review] Bug 35431: Check TransportCostMatrix if hold not filled by HoldsQueuePrioritizeBranch This patch changes an elsif to an if to ensure the matrix is checked every time To test: 1 - Apply unit test patch only 2 - prove -v t/db_dependent/HoldQueue.t 3 - It fails 4 - Apply this patch 5 - prove -v t/db_dependent/HoldQueue.t 6 - It passes 7 - All other tests continue to pass as well
This was a lesson in how the holds queue works, the TCM is skipped here in the case where there are items available/held at the pickup location of the hold, even when checked in this piece of the code, we are looking for HoldsQueuePrioritizeBranch matches We set the 'holdingbranch' variable here to either the pickup library (if there are items held there) or to the lowest cost branch. The code next checks if there are items where the items home branch and the holds pickup location match, and fills those - trying to get items back to where they are from If there are none, then the holdingbranch variable is checked to see if items from that library can fill - this is the actual matrix checking Filing a new bug for adding more comments to the routine here