https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=11999 CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19294 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19294 [Bug 19294] ILS-DI: HoldTitle and HoldItem don't consider "on shelf holds allowed" setting -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |arthur.suzuki@biblibre.com |ity.org | Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- *** Bug 19294 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86874|0 |1 is obsolete| | --- Comment #5 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 89723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89723&action=edit Bug 20985 : Add OnShelfHoldsAllowed checks to CanItemBeReserved Test plan : 1 / Check that default circulation have OnShelfHoldsAllowed to Yes 2 / Place a hold on a book with a single item, item being available for loan, verify that hold can be placed. 3 / Set OnShelfHoldsAllowed to any other value, verify that hold cannot be placed for reason "onShelfHoldsNotAllowed" (shown in the item table). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #7 from Hayley Mapley <hayleymapley@catalyst.net.nz> --- Hi Arthur, Looks good, only the description is not displayed in the Hold column of the item table when the 'On shelf holds allowed' preference is set to 'If all unavailable'. It is shown though when the preference is set to 'If any unavailable' -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |25408 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 [Bug 25408] CanBookBeReserved & CanItemBeReserved should check "opacitemholds" policy -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89723|0 |1 is obsolete| | --- Comment #9 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 105991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105991&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved Test plan : 1 / Check that default circulation have OnShelfHoldsAllowed to Yes 2 / Place a hold on a book with a single item, item being available for loan, verify that hold can be placed. 3 / Set OnShelfHoldsAllowed to any other value, verify that hold cannot be placed for reason "onShelfHoldsNotAllowed" (shown in the item table). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26463 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #12 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Hi, why this addition : + $item = Koha::Items->find( $itemnumber ); $item object is already fetched at beginning of method. If it is to refresh the ORM object, I think you must use ->get_from_storage() -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #110095|0 |1 is obsolete| | --- Comment #13 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 111954 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111954&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved Via ILSDI we use the routines mentioned above to determine holdability, however, neither of these routines check on shelf hold policies Test plan : 1 / Check that default circulation have OnShelfHoldsAllowed to Yes 2 / Place a hold on a book with a single item, item being available for loan, verify that hold can be placed. 3 / Set OnShelfHoldsAllowed to any other value, verify that hold cannot be placed for reason "onShelfHoldsNotAllowed" (shown in the item table). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112688|0 |1 is obsolete| | --- Comment #15 from David Nind <david@davidnind.com> --- Comment on attachment 112688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112688 Bug 25898: Fix indirect object notation in debian/list-deps Apologies - applied the wrong patch! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111954|0 |1 is obsolete| | --- Comment #16 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 113730 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113730&action=edit Bug 20985: Unit tests Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #18 from David Nind <david@davidnind.com> --- Hi Arthur. Apologies for taking so long to get back to this (and work out what this was about!). I've read the description properly now and bug 19294, so the aim is to make sure that if using ILS-DI the "On shelf holds allowed" options for circulation and fines rules are correctly applied - at the moment they aren't. Here is an updated test plan - I've worked through it and everything works as expected. (I'm not sure I've made the test plan better, but it covers both the staff interface and using ILS-DI.) Suggested updated test plan =========================== The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan: 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: . "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: . "Yes" ==> information column in the item table displays "Not on hold", the hold is placed, cancel the hold . "If any unavailable" and "If all unavailable" ==> the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold". 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): . Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold . Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests prove t/db_dependent/Reserves.t - these should pass. 7. Apply the patch (and flush_memcached and restart_all if using koha-testing-docker). 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: . For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". . For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) . Tests: should still pass. 9. Sign off. Changes ======= Tests fail ~~~~~~~~~~ The number of tests need updating from 66 to 67 (the tests pass if I update this myself): ok 67 - MergeHolds # Looks like you planned 66 tests but ran 67. Dubious, test returned 255 (wstat 65280, 0xff00) All 66 subtests passed Test Summary Report ------------------- t/db_dependent/Reserves.t (Wstat: 65280 Tests: 67 Failed: 1) Failed test: 67 Non-zero exit status: 255 Parse errors: Bad plan. You planned 66 tests but ran 67. Files=1, Tests=67, 8 wallclock secs ( 0.05 usr 0.01 sys + 6.65 cusr 0.93 csys = 7.64 CPU) Result: FAIL Wording change suggestion ~~~~~~~~~~~~~~~~~~~~~~~~~ I didn't notice any change in behaviour except for Step 4 with the patch applied, red "X onShelfHoldsNotAllowed". I would prefer to see a human readable message for the staff interface, rather than the code "onShelfHoldsNotAllowed", maybe "Holds not allowed for items on shelves" or "Holds not allowed for items available". Happy to sign off once tests fixed. David -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #20 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 114802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114802&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan: 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: . "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: . "Yes" ==> information column in the item table displays "Not on hold", the hold is placed, cancel the hold . "If any unavailable" and "If all unavailable" ==> the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold". 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): . Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold . Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests prove t/db_dependent/Reserves.t - these should pass. 7. Apply the patch (and flush_memcached and restart_all if using koha-testing-docker). 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: . For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". . For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) . Tests: should still pass. 9. Sign off. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #22 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 114805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114805&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan: 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: . "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: . "Yes" ==> information column in the item table displays "Not on hold", the hold is placed, cancel the hold . "If any unavailable" and "If all unavailable" ==> the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold". 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): . Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold . Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests prove t/db_dependent/Reserves.t - these should pass. 7. Apply the patch (and flush_memcached and restart_all if using koha-testing-docker). 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: . For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". . For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) . Tests: should still pass. 9. Sign off. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114805|0 |1 is obsolete| | --- Comment #25 from David Nind <david@davidnind.com> --- Created attachment 115733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115733&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan: 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: . "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: . "Yes" ==> information column in the item table displays "Not on hold", the hold is placed, cancel the hold . "If any unavailable" and "If all unavailable" ==> the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold". 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): . Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold . Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests prove t/db_dependent/Reserves.t - these should pass. 7. Apply the patch (and flush_memcached and restart_all if using koha-testing-docker). 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: . For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". . For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) . Tests: should still pass. 9. Sign off. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|25408 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25408 [Bug 25408] CanBookBeReserved & CanItemBeReserved should check "opacitemholds" policy -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #115732|0 |1 is obsolete| | --- Comment #27 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121100 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121100&action=edit Bug 20985: Unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121101|0 |1 is obsolete| | --- Comment #29 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121102&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan: 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: . "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: . "Yes" ==> information column in the item table displays "Not on hold", the hold is placed, cancel the hold . "If any unavailable" and "If all unavailable" ==> the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold". 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): . Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold . Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests prove t/db_dependent/Reserves.t - these should pass. 7. Apply the patch (and flush_memcached and restart_all if using koha-testing-docker). 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: . For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". . For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) . Tests: should still pass. 9. Sign off. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #31 from David Nind <david@davidnind.com> --- Hi Arthur. I quickly ran through the test plan. Everything works as expected, except the tests fail. If I manually update the number of tests to 68, everything passes. David -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #33 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hi David, just pushed a new version of the test patch with number of tests updated. Arthur -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121102|0 |1 is obsolete| | --- Comment #35 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 121176 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121176&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan: 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: . "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: . "Yes" ==> information column in the item table displays "Not on hold", the hold is placed, cancel the hold . "If any unavailable" and "If all unavailable" ==> the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold". 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): . Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold . Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) ==> Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests prove t/db_dependent/Reserves.t - these should pass. 7. Apply the patch (and flush_memcached and restart_all if using koha-testing-docker). 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: . For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". . For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) . Tests: should still pass. 9. Sign off. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #37 from David Nind <david@davidnind.com> ---

