https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #1 from Hans Pålsson <hans.palsson@hkr.se> --- The problem can be reproduced by doing the following; The basic assumption is that no on shelf holds are allowed ("if all unavailable"), regardless of item type. No holds on Item level. Setting a Koha item type on the biblio (942 c) has no effect on the issue. In smart rules define 1 category of items that allow holds on biblio level if all unavailable ("course literature" in our catalog), and one category that do not allow holds whatsoever ("dayloan"). Lend the "course literature" item specimen that is eligeble for holds to a patron. Leave the other copy that do not allow holds on the shelf. As a patron that has no extra powers than a normal user, place a hold on biblio level. Koha will report that _there are no items that can be placed on hold_. It seems that smart rules that should be applied on item levels are not if there is a mix of rules for items on one biblio. -- 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=24683 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100966 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100966&action=edit Bug 24683: Add description of QueryFuzzy support in ES to syspref description To test: Apply patch Find QueryFuzzy in sysprefs Verify the new text makes sense -- 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=24683 Hans Pålsson <hans.palsson@hkr.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #4 from Hans Pålsson <hans.palsson@hkr.se> --- (In reply to Nick Clemens from comment #2)

Created attachment 100966 [details] [review] Bug 24683: Add description of QueryFuzzy support in ES to syspref description

To test: Apply patch Find QueryFuzzy in sysprefs Verify the new text makes sense

