It is a regression from bug 38650 that issuing non-holdable items now fills holds. The subroutine C4::Reserves::MoveReserve would previously only do anything if there where a hold that had been filled with the specified item, which mean that the item issued was holdable. But bug 38650 opened up the possibility to fill the borrowers hold even if the issued item is not holdable. I suggest a check that the item issued is holdable if the biblio is reserved by the borrower.
Created attachment 186376 [details] [review] Bug 40792: Do not fill holds with non-reservable items. Verify that the item issued is reservable before filling the patron's hold. 1. Go to circulation and fine rules and add a rule for an itemtype that can be issued, for instance "Maps" in koha-testing-docker, with 0 in the column "Holds per record (count)". 2. Go to any biblio where there are items that may be reserved. Create an item with the above created item type and give it a barcode, e.g., 'tempbarcode'. 3. Place a hold for any patron (for instance 23529000120056 in koha-testing-docker). 4. Issue the item 'tempbarcode' to the patron with the hold. 5. Make sure the hold remains. 6. Create a new hold to another borrower (for instance 23529000197047), and make sure this new hold has priority 2. 7. Return the item 'tempbarcode' and issue it to the second patron. 8. Make sure both holds remain.
Tested in production environment, works as expected. Great work! Note: this is to enable patrons to have a title level reservation on a title that has multiple item types, one type that is allowed to fill a hold (normal loan) and one type that is not allowed to fill a hold (might be short-time loan etc). Patrons must be able to keep their title level reservation on normal loan item type while they can check out the short-time item type.
There are several conflicts. CONFLICT (content): Merge conflict in C4/Reserves.pm Please rebase.
Created attachment 186565 [details] [review] Bug 40792: Do not fill holds with non-reservable items. Verify that the item issued is reservable before filling the patron's hold. 1. Go to circulation and fine rules and add a rule for an itemtype that can be issued, for instance "Maps" in koha-testing-docker, with 0 in the column "Holds per record (count)". 2. Go to any biblio where there are items that may be reserved. Create an item with the above created item type and give it a barcode, e.g., 'tempbarcode'. 3. Place a hold for any patron (for instance 23529000120056 in koha-testing-docker). 4. Issue the item 'tempbarcode' to the patron with the hold. 5. Make sure the hold remains. 6. Create a new hold to another borrower (for instance 23529000197047), and make sure this new hold has priority 2. 7. Return the item 'tempbarcode' and issue it to the second patron. 8. Make sure both holds remain.
Rebased on main.
Hi, This patch changes a module (Reserves) and does not add tests. Please add unit tests. And verify that your change does not break other tests.
And please fill Assignee next time :)
I believe this also needs a system preference or a switch in the circulation rules. The intention on 38650 was to fill the hold regardless of the item's holdability. We have some libraries who maintain a 'lucky day' collection of popular books which cannot have holds placed directly, however, a patron having a 'lucky day' can find the book they want on the shelf and check it out - when doing so, the hold is expected to be filled
(In reply to Nick Clemens (kidclamp) from comment #8) > I believe this also needs a system preference or a switch in the circulation > rules. The intention on 38650 was to fill the hold regardless of the item's > holdability. > > We have some libraries who maintain a 'lucky day' collection of popular > books which cannot have holds placed directly, however, a patron having a > 'lucky day' can find the book they want on the shelf and check it out - when > doing so, the hold is expected to be filled Maybe a boolean parameter on the itemtype: "may fill reservations"? Or even a circulation rule?
I could see both sides of this... that is wanting to satisfy the hold and not wanted to satisfy the hold. I think the most elegant way to do this would be at the itemtype level. Just a true/false that allows an itemtype to fill a hold. That would allow a library or consortia to take a more nuanced, situational approach.