https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #1 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96079 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96079&action=edit Bug 24185: 'If all unavailable' for 'on shelf holds' makes holds page slow When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items+patron, it goes very slow. This happens because in subloop there are re-checking if all other item unavailable so it is O(n^2) and it re-checks each time a lot of the same info for each item with repeating DB/data requests, these three: - GetBranchItemRule (per item type) - IsItemOnHoldAndFound (per item) - notforloan (per item) Proposed fix: I created patch to have in-variable cache in upper-level subroutine before entering the first "items" loop and passing that hash reference down into subs to cache these three values by keys per-item and per-item-type. It is simplest way to make it fast without re-thinking pretty complicated whole algorithm and business logic, and, when tested by speed it gave big improvement in execution time. How to reproduce: 1) on fresh installed kohadevbox create/import one book, record that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", so that rule will match above book items/library/patron, check that rule to have non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create direct link by yourself, for example in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time. I tested on my computer in VirtualBox for this page generation times, (stats below is for many runs and smallest value taken from similar in a repetition) *** "On shelf holds allowed" != "If all unavailable": 100 items: 5 seconds, 200 items: 10 seconds, 300 items: 11 seconds, *** "On shelf holds allowed" == "If all unavailable": 100 items: 50 seconds, 200 items: 3.1 minutes, 300 items: 7.0 minutes, *** "On shelf holds allowed" == "If all unavailable" with cache: 100 items: 7 seconds, 200 items: 20 seconds, 300 items: 38 seconds, Upper last one block with times with this in-var-cache solution. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #96079|0 |1 is obsolete| | --- Comment #2 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96080 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96080&action=edit Bug 24185: 'If all unavailable' for 'on shelf holds' makes holds page slow When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items+patron, it goes very slow. This happens because in subloop there are re-checking if all other item unavailable so it is O(n^2) and it re-checks each time a lot of the same info for each item with repeating DB/data requests, these three: - GetBranchItemRule (per item type) - IsItemOnHoldAndFound (per item) - notforloan (per item) Fix: Use in-variable cache in upper-level subroutine before entering the first "items" loop and passing that hash reference down into subs to cache these three values by keys per-item and per-item-type. It is simplest way to make it fast without re-thinking pretty complicated whole algorithm and business logic, and, when tested by speed it gave big improvement in execution time. Also "$can_item_be_reserved->{status} eq 'OK'" moved in "&&" before IsAvailableForItemLevelRequest call to cut away if it false before calling to subroutine. How to reproduce: 1) on fresh installed kohadevbox create/import one book, record that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", so that rule will match above book items/library/patron, check that rule to have non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create direct link by yourself, for example in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time. I tested on my computer in VirtualBox for this page generation times, (stats below is for many runs and smallest value taken from similar in a repetition) *** "On shelf holds allowed" != "If all unavailable": 100 items: 5 seconds, 200 items: 10 seconds, 300 items: 11 seconds, *** "On shelf holds allowed" == "If all unavailable": 100 items: 50 seconds, 200 items: 3.1 minutes, 300 items: 7.0 minutes, *** "On shelf holds allowed" == "If all unavailable" with cache: 100 items: 7 seconds, 200 items: 20 seconds, 300 items: 38 seconds, Upper last one block with times with this in-var-cache solution. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #96080|0 |1 is obsolete| | --- Comment #4 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96151 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96151&action=edit Bug 24185: Make holds page fast when 'on shelf holds' set to 'If all unavailable' When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items+patron, it goes very slow. This happens because in subloop there are re-checking if all other item unavailable so it is O(n^2) and it re-checks each time a lot of the same info for each item with repeating DB/data requests, these three: - GetBranchItemRule (per item type) - IsItemOnHoldAndFound (per item) - notforloan (per item) Fix: Use in-variable cache in upper-level subroutine before entering the first "items" loop and passing that hash reference down into subs to cache these three values by keys per-item and per-item-type. It is simplest way to make it fast without re-thinking pretty complicated whole algorithm and business logic, and, when tested by speed it gave big improvement in execution time. Also "$can_item_be_reserved->{status} eq 'OK'" moved in "&&" before IsAvailableForItemLevelRequest call to cut away if it false before calling to subroutine. How to reproduce: 1) on fresh installed kohadevbox create/import one book, record that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", so that rule will match above book items/library/patron, check that rule to have non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create direct link by yourself, for example in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time. I tested on my computer in VirtualBox for this page generation times, (stats below is for many runs and smallest value taken from similar in a repetition) (NOT buggy clause, just example how it fast when it not enters that "subloop"): *** "On shelf holds allowed" != "If all unavailable": 100 items: 5 seconds, 200 items: 10 seconds, 300 items: 11 seconds, (buggy clause): *** "On shelf holds allowed" == "If all unavailable": 100 items: 50 seconds, 200 items: 3.1 minutes, 300 items: 7.0 minutes, (same as above clause but fixed with caching in hash): *** "On shelf holds allowed" == "If all unavailable" with cache: 100 items: 7 seconds, 200 items: 20 seconds, 300 items: 38 seconds, Upper last one block with times with this in-var-cache solution. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Make holds page fast when |'If all unavailable' state |'on shelf holds' set to 'If |for 'on shelf holds' makes |all unavailable' |holds page very slow if | |there's a lot of items -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #96151|0 |1 is obsolete| | --- Comment #5 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96354 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96354&action=edit Bug 24185: Make holds page fast when 'on shelf holds' set to 'If all unavailable' When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items + patron, it goes slow. It happens because in subloop there is checking if all items available so it is O(n^2) and it re-checks each time the same info for each item with repeating DB/data requests. Fix: The inner loop 1:1 picked out from another loop and put before that items processing loop, 'items_any_available' variable precalculated, this variable passed to IsAvailableForItemLevelRequest to be used inside as the precalculated result. On tests it gave a drastic improvement in execution time. Also "$can_item_be_reserved->{status} eq 'OK'" moved in "&&" before IsAvailableForItemLevelRequest call to cut away if it false before calling to subroutine. How to reproduce: 1) on freshly installed kohadevbox create/import one book, remember that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up a single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", that rule should match above book items/library/patron, check that rule to have a non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create a direct link by yourself, for example, in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time on old code, densely increasing for every hundred items added. In the case of this solution, it's fast, and time increases a little only, linear. I tested on my computer in VirtualBox for page generation times, did 3-5 runs for same case to check if results are stable, and got such values: (old code): 100 items: 38 seconds 200 items: 2.5 minutes 300 items: 5.4 minutes (version with fix): 100 items: 3.7 seconds 200 items: 6.4 seconds 300 items: 8.7 seconds -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #96354|0 |1 is obsolete| | --- Comment #7 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96460 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96460&action=edit Bug 24185: Make holds page faster - improved "if" `$can_item_be_reserved eq 'OK'` moved in `&&` before `IsAvailableForItemLevelRequest` to cut away with static known values before calling to more resource consuming subroutine. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #9 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96464 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96464&action=edit Bug 24185: Make holds page fast when 'on shelf holds' set to 'If all unavailable' When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items + patron, it goes slow. It happens because in subloop there is checking if all items available so it is O(n^2) and it re-checks each time the same info for each item with repeating DB/data requests. Fix: The inner loop 1:1 picked out from another loop and put before that items processing loop, 'items_any_available' variable precalculated, this variable passed to IsAvailableForItemLevelRequest to be used inside as the precalculated result. On tests it gave a drastic improvement in execution time. How to reproduce: 1) on freshly installed kohadevbox create/import one book, remember that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up a single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", that rule should match above book items/library/patron, check that rule to have a non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create a direct link by yourself, for example, in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time on old code, densely increasing for every hundred items added. In the case of this solution, it's fast, and time increases a little only, linear. I tested on my computer in VirtualBox for page generation times, did 3-5 runs for same case to check if results are stable, and got such values: (old code): 100 items: 38 seconds 200 items: 2.5 minutes 300 items: 5.4 minutes (version with fix): 100 items: 3.7 seconds 200 items: 6.4 seconds 300 items: 8.7 seconds -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #11 from Andrew Nugged <nugged@gmail.com> --- Created attachment 96492 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=96492&action=edit Bug 24185: Make holds page faster - call to subroutine moved below "if" `$on_shelf_holds` was assigned before "return .. if" but not used in that code piece, so sometimes it was useless. Moved assignment after "return .. if". -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 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=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |String patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #15 from Andrew Nugged <nugged@gmail.com> --- Created attachment 97376 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97376&action=edit Bug 24185: Make holds page faster - call to subroutine moved below "if" `$on_shelf_holds` was assigned before "return .. if" but not used in that code piece, so sometimes it was useless. Moved assignment after "return .. if". -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #16 from Andrew Nugged <nugged@gmail.com> --- Created attachment 97377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97377&action=edit Bug 24185: Make holds page faster: preparatory refactoring This is just refactoring. extracting logically independent code to separate sub + tests update. No logic change yet. Searching for "any_available" item among all biblionumber items was done inside of "elsif on_shelf_holds == 2", and it is logically very independent piece of code (this "@items" loop), it needs just biblionumber and patron as parameters so it can be extracted into separate subroutine, and later also called/reused from somewhere else. This ability to call from another place also made for future patch to remove O(n^2) problem with nested loops. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@bugs.koha-c |y.org |ommunity.org CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Andrew, that makes sense to me, thanks for the excellent writing of these patches (split, comments and commit messages are perfect!). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97375|0 |1 is obsolete| | Attachment #97376|0 |1 is obsolete| | Attachment #97377|0 |1 is obsolete| | Attachment #97378|0 |1 is obsolete| | --- Comment #19 from Andrew Nugged <nugged@gmail.com> --- Created attachment 101355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101355&action=edit Bug 24185: Make holds page faster - improved "if" `$can_item_be_reserved eq 'OK'` moved in `&&` before `IsAvailableForItemLevelRequest` to cut away with static known values before calling to more resource consuming subroutine. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #21 from Andrew Nugged <nugged@gmail.com> --- Created attachment 101357 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101357&action=edit Bug 24185: Make holds page faster: preparatory refactoring This is just refactoring. extracting logically independent code to separate sub + tests update. No logic change yet. Searching for "any_available" item among all biblionumber items was done inside of "elsif on_shelf_holds == 2", and it is logically very independent piece of code (this "@items" loop), it needs just biblionumber and patron as parameters so it can be extracted into separate subroutine, and later also called/reused from somewhere else. This ability to call from another place also made for future patch to remove O(n^2) problem with nested loops. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #22 from Andrew Nugged <nugged@gmail.com> --- Created attachment 101358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101358&action=edit Bug 24185: Make holds page fast when 'on shelf holds' set to 'If all unavailable' When "reserve/request.pl -> C4/Reserves.pm::IsAvailableForItemLevelRequest" called many times with hundred of items and "on shelf holds" parameter set to "If all unavailable" for these items + patron, it goes slow. It happens because in subloop it is checking if all items available so it is O(n^2) and it re-checks each time the same info for each item with repeating DB/data requests. Fix: The inner loop 1:1 picked out into separate subroutine and called outside of the loop, saving data in 'items_any_available' variable once, this variable passed to IsAvailableForItemLevelRequest to be used inside as the precalculated result. This made algorithm O(n) instead of O(n^2) so there is noticeable speed increase. How to reproduce: 1) on freshly installed kohadevbox create/import one book, remember that biblionumber for later use it in down below, 2) add 100 items for that book for some library, 3) find some patron, that patron's card number we will use as a borrower down below to open holds page, 4) check for the rule or set up a single circulation rule in admin "/cgi-bin/koha/admin/smart-rules.pl", that rule should match above book items/library/patron, check that rule to have a non-zero number of holds (total, daily, count) allowed, and, IMPORTANT: set up "On shelf holds allowed" to "If all unavailable", ("item level holds" doesn't matter). 5) open "Home > Catalog > THAT_BOOK > Place a hold on THAT_BOOK" page ("holds" tab), and enter patron code in the search field, or you can create a direct link by yourself, for example, in my case it was: /cgi-bin/koha/reserve/request.pl?biblionumber=4&findborrower=23529000686353 6) it should be pretty long page generation time on old code, densely increasing for every hundred items added. In the case of this solution, it's fast, and time increases a little only, linear. I tested on my computer in VirtualBox for page generation times, did 3-5 runs for same case to check if results are stable, and got such values: (old code): 100 items: 50 seconds 200 items: 3.2 minutes 300 items: 7.3 minutes (version with fix): 100 items: 4.4 seconds 200 items: 7.5 seconds 300 items: 10.4 seconds -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101355|0 |1 is obsolete| | --- Comment #23 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 101607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101607&action=edit Bug 24185: Make holds page faster - improved "if" `$can_item_be_reserved eq 'OK'` moved in `&&` before `IsAvailableForItemLevelRequest` to cut away with static known values before calling to more resource consuming subroutine. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101357|0 |1 is obsolete| | --- Comment #25 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 101609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101609&action=edit Bug 24185: Make holds page faster: preparatory refactoring This is just refactoring. extracting logically independent code to separate sub + tests update. No logic change yet. Searching for "any_available" item among all biblionumber items was done inside of "elsif on_shelf_holds == 2", and it is logically very independent piece of code (this "@items" loop), it needs just biblionumber and patron as parameters so it can be extracted into separate subroutine, and later also called/reused from somewhere else. This ability to call from another place also made for future patch to remove O(n^2) problem with nested loops. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #27 from Andrew Nugged <nugged@gmail.com> --- oh, thanks for REBASE! In a case, if another assistance needed – I am monitoring this actively. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #29 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- (In reply to Andrew Nugged from comment #27)