Nick: this is not the bug you are looking for. -- 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=24683 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100966|0 |1 is obsolete| | -- 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=24683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW CC| |katrin.fischer@bsz-bw.de --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Changing to NEW as there is no patch and no assignee. Please update if you plan to work on this! -- 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=24683 --- Comment #7 from Hans Pålsson <hans.palsson@hkr.se> --- In version 18.11 we successfully tried adding the following section of code to lines 1191 to 1193 in Reserves.pm, however the same code is not used in the current installation. It might be of some use. if($i->itype() eq "DL") { next; } -- 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=24683 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |slavashishkin@gmail.com, | |stalkernoid@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #9 from Andrew Nugged <nugged@gmail.com> --- Created attachment 105652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105652&action=edit Bug 24683: Fix for take smart rules into account in "if all unavailable" Inside of ItemsAnyAvailableAndNotRestricted was no effect from main set of smart rules (per record and other limits): i.e. call to "CanItemBeReserved" was absent totally. Because of this there was a bug: for example none of two items were allowed to be held when first was allowed by one smart rule, BUT on loan, and second was disallowed by another smart rule (for example, 0 "Holds per record"), i.e. in this case both items unavailable: so on-shelf holds setting "allow hold if all unavailable" should allow to hold first one, and not the second one. But it was that both wasn't allowed to be held. Solution: call to sub "CanItemBeReserved" added so it checked for "...->{status} ne 'OK'" so now if item restricted by smart rule it also accounted as "unavailable" and "AnyAvailavble" not counts it. How to reproduce: 1. Add 2 smart rules (/cgi-bin/koha/admin/smart-rules.pl) with "on shelf holds": "if all unavailable" for all rules, no "item level holds", and set "holds per record" to 2 for "books" and "0" for "computer files". 2. Create only 2 items for one biblio, but different types, "book" and "computer file". For example in misc4dev env: /cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additem 3. Check out that item of type "book" to some person, for example, in misc4dev: /cgi-bin/koha/circ/circulation.pl?borrowernumber=2&barcode=3999900000001 4. Open reserve/request, for example, for item 1 and patron 1 in misc4dev env (/cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1) 5. It does not allow to hold, both red crossed, but computer file says "Exceeded max holds per record" because of "0" limit set on step 1. 6. Apply the patch. 7. Reload page on step 5 and see that "book" will be available for hold, but "computer file" still will be red-crossed "Exceeded max holds per record", now that's correct because both items unavailable: one because on load, another because of "0" limit for computer files. 8. Check-in book from step 3 so it will be returned to the library, 9. Reload page on step 5 and see that again no any holds available, but it's now also correct: "book" now returned but "on shelf holds" set to "if all unavailable". -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I confirm this patch fixes another bug: If there is a biblio with 2 items having different itemtypes. First item is not available for holds, the other one is checked out, and available "if all unavailable". At the OPAC there was a blocking message "There are no items that can be placed on hold." With this patch I am able to place a hold on the second item. Technical comments: It seems ok, only minor changes: * perltidy a bit the tests * add tests in a subtest * AddReturn statement is not needed * it would be great to have IsAvailableForItemLevelRequest tested as well (just in case, especially because we are lacking test coverage for this circ rule! This comment does not block the signoff process. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #12 from Andrew Nugged <nugged@gmail.com> --- I thought to hold sign off for a few days and group tests and fix those tech comments Joubu requested... Or not needed? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I was just wondering, better to use "Failed QA" if you want to fix those things first - In Discussion can be a bit of a dead end at times as it indicated an unresolved disagreement. You could also leave as "needs sign-off" and add the tests as follow-up - judging from the last line of Joubu's comment. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105651|0 |1 is obsolete| | Attachment #105652|0 |1 is obsolete| | --- Comment #15 from Andrew Nugged <nugged@gmail.com> --- Created attachment 107072 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107072&action=edit Bug 24683: Subroutine name changed (fix), no code logic changed This is the intermediate refactor: renamed subroutine only. Naming mistake came because this sub is used to detect if anything available for hold, but it used in "if ANY UNAVAILABLE rule", so actually results of this sub negated (see below "return" in the code). In details: when previous refactor was done, name for subroutine was chosen wrongly in "opposite" direction from what it actually does: it was named "ItemsAnyAvailableForHold", but this subroutine gave truth (1) if at least one of the items available on shelf, not lost, not on loan, not held, and not restricted by smart rules and damaged status. So, if this sub says that item is still "available", this actually PREVENTS item from hold in parent sub (see negated return): sub IsAvailableForItemLevelRequest { ... my $any_available = ItemsAnyAvailableAndNotRestricted... return $any_available ? 0 : 1; # ^^^ if any available and not restricted - we don't allow # on-shelf holds ... I.e. like it named now: "ItemsAnyAvailableAndNotRestricted". Small aside fix: white space for '&&' inside brackets added to join operation by priority visually. Testing plan not needed: all places where sub used it just renamed. More: all this places/code was introduced in one older commit so there is also no overlaps or other calls/uses for this subroutine. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #17 from Andrew Nugged <nugged@gmail.com> --- Created attachment 107074 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107074&action=edit Bug 24683: Fix for take smart rules into account in "if all unavailable" Inside of ItemsAnyAvailableAndNotRestricted was no effect from main set of smart rules (per record and other limits): i.e. call to "CanItemBeReserved" was absent totally. Because of this there was a bug: for example none of two items were allowed to be held when first was allowed by one smart rule, BUT on loan, and second was disallowed by another smart rule (for example, 0 "Holds per record"), i.e. in this case both items unavailable: so on-shelf holds setting "allow hold if all unavailable" should allow to hold first one, and not the second one. But it was that both wasn't allowed to be held. Solution: call to sub "CanItemBeReserved" added so it checked for "...->{status} ne 'OK'" so now if item restricted by smart rule it also accounted as "unavailable" and "AnyAvailavble" not counts it. How to reproduce: 1. Add 2 smart rules (/cgi-bin/koha/admin/smart-rules.pl) with "on shelf holds": "if all unavailable" for all rules, no "item level holds", and set "holds per record" to 2 for "books" and "0" for "computer files". 2. Create only 2 items for one biblio, but different types, "book" and "computer file". For example in misc4dev env: /cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additem 3. Check out that item of type "book" to some person, for example, in misc4dev: /cgi-bin/koha/circ/circulation.pl?borrowernumber=2&barcode=3999900000001 4. Open reserve/request, for example, for item 1 and patron 1 in misc4dev env (/cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1) 5. It does not allow to hold, both red crossed, but computer file says "Exceeded max holds per record" because of "0" limit set on step 1. 6. Apply the patch. 7. Reload page on step 5 and see that "book" will be available for hold, but "computer file" still will be red-crossed "Exceeded max holds per record", now that's correct because both items unavailable: one because on load, another because of "0" limit for computer files. 8. Check-in book from step 3 so it will be returned to the library, 9. Reload page on step 5 and see that again no any holds available, but it's now also correct: "book" now returned but "on shelf holds" set to "if all unavailable". -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Andrew Nugged <nugged@gmail.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=24683 --- Comment #20 from Andrew Nugged <nugged@gmail.com> --- (In reply to Jonathan Druart from comment #10)

Technical comments:

* perltidy a bit the tests

done

* add tests in a subtest

done

* AddReturn statement is not needed

done