Should work now.

Everything works now - sign off on the way.. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121139|0 |1 is obsolete| | --- Comment #38 from David Nind <david@davidnind.com> --- Created attachment 121185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121185&action=edit Bug 20985: Unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #40 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Thx! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121185|0 |1 is obsolete| | Attachment #121186|0 |1 is obsolete| | --- Comment #41 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 121485 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121485&action=edit Bug 20985: Unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #44 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- There are tons of tests failing. t/db_dependent/Holds.t, t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t, t/db_dependent/api/v1/holds.t, t/db_dependent/ILSDI_Services.t and t/db_dependent/Reserves/MultiplePerRecord.t. Please fix ASAP or I will revert. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This bug fixes the "On |This bug fixes the "On release notes|shelf holds allowed" rules |shelf holds allowed" rules |so that they work correctly |so that they work correctly |when using ILS-DI to place |when using ILS-DI to place |holds. |holds. | | |The expected |The expected |behaviour for "On shelf |behaviour for "On shelf |holds allowed" settings for |holds allowed" settings for |the circulation rules (Koha |the circulation rules (Koha |administration > Patrons |administration > Patrons |and circulation > |and circulation > |Circulation and fines |Circulation and fines |rules) are: |rules) are: |- If set to |- If set to |"Yes", patrons can place |"Yes", patrons can place |holds on items currently |holds on items currently |checked in. |checked in. |- If set to "If |- If set to "If |any unavailable", patrons |any unavailable", patrons |can only place holds on |can only place holds on |items that are not |items that are not |unavailable. |unavailable. |- If set to |- If set to |"If all unavailable", |"If all unavailable", |patrons can only place |patrons can only place |holds on items where *all* |holds on items where *all* |items on the record are |items on the record are |unavailable. |unavailable. | | | |RMNOTE: | |Reverted! Status|Pushed to master |ASSIGNED Version(s)|21.11.00 | released in| | --- Comment #46 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Patches reverted. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|19294 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19294 [Bug 19294] ILS-DI: HoldTitle and HoldItem don't consider "on shelf holds allowed" setting -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #49 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Fridolin Somers from comment #47)