oh, thanks for REBASE! In a case, if another assistance needed – I am monitoring this actively.

Your welcome, please check its OK -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101607|0 |1 is obsolete| | Attachment #101608|0 |1 is obsolete| | Attachment #101609|0 |1 is obsolete| | Attachment #101611|0 |1 is obsolete| | --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101613 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101613&action=edit Bug 24185: Make holds page faster - improved "if" `$can_item_be_reserved eq 'OK'` moved in `&&` before `IsAvailableForItemLevelRequest` to cut away with static known values before calling to more resource consuming subroutine. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101615&action=edit Bug 24185: Make holds page faster: preparatory refactoring This is just refactoring. extracting logically independent code to separate sub + tests update. No logic change yet. Searching for "any_available" item among all biblionumber items was done inside of "elsif on_shelf_holds == 2", and it is logically very independent piece of code (this "@items" loop), it needs just biblionumber and patron as parameters so it can be extracted into separate subroutine, and later also called/reused from somewhere else. This ability to call from another place also made for future patch to remove O(n^2) problem with nested loops. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Amended patch, I moved up the last line of the test, sounds more logical to me. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Excellent work, thanks everyone! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 --- Comment #37 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Andrew Nugged from comment #38)

I've backported this to v19.11.x because people requested: where I should push patches for 19.11.06? Should I create a separate ticket?

Usually the same bug is used and you can state the version in your commit message for example by adding [19.11] at the beginning of the subject line so it shows in the list of patches here. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24185 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26523 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26523 [Bug 26523] In reserve/request.pl biblioitemnumber is used where biblionumber is required. -- You are receiving this mail because: You are watching all bug changes.