* it would be great to have IsAvailableForItemLevelRequest tested as well (just in case, especially because we are lacking test coverage for this circ rule!

done + also because this I propose (last 107076 patch) some clarification in subroutine description (I agree that's tricky logic but I reversed up by calls through code and that how it made and looks expected this sub behavior, so I just decided to note this in sub desc). Also, Jonathan: this have one more integral update: optimization added to cut of loop if first available found: thanks for Joonas for the idea. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Agustín Moyano <agustinmoyano@theke.io> 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=24683 --- Comment #23 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 107117 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107117&action=edit Bug 24683: Optimize loop in ItemsAnyAvailableAndNotRestricted Add cut-off shortcut (return from inside the loop) when first "Any Available And Not Restricted" item found, because one is enough for "Any". Testing: no change visible for code behavior/results, it is just faster because won't loop over the whole set. Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #25 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 107119 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=107119&action=edit Bug 24683: whole test formatted by 'perltidy' This is complementary patch using styling from bundled /xt/perltidyrc file Almost no code change except a few long constant strings broken to parts by concatenation. Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |agustinmoyano@theke.io --- Comment #27 from Agustín Moyano <agustinmoyano@theke.io> --- Great work Andrew! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #29 from Andrew Nugged <nugged@gmail.com> --- (In reply to Agustín Moyano from comment #27)

Great work Andrew!

Agustín: :safe_handshake: thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 clodagh.kerin@educampus.ie changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clodagh.kerin@educampus.ie -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24160 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107116|0 |1 is obsolete| | --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108905 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108905&action=edit Bug 24683: Subroutine name changed (fix), no code logic changed This is the intermediate refactor: renamed subroutine only. Naming mistake came because this sub is used to detect if anything available for hold, but it used in "if ANY UNAVAILABLE rule", so actually results of this sub negated (see below "return" in the code). In details: when previous refactor was done, name for subroutine was chosen wrongly in "opposite" direction from what it actually does: it was named "ItemsAnyAvailableForHold", but this subroutine gave truth (1) if at least one of the items available on shelf, not lost, not on loan, not held, and not restricted by smart rules and damaged status. So, if this sub says that item is still "available", this actually PREVENTS item from hold in parent sub (see negated return): sub IsAvailableForItemLevelRequest { ... my $any_available = ItemsAnyAvailableAndNotRestricted... return $any_available ? 0 : 1; # ^^^ if any available and not restricted - we don't allow # on-shelf holds ... I.e. like it named now: "ItemsAnyAvailableAndNotRestricted". Small aside fix: white space for '&&' inside brackets added to join operation by priority visually. Testing plan not needed: all places where sub used it just renamed. More: all this places/code was introduced in one older commit so there is also no overlaps or other calls/uses for this subroutine. Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107118|0 |1 is obsolete| | --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108907&action=edit Bug 24683: Fix for take smart rules into account in "if all unavailable" Inside of ItemsAnyAvailableAndNotRestricted was no effect from main set of smart rules (per record and other limits): i.e. call to "CanItemBeReserved" was absent totally. Because of this there was a bug: for example none of two items were allowed to be held when first was allowed by one smart rule, BUT on loan, and second was disallowed by another smart rule (for example, 0 "Holds per record"), i.e. in this case both items unavailable: so on-shelf holds setting "allow hold if all unavailable" should allow to hold first one, and not the second one. But it was that both wasn't allowed to be held. Solution: call to sub "CanItemBeReserved" added so it checked for "...->{status} ne 'OK'" so now if item restricted by smart rule it also accounted as "unavailable" and "AnyAvailavble" not counts it. How to reproduce: 1. Add 2 smart rules (/cgi-bin/koha/admin/smart-rules.pl) with "on shelf holds": "if all unavailable" for all rules, no "item level holds", and set "holds per record" to 2 for "books" and "0" for "computer files". 2. Create only 2 items for one biblio, but different types, "book" and "computer file". For example in misc4dev env: /cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additem 3. Check out that item of type "book" to some person, for example, in misc4dev: /cgi-bin/koha/circ/circulation.pl?borrowernumber=2&barcode=3999900000001 4. Open reserve/request, for example, for item 1 and patron 1 in misc4dev env (/cgi-bin/koha/reserve/request.pl?biblionumber=1&borrowernumber=1) 5. It does not allow to hold, both red crossed, but computer file says "Exceeded max holds per record" because of "0" limit set on step 1. 6. Apply the patch. 7. Reload page on step 5 and see that "book" will be available for hold, but "computer file" still will be red-crossed "Exceeded max holds per record", now that's correct because both items unavailable: one because on load, another because of "0" limit for computer files. 8. Check-in book from step 3 so it will be returned to the library, 9. Reload page on step 5 and see that again no any holds available, but it's now also correct: "book" now returned but "on shelf holds" set to "if all unavailable". Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #107120|0 |1 is obsolete| | --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108909&action=edit Bug 24683: IsAvailableForItemLevelRequest sub description expanded Signed-off-by: Agustin Moyano <agustinmoyano@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 --- Comment #38 from Andrew Nugged <nugged@gmail.com> --- I will port (also becase Hans Pålsson requested to have it on older version and I already did pre-test port for Hans earlier). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #40 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 20.05.x for 20.05.04 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Lisette Scheer <lisetteslatah@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26815 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24683 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28529 -- You are receiving this mail because: You are watching all bug changes.