+ # Check for item on shelves and OnShelfHoldsAllowed + return { status => 'onShelfHoldsNotAllowed' } + unless IsAvailableForItemLevelRequest($item, $patron, $pickup_branchcode,1);

Strange : IsAvailableForItemLevelRequest() is not the feature of on shelf holds allowed, it means hold can be on item level or forced to biblio level o_O

Ah this method is calling : Koha::CirculationRules->get_onshelfholds_policy() -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |shi-yao.wang@inLibro.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 mathieu saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathsabypro@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #51 from Julian Maurice <julian.maurice@biblibre.com> --- Patch rebased and reworked. The original patch had a problem that caused infinite recursion (CanItemBeReserved called IsAvailableForItemLevelRequest which called ItemsAnyAvailableAndNotRestricted which called CanItemBeReserved) Replaced the call to IsAvailableForItemLevelRequest by a call to a new subroutine IsOnShelfHoldsPolicySatisfied which does only what its name suggests All tests mentioned in comment 44 should pass. I removed the Signed-off-by lines because the patch changed significantly, but the test plan did not change. Please test again -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #53 from Fridolin Somers <fridolin.somers@biblibre.com> --- + unless ( $params->{ignore_onshelfholds_policy} ) { + unless ( IsOnShelfHoldsPolicySatisfied( { item => $item, patron => $patron } ) ) { + return _cache { status => 'onShelfHoldsNotAllowed' }; + } + } Why call _cache() ? I cant find this method -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 pierre.genty@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pierre.genty@biblibre.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32702 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #57 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to Nick Clemens (kidclamp) from comment #52)

I believe other calls to CanItemBeRserved should be adjusted or taken into consideration

Both reserve/request.pl and opac/opac-reserve.pl currently call CanItemBeReserved and then call IsAvailableForItemLevelRequest

After this code, if I understand correctly, IsOnShelfHoldsPolicySatisfied will now be called twice for each item - once from CanItem.. and once from IsAvailable..

