If a Borrower cannot get an Item transfered to his pickup location of choice, the Borrower needs to know that before he places a hold. This is especially true when placing Item-level holds. If a patron can place a hold on an item, he expects the item to be available at his pickup location of choice. Currently there is no warning or hold block for holds that violate the UseBranchTransferLimits-setting. Solution. Create or use an existing centralized CheckBranchTransferLimits-function inside the C4::Reserves::CanItemBeReserved(), to see if an item can be reserved for the desired pickup location. When the borrower changes his pickup location in koha/opac-reserve.pl?biblionumber=xxx, the BranchTransferLimits need to be recalculated. UseBranchTransferLimits should also affect placing holds via the Restful API's. $pickuplocation is not always available in the Restful API -requests so we can use the Borrowers homebranch as the default pickup location.
BTW: Should this be optional? Is it against the initial spirit of UseBranchTransferLimits?
Also when a Patron is placing a hold, the Patron should be able to choose the pickup location even if there are no available/holdable Items in his home branch. So instead of failing to reach the "Place hold"-dialogue and choose the pickuplocation, the Patron should be sent to the koha/opac-reserve.pl?biblionumber=xxx with a notifcation that no items are available to his pickup location, prompting the Patron to choose another pickup location.
Created attachment 21891 [details] [review] Bug 10993 [ENH] - Transfer limits should be checked when placing a hold/reservation in OPAC.
Created attachment 21892 [details] quickie test document
Sponsored by the Joensuu Regional Library
Created attachment 22000 [details] [review] Bug 10993 - Transfer limits should be checked when placing a hold/reservation in OPAC. Depends on bug 11005, 11020 Prevents a user from placing holds on material whose branch transfer is denied. Displays clear messages that items are unavailable and re-checks in the business layer in case of UI blocks failing. Enables choosing another pickup location. Manual tests included.
Created attachment 22001 [details] Manual test cases
Some QA comments about this patch (I haven't tested, just looking at the code) * you must use 4 spaces and not tabs * +1 for the "manual test case". Note that we use the term "test plan". It could confuse some tester that are not english natives & could search the test plan No other code or patch comment (which does not mean passed QA yet, I haven't tested it either)
Created attachment 22322 [details] [review] Bug 10993 - Transfer limits should be checked when placing a hold/reservation in OPAC. Depends on bug 11005, 11020 Prevents a user from placing holds on material whose branch transfer is denied. Displays clear messages that items are unavailable and re-checks in the business layer in case of UI blocks failing. Enables choosing another pickup location. Manual test plan included.
Fixed tabs to spaces in indentation. Didn't realize my IDE uses tabs instead of spaces until bug 11024/7376. Not sure about templates since they are such a mess regarding indentation.
Created attachment 23163 [details] [review] Bug 10993 - Transfer limits should be checked when placing a hold/reservation in OPAC. Depends on bug 11005, 11020 Prevents a user from placing holds on material whose branch transfer is denied. Displays clear messages that items are unavailable and re-checks in the business layer in case of UI blocks failing. Enables choosing another pickup location. Manual test plan included. Formatted with perltidy and Perl::Critic level 5
trying to test this patch during hackfest KohaCon14. Unfortunately, it does not apply : fatal: sha1 information is lacking or useless (C4/Circulation.pm). Another problem: you're mixed "real", "new" code and reindent/perlity. This patch cannot be QAed. I encourage you to perltidy/reindent, but in a separate patch please.
Sorry, I cannot work on this currently. I will let this slide to the next one who encounters this issue and wants to fix it. Hopefully using some of my code :)
(In reply to Olli-Antti Kivilahti from comment #13) > Sorry, I cannot work on this currently. I will let this slide to the next > one who encounters this issue and wants to fix it. Hopefully using some of > my code :) I felt like fainting getting half way through with no sign of any obvious changes, except for white space. Do you have time to make reading and testing it more feasible?
Koha seems to correctly apply the transfer limit now, but Koha doesn't warn patron when the hold cannot be placed. Specifications: Koha with at least 2 branches: branch A , branch B Systeme preference: UseBranchTransferLimits ---> Enforce Systeme preference BranchTransferLimitsType ---> item type System preference OPACAllowUserToChooseBranch ---> Don’t allow Configurations – transfer limit 1. Go to Administration › Set Library checkin and transfer limit 2. Select a branch A (top menu) 3. Click on the item type tab to block any transfer 4. Uncheck branch B 5. Save 6. Select branch B (top menu) 7. Click on the item type tab 8. Uncheck branch A to block any transfer 9. Save Test Plan 1. Find a patron from branch B 2. Go on the OPAC, find an item currently checkout from branch A (same item type that should be blocked) 3. Place a hold in the patron account (branch B) 4. Confirm the hold –> notice that Koha doesn’t add the hold in the patron account, but there is no explanation, it is uncleared what just happen. The usual message «There are no items that can be placed on holds» isn’t popping up on screen to confirm that this action cannot be done.
Further notes: If you are modifying the system preference OPACAllowUserToChooseBranch to allow the pick up branch selection, Koha will block the hold by removing the blocked branch name from the menu. Therefore, Koha prevent the patron to place an hold and send it to their branch when the transfer is not allowed.