Created attachment 17872 [details] Screenshot showing all holds blocked because 2 out of 3 are un-holdable When placing holds on multiple titles in the OPAC the entire operation is blocked if any one of those multiple titles cannot be placed on hold.
Created attachment 17873 [details] [review] Bug 10166 - Can't place holds on multiple titles if one or more are un-holdable opac-reserve.pl tries to check whether all selected titles in a multiple-hold batch are unavailable to be placed on hold. However, the logic is flawed in such a way that if the last item in the batch cannot be placed on hold the script assumes none can be placed on hold. This patch modifies the way the script tracks the "no titles available for holds" variable in order to correct the error. To test, place multiple holds by selecting titles from a list of search results. Test three conditions: - All titles are available to be placed on hold You should see no onscreen warnings, and all titles should be selectable on the place hold screen. A "Place hold" button should appear at the bottom. - Some titles can be placed on hold, some cannot The titles which can be placed on hold should be selectable. Titles which cannot be placed on hold should show a warning message. A "Place hold" button should appear at the bottom. - No titles can be placed on hold "Sorry, none of these items can be placed on hold." should appear at the top of the page. All titles should appear with warning messages. There should be no "Place hold" button.
Created attachment 17936 [details] [review] Bug 10166 - Can't place holds on multiple titles if one or more are un-holdable opac-reserve.pl tries to check whether all selected titles in a multiple-hold batch are unavailable to be placed on hold. However, the logic is flawed in such a way that if the last item in the batch cannot be placed on hold the script assumes none can be placed on hold. This patch modifies the way the script tracks the "no titles available for holds" variable in order to correct the error. To test, place multiple holds by selecting titles from a list of search results. Test three conditions: - All titles are available to be placed on hold You should see no onscreen warnings, and all titles should be selectable on the place hold screen. A "Place hold" button should appear at the bottom. - Some titles can be placed on hold, some cannot The titles which can be placed on hold should be selectable. Titles which cannot be placed on hold should show a warning message. A "Place hold" button should appear at the bottom. - No titles can be placed on hold "Sorry, none of these items can be placed on hold." should appear at the top of the page. All titles should appear with warning messages. There should be no "Place hold" button. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 17947 [details] [review] [PASSED QA] Bug 10166 - Can't place holds on multiple titles if one or more are un-holdable opac-reserve.pl tries to check whether all selected titles in a multiple-hold batch are unavailable to be placed on hold. However, the logic is flawed in such a way that if the last item in the batch cannot be placed on hold the script assumes none can be placed on hold. This patch modifies the way the script tracks the "no titles available for holds" variable in order to correct the error. To test, place multiple holds by selecting titles from a list of search results. Test three conditions: - All titles are available to be placed on hold You should see no onscreen warnings, and all titles should be selectable on the place hold screen. A "Place hold" button should appear at the bottom. - Some titles can be placed on hold, some cannot The titles which can be placed on hold should be selectable. Titles which cannot be placed on hold should show a warning message. A "Place hold" button should appear at the bottom. - No titles can be placed on hold "Sorry, none of these items can be placed on hold." should appear at the top of the page. All titles should appear with warning messages. There should be no "Place hold" button. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and QA script pass.
I have run through the test plan before and after applying the patch. I couldn't reproduce the problem, but I also can't find any problems with the patch. Leaving this for Jared as I think he was involved in solving the problem.
Sorry if I didn't provide enough information to reproduce the problem. The screenshot helps explain: If the last item in the list of multiple titles to hold is unavailable it prevents all from being placed on hold. The problem doesn't exist if the last item in the list *is* available to be placed on hold. To reproduce you have to make sure the last title you check in the search results list is not available to be placed on hold.
Hi Owen, maybe I got a bit lot between records and items here - does it mean the last item on the record or the last entry in the list of records? I think I tried both and it worked, but I could try to reproduce again.
This patch has been pushed to master and 3.12.x. I did a small follow-up which I am pretty sure will be necessary when someone addresses the maxholds issue.
Does not apply to 3.10.x please send a new patch if it is needed for 3.10.x
Created attachment 18167 [details] [review] Bug 10166 [3.10.x] Can't place holds on multiple titles if one or more are un-holdable opac-reserve.pl tries to check whether all selected titles in a multiple-hold batch are unavailable to be placed on hold. However, the logic is flawed in such a way that if the last item in the batch cannot be placed on hold the script assumes none can be placed on hold. This patch modifies the way the script tracks the "no titles available for holds" variable in order to correct the error. To test, place multiple holds by selecting titles from a list of search results. Test three conditions: - All titles are available to be placed on hold You should see no onscreen warnings, and all titles should be selectable on the place hold screen. A "Place hold" button should appear at the bottom. - Some titles can be placed on hold, some cannot The titles which can be placed on hold should be selectable. Titles which cannot be placed on hold should show a warning message. A "Place hold" button should appear at the bottom. - No titles can be placed on hold "Sorry, none of these items can be placed on hold." should appear at the top of the page. All titles should appear with warning messages. There should be no "Place hold" button.
Created attachment 18395 [details] [review] Bug 10166 [3.10.x] Can't place holds on multiple titles if one or more are un-holdable opac-reserve.pl tries to check whether all selected titles in a multiple-hold batch are unavailable to be placed on hold. However, the logic is flawed in such a way that if the last item in the batch cannot be placed on hold the script assumes none can be placed on hold. This patch modifies the way the script tracks the "no titles available for holds" variable in order to correct the error. To test, place multiple holds by selecting titles from a list of search results. Test three conditions: - All titles are available to be placed on hold You should see no onscreen warnings, and all titles should be selectable on the place hold screen. A "Place hold" button should appear at the bottom. - Some titles can be placed on hold, some cannot The titles which can be placed on hold should be selectable. Titles which cannot be placed on hold should show a warning message. A "Place hold" button should appear at the bottom. - No titles can be placed on hold "Sorry, none of these items can be placed on hold." should appear at the top of the page. All titles should appear with warning messages. There should be no "Place hold" button. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
The last patch fixes the issue for the 3.10.x branch. Maybe the Jared's patch should be retrieved too: commit 5c9522382c83f078692051fa572c56b21b54fa77 Author: Jared Camins-Esakov <jcamins@cpbibliography.com> AuthorDate: Mon May 6 08:35:00 2013 -0400 Commit: Jared Camins-Esakov <jcamins@cpbibliography.com> CommitDate: Mon May 6 08:38:09 2013 -0400 Bug 10166: Increment holdable counter Marked as Passed QA.
Created attachment 18989 [details] [review] Bug 10166 [3.10.x] Can't place holds on multiple titles if one or more are un-holdable opac-reserve.pl tries to check whether all selected titles in a multiple-hold batch are unavailable to be placed on hold. However, the logic is flawed in such a way that if the last item in the batch cannot be placed on hold the script assumes none can be placed on hold. This patch modifies the way the script tracks the "no titles available for holds" variable in order to correct the error. To test, place multiple holds by selecting titles from a list of search results. Test three conditions: - All titles are available to be placed on hold You should see no onscreen warnings, and all titles should be selectable on the place hold screen. A "Place hold" button should appear at the bottom. - Some titles can be placed on hold, some cannot The titles which can be placed on hold should be selectable. Titles which cannot be placed on hold should show a warning message. A "Place hold" button should appear at the bottom. - No titles can be placed on hold "Sorry, none of these items can be placed on hold." should appear at the top of the page. All titles should appear with warning messages. There should be no "Place hold" button. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Pushed to 3.10.x, will be in 3.10.7 Thanks Owen and thanks Chris for the tip.
Pushed to 3.8.x, will be in 3.8.14