See bug 32702 - we really need to consolidate these two routines, or if a check is moved into CanItemBeReserved, it should be removed from the other?

there is another bug to merge CanItemBeReserved and IsAvailableForItemLevelRequest in a single function. Also I've seen that most time both functions are called, IsAvailableForItemLevelRequest is only called if CanItemBeReserved is "ok". that makes less calls. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff CC| |m.de.rooy@rijksmuseum.nl --- Comment #58 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Changing status to NSO. This needs to be tested thoroughly. The last comments may need further attention too. Note that it went from NSO to FQA and back from FQA to SO in recent history. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #60 from David Nind <david@davidnind.com> --- Have failed QA 8-(. There are problems with the tests, everything else works as per the test plan. Failing tests - see further down for details: 1. t/db_dependent/Reserves.t: - Fails before and after the patch if "On shelf holds allowed" for the All All circulation and fine rules is set to "If any unavailable" and "If all unavailable" - I'm going to assume that this should be dealt with on another bug - The number of tests needs adjusting from 71 to 72 2. t/db_dependent/api/v1/holds.t: only passes after the patch if "On shelf holds allowed" is set to "Yes", they fail if set to the other options. The tests passed before the patch for all the options. Comment about the message in the holds column - "onShelfHoldsNotAllowed": 1. That seems a bit more "developer" focused, than for library staff. Could it be a friendlier message for staff? I realise we want to keep it succinct. 2. Examples: "Cannot place hold", "Hold not allowed", or "Not available to place a hold". 3. The message at the top of the page is: Cannot place hold - No items are available to be placed on hold. Testing notes (using KTD): 1. Set ILS-DI:AuthorizedIPs to 0.0.0.0/0 2. Step 5, after patch is applied for ILS-DI: - If "On shelf holds allowed" = "Yes", hold placed (as expected) - If "If any unavailable" and "If all unavailable", onShelfHoldsNotAllowed message returned (now working as expected) Failing tests before and after patches for t/db_dependent/Reserves.t ==================================================================== t/db_dependent/Reserves.t fails before and after the patch if "On shelf holds allowed" for the All All circulation and fine rules is set to "If any unavailable" and "If all unavailable". Note the number of tests needs adjusting from 71 to 72 after the patch is applied. prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 57/71 # Failed test 'Item not available for item-level hold because no effective item type' # at t/db_dependent/Reserves.t line 1486. # Looks like you failed 1 test of 3. # Failed test 'IsAvailableForItemLevelRequest() tests' # at t/db_dependent/Reserves.t line 1507. t/db_dependent/Reserves.t .. 66/71 # Looks like you failed 1 test of 71. t/db_dependent/Reserves.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/71 subtests Test Summary Report ------------------- t/db_dependent/Reserves.t (Wstat: 256 (exited 1) Tests: 71 Failed: 1) Failed test: 61 Non-zero exit status: 1 Files=1, Tests=71, 7 wallclock secs ( 0.03 usr 0.00 sys + 5.10 cusr 1.93 csys = 7.06 CPU) Result: FAIL Failing tests after patches for t/db_dependent/api/v1/holds.t ============================================================= After the patch, prove t/db_dependent/api/v1/holds.t only passes if "On shelf holds allowed" set to "Yes", if set to the other options they fail. The tests passed before the patch for all options. prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 10/20 # Failed test 'Override works for 'expired'' # at t/db_dependent/api/v1/holds.t line 859. # got: '403' # expected: '201' # Looks like you failed 1 test of 5. # Failed test ''expired' tests' # at t/db_dependent/api/v1/holds.t line 861. # Failed test 'Override works for 'debt_limit'' # at t/db_dependent/api/v1/holds.t line 888. # got: '403' # expected: '201' # Looks like you planned 7 tests but ran 5. # Looks like you failed 1 test of 5 run. # Failed test ''debt_limit' tests' # at t/db_dependent/api/v1/holds.t line 896. # Looks like you planned 7 tests but ran 2. # Looks like you failed 2 tests of 2 run. t/db_dependent/api/v1/holds.t .. 11/20 # Failed test 'add() + can_place_holds() tests' # at t/db_dependent/api/v1/holds.t line 1061. Can't call method "delete" on an undefined value at t/db_dependent/api/v1/holds.t line 890. # Looks like your test exited with 11 just after 12. t/db_dependent/api/v1/holds.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 9/20 subtests Test Summary Report ------------------- t/db_dependent/api/v1/holds.t (Wstat: 2816 (exited 11) Tests: 12 Failed: 1) Failed test: 11 Non-zero exit status: 11 Parse errors: Bad plan. You planned 20 tests but ran 12. Files=1, Tests=12, 9 wallclock secs ( 0.03 usr 0.01 sys + 6.98 cusr 1.41 csys = 8.43 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #61 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 199291 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199291&action=edit Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved The expected behaviour for "On shelf holds allowed" setting for the circulation rules (Koha administration > Patrons and circulation > Circulation and fines rules): - Allow holds only on items that are currently checked out or otherwise unavailable. - If set to "Yes", patrons can place holds on items currently checked in. - If set to "If any unavailable", patrons can only place holds on items that are not unavailable. - If set to "If all unavailable", patrons can only place holds on items where *all* items on the record are unavailable. (Adapted from https://bywatersolutions.com/education/preparing-for-library-closures) These rules should also work when using ILS-DI, but currently they don't. This bug makes sure that the "On shelf holds allowed" rules work correctly when using ILS-DI to place holds. Test plan --------- 1. Enable ILS-DI (set the ILS-DI system preference to Enable). 2. Go to Koha administration > Patrons and circulation > Circulation and fines rules. 3. Work through steps 4-5 for each of the settings for "On shelf holds allowed" for all libraries/patron categories/item types: "Yes", "If any unavailable", and "If all unavailable" 4. Staff interface - place a hold on a record with items available for loan, the rules should work as expected before and after the patch is applied: "Yes": information column in the item table displays "Not on hold", the hold is placed, cancel the hold "If any unavailable" and "If all unavailable": the hold is not placed, message is "Cannot place hold. No items are available to be placed on hold.", red "X" in the hold column and the information column displays "Not on hold" 5. ILS-DI - place a hold on a record with items available for loan (note: without the patch, holds can be placed): - Query to place a hold using ILS-DI on a title that have all its items available, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 Without the patch the hold is placed but it shouldn't be allowed, cancel the hold - Query to place a hold using ILS-DI on an available item, example query: http://127.0.0.1:8080/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) Without the patch the hold is placed but it shouldn't be allowed, cancel the hold 6. Run the tests in t/db_dependent/Reserves.t - these should pass. 7. Apply the patch, flush the cache and restart Koha (if using koha-testing-docker, run flush_memcached and restart_all) 8. Run through steps 3-6 again, and note the changes when "If any unavailable" and "If all unavailable" options are used: For the staff interface: there should be no change in behavour and should work as expected, for the red "X" in the items table additional text is added "onShelfHoldsNotAllowed". For ILS-DI: these should now work as expected, with holds not placed, and this message in the results returned <code>onShelfHoldsNotAllowed</code> (check to confirm no holds place for either the patron or the item) Tests: should still pass, in particular the following tests: - t/db_dependent/Reserves.t - t/db_dependent/Holds.t - t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t - t/db_dependent/api/v1/holds.t - t/db_dependent/ILSDI_Services.t - t/db_dependent/Reserves/MultiplePerRecord.t - t/db_dependent/Reserves/IsOnShelfHoldsPolicySatisfied.t (new test file) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20985 --- Comment #64 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- Created attachment 201631 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201631&action=edit Bug 20985: (follow-up) Reword message for onShelfHoldsNotAllowed case -- You are receiving this mail because: You are watching all bug changes.