https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 --- Comment #2 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- I think this bug may be due to Bug 29786 which removed the notforloan check in Koha/Holds.pm ? -- 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=30730 --- Comment #3 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 135074 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135074&action=edit Bug 30730: Holds to Pull should not list items with a notforloan status Remove items with a notforloan value other than 0 because items that are potentially holdable (with negative notforloan status) are not wanted in holds to pull. Test plan: 1- Have a bib with 2 items, both holdable. Give one of these items a notforloan status with a negative value (like Ordered in the testing docker data). 2- Place a title-level hold on your bib 3- Confirm your bib is showing on Holds to Pull as expected, prompting staff to pull the item that does not have a not for loan status (correct!) 4- Take the item that does not have a not for loan status and check it out to a different patron, make sure not to fill or cancel your hold 5- Reload Holds to Pull, see your title is still listed and now Koha is directing staff to the item with a notforloan status 6- Apply the patch 7- Reload Holds to Pull, see your title with a notforloan status is gone 8- Run `prove t/db_dependent/Koha/Holds.t` and all tests should pass -- You are receiving this mail because: You are watching all bug changes.

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135074|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 135081 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135081&action=edit Bug 30730: Holds to Pull should not list items with a notforloan status Remove items with a notforloan value other than 0 because items that are potentially holdable (with negative notforloan status) are not wanted in holds to pull. Test plan: 1- Have a bib with 2 items, both holdable. Give one of these items a notforloan status with a negative value (like Ordered in the testing docker data). 2- Place a title-level hold on your bib 3- Confirm your bib is showing on Holds to Pull as expected, prompting staff to pull the item that does not have a not for loan status (correct!) 4- Take the item that does not have a not for loan status and check it out to a different patron, make sure not to fill or cancel your hold 5- Reload Holds to Pull, see your title is still listed and now Koha is directing staff to the item with a notforloan status 6- Apply the patch 7- Reload Holds to Pull, see your title with a notforloan status is gone 8- Run `prove t/db_dependent/Koha/Holds.t` and all tests should pass 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=30730 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #7 from David Nind <david@davidnind.com> --- I originally started this an FYI note, but I think someone with a better knowledge of Koha needs to review this and decide what needs to be done, if anything. I'm a little confused with the terminology used. My current understanding: You can place a record-level hold or an item-level hold for items that have a negative not for loan value. This seems to be a feature rather than a bug, whatever the logical part of my brain thinks! The development version of the manual (which is not up-to-date) says for NOT_LOAN (Not for loan) authorised values, that: "- Negative number values will still allow holds (use for ‘on order’ statuses, for example) - Positive numbers will not allow holds or checkouts. - A value of 0 means ‘for loan’." https://koha-community.org/manual/22.05/en/html/administration.html#existing... Reference: https://koha-community.org/manual/22.05/en/html/administration.html#existing... (as the manual isn't up-to-date, this may no longer be the current situation) I also came across the SkipHoldTrapOnNotForLoanValue system preference: "This system preference is used to completely exclude items with ‘not for loan’ values from filling holds." This was added in 20.11 and back ported to 20.05 in bug 25232 - from the description "this .. prevents items with that notforloan value from triggering holds at checkin, allowing for a temporary quarantine or any other circumstance in which an item should be temporarily delayed from circulation. This presents a more customizable alternative to the existing TrapHoldsOnOrder system preference.". Reference (search the page for the system preference name): https://koha-community.org/manual/22.05/en/html/circulationpreferences.html

From this, it would seem that:

- You should be able to place holds for items with a 'Not for hold' status with a negative value. (Confirmed by testing master) - These should show up on the holds to pull list. (This is the current behaviour before this bug). - If you don't want holds to be placed on 'not for loan' items, then make these values greater than 0. For example, if you don't want holds placed on items that are on order, make the value greater than 0. I originally thought that you should be able to put that value in the SkipHoldTrapOnNotForLoanValue , but as per bug 25232 it wasn't intended for this scenario and doesn't work. Where to from here? Some options/ideas - not exhaustive: 1. Option 1: make 'Not for loan' setting make more sense (minimal effort): - update the documentation for the 'not for loan' authorised value section and system preferences SkipHoldTrapOnNotForLoanValue to clarify that: . use not for loan authorised values greater than 0 if you don't want holds to be placed, or conversely don't allow negative numbers or holds to be placed on items not for loan . use negative not for loan authorised values where items are not for loan (the logic of allowing holds on items not for loan escapes me, and I find it really confusing!) - update the default data for Koha so that the 'Ordered' value is greater than 0: that is, if it is not for loan as it is on order then you can't place holds by default; provide some SQL queries to update any existing 'Ordered' values that are negative - update the description for the SkipHoldTrapOnNotForLoanValue to clarify that this is only for when items are checked in and the scenario it is intended for => end result: holds to pull should work as intended and needs no change 2. Option 2: rethink how to implement this: . To me, if an item is 'Not for loan' then you should not be able to place holds on these items. . If you want to be able to override an item that is 'Not for loan' then maybe have a system preference OverrideNotForLoan or something similar that does what SkipHoldTrapOnNotForLoanValue does, except for placing holds. I think that this a pattern we use elsewhere in Koha. 3. Option 3: identify or ask the Koha Community for the scenarios where you may want to allow holds on items that are not for loan, and then go from there. I hope I haven't confused things even more! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 --- Comment #8 from Shi Yao Wang <shi-yao.wang@inLibro.com> ---

From my understanding/interpretation, when we put "Ordered" in "notforloan" it means the item was ordered but still not in the library, but it would make sense that patrons can place a hold on that because it will be available when it arrives at the library (and thus "notforloan" should be cleared of the "Ordered" status). Is it how it was meant to be used?

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Wow, the prefs do make this complicated! OK.. I spent some time working through this.. 'Trap' is very much about 'At Checkin' prompts.. in this case we're building lists of items to go and find.. so the preferences don't really come into play. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135082|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135208&action=edit Bug 30730: Unit tests Added an entry in holds for an item with negative notforloan value to test if get_items_that_can_fill returns the right items. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135207|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135210&action=edit Bug 30730: Holds to Pull should not list items with a notforloan status Remove items with a notforloan value other than 0 because items that are potentially holdable (with negative notforloan status) are not wanted in holds to pull. Test plan: 1- Have a bib with 2 items, both holdable. Give one of these items a notforloan status with a negative value (like Ordered in the testing docker data). 2- Place a title-level hold on your bib 3- Confirm your bib is showing on Holds to Pull as expected, prompting staff to pull the item that does not have a not for loan status (correct!) 4- Take the item that does not have a not for loan status and check it out to a different patron, make sure not to fill or cancel your hold 5- Reload Holds to Pull, see your title is still listed and now Koha is directing staff to the item with a notforloan status 6- Apply the patch 7- Reload Holds to Pull, see your title with a notforloan status is gone 8- Run `prove t/db_dependent/Koha/Holds.t` and all tests should pass Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135209|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135212&action=edit Bug 30730: (follow-up) Simplify query This patch drops the 'fetch all non-loanable items' query used in the subsequent query as part of the 'not_in' and instead replaces it with a simple filter on 'notforloan => 0' Test plan Run the included unit tests before and apply applying the followup. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression, | |rel_22_05_candidate -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 --- Comment #18 from David Nind <david@davidnind.com> --- Thanks Martin, apologies for confusing things.... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.05.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=30730 Emmanuel Bétemps <e.betemps@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |e.betemps@gmail.com -- You are receiving this mail because: You are watching all bug changes.

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30730 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_05_candidate | -- You are receiving this mail because: You are watching all bug changes.