[Bug 30860] New: Add option for CanBookBeReserved to return all item values
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Bug ID: 30860 Summary: Add option for CanBookBeReserved to return all item values Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com When placing a request we call CanBookBeReserved and loop items, calling CanItemBeReserved until we ifnd one that can fill a hold. We then loop CanItemBeReserved to check which items can be held. In some cases it would be nice to get the return values for all items passed back and avoid needing to call again -- 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=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=30860 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135426 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135426&action=edit Bug 30860: Add return_all_items parameter to CanBookBeReserved -- 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=30860 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135427 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135427&action=edit Bug 30860: Unit test -- 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=30860 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135428 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135428&action=edit Bug 30860: Return all items from CanBookBeReserved on request.pl This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before -- 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=30860 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 135429 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135429&action=edit Bug 30860: Get all items info for CanBookBeReserved on opac-reserve.pl Repeat test above on OPAC -- 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=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=30860 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrewfh@dubcolib.org Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135426|0 |1 is obsolete| | Attachment #135427|0 |1 is obsolete| | Attachment #135428|0 |1 is obsolete| | Attachment #135429|0 |1 is obsolete| | --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144480 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144480&action=edit Bug 30860: Add return_all_items parameter to CanBookBeReserved -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144481 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144481&action=edit Bug 30860: Unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144482 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144482&action=edit Bug 30860: Return all items from CanBookBeReserved on request.pl This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144483 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144483&action=edit Bug 30860: Get all items info for CanBookBeReserved on opac-reserve.pl Repeat test above on OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144480|0 |1 is obsolete| | --- Comment #9 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 144506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144506&action=edit Bug 30860: Add return_all_items parameter to CanBookBeReserved Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144481|0 |1 is obsolete| | --- Comment #10 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 144507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144507&action=edit Bug 30860: Unit test Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144482|0 |1 is obsolete| | --- Comment #11 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 144508 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144508&action=edit Bug 30860: Return all items from CanBookBeReserved on request.pl This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144483|0 |1 is obsolete| | --- Comment #12 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 144509 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144509&action=edit Bug 30860: Get all items info for CanBookBeReserved on opac-reserve.pl Repeat test above on OPAC Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #13 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- Whoops, I added a signoff via a sandbox, was about to change the status here, and realized I hadn't tested on the OPAC. My bib has 12 items. When trying to place a hold via the staff client, 2 items show as holdable. The other 10 are not holdable: 1 is damaged, 1 is not for loan, the other 8 don't match the patron's library. When trying to place a hold on the OPAC for the same title and patron, I'm told there are no items that can be placed on hold. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Andrew Fuerste-Henry from comment #13)
Whoops, I added a signoff via a sandbox, was about to change the status here, and realized I hadn't tested on the OPAC.
My bib has 12 items. When trying to place a hold via the staff client, 2 items show as holdable. The other 10 are not holdable: 1 is damaged, 1 is not for loan, the other 8 don't match the patron's library.
When trying to place a hold on the OPAC for the same title and patron, I'm told there are no items that can be placed on hold.
In this case I believe the correct response is no items available on either interface, and that is what I see when I try to recreate. Can you retest, and share a screen shot of the record if you recreate? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144506|0 |1 is obsolete| | Attachment #144507|0 |1 is obsolete| | Attachment #144508|0 |1 is obsolete| | Attachment #144509|0 |1 is obsolete| | --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145577&action=edit Bug 30860: Add return_all_items parameter to CanBookBeReserved Current status: Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145578&action=edit Bug 30860: Unit test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145579&action=edit Bug 30860: Return all items from CanBookBeReserved on request.pl This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145580 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145580&action=edit Bug 30860: Get all items info for CanBookBeReserved on opac-reserve.pl Repeat test above on OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145577|0 |1 is obsolete| | --- Comment #19 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 145581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145581&action=edit Bug 30860: Add return_all_items parameter to CanBookBeReserved -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145578|0 |1 is obsolete| | --- Comment #20 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 145582 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145582&action=edit Bug 30860: Unit test Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145579|0 |1 is obsolete| | --- Comment #21 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 145583 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145583&action=edit Bug 30860: Return all items from CanBookBeReserved on request.pl This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145580|0 |1 is obsolete| | --- Comment #22 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 145584 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145584&action=edit Bug 30860: Get all items info for CanBookBeReserved on opac-reserve.pl Repeat test above on OPAC Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Andrew Fuerste-Henry <andrewfh@dubcolib.org> 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=30860 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Can you please take a look? FAIL t/db_dependent/Reserves.t FAIL valid "my" variable $can_be_all masks earlier declaration in same scope -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #24 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 147695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147695&action=edit Bug 30860: (follow-up) Remove repeated variable declaration -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #25 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I get the idea, but the implementation is weird. Why don't you simply cache the items holdability info in L1? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #26 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #25)
I get the idea, but the implementation is weird. Why don't you simply cache the items holdability info in L1?
"Simply" => It's not as simple as what you wrote. The idea is simple, not the implementation. But maybe it's worth spending time on it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #27 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #25)
I get the idea, but the implementation is weird. Why don't you simply cache the items holdability info in L1?
I didn't think it was weird, it seemed straightforward to me. Rather than put in cache, retrieve from cache we just call and get all the values. I think moving to L1 cache would be a great patch if you want to write it :-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #28 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #26)
(In reply to Jonathan Druart from comment #25)
I get the idea, but the implementation is weird. Why don't you simply cache the items holdability info in L1?
"Simply" => It's not as simple as what you wrote. The idea is simple, not the implementation. But maybe it's worth spending time on it?
I was thinking about the daemon, but maybe we deal with that already (there is a flush in rebuild_zebra). So it may be pretty easy actually. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #29 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 148562 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148562&action=edit Bug 30860: [alternative patch] Cache CanItemBeReserved return value Instead of hacking CanBookBeReserved this patch suggests to simply cache the return value of CanItemBeReserved that could be then returned *on demand* We don't want to introduce side-effects hard to catch from this simple change, so let's return the cache value only from the 2 scripts we are dealing with. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #30 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #29)
Created attachment 148562 [details] [review] Bug 30860: [alternative patch] Cache CanItemBeReserved return value
Instead of hacking CanBookBeReserved this patch suggests to simply cache the return value of CanItemBeReserved that could be then returned *on demand* We don't want to introduce side-effects hard to catch from this simple change, so let's return the cache value only from the 2 scripts we are dealing with.
This works, and in initial testing seems to offer improvement. I have one hesitation Current: code calls CanBookBeReserved, which exits on first holdable item, then we call CanItemBeReserved for every item Worst case, last item is holdable - for 100 items we call CanItemBeReserved 200 times My patches: code calls CanBookBeReserved which in turn calls CanItemBeReserved for every item. Then we loop over the results for each item Worst case, we call CanItemBeReserved 100 times - we do this no matter what Alternate patches: code calls CanBookBeReserved which exits on first holdable item we then retrieve any alreayd calculated from cache, then we cache each result as we go forward. Worst case - first item is holdable - we retrieve a single value from cache, then cache every subsequent CanItemBeReserved call and never use it - though we still only call CanItemBeReserved 100 times max I don't know what the actual tendency for number of items holdable, but it feels like we can do a lot of caching that we don't necessarily need. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #31 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Nick Clemens from comment #30)
I don't know what the actual tendency for number of items holdable, but it feels like we can do a lot of caching that we don't necessarily need.
We can consider L1 caching insignificant in term of performance (unless we are caching huge structures!). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145581|0 |1 is obsolete| | Attachment #145582|0 |1 is obsolete| | Attachment #145583|0 |1 is obsolete| | Attachment #145584|0 |1 is obsolete| | Attachment #147695|0 |1 is obsolete| | Attachment #148562|0 |1 is obsolete| | --- Comment #32 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 148812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148812&action=edit Bug 30860: Cache CanItemBeReserved return value This patch caches the return value of CanItemBeReserved that could be then returned *on demand* We don't want to introduce side-effects hard to catch from this simple change, so let's return the cache value only from the 2 scripts we are dealing with. This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |33302 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33302 [Bug 33302] Placing item level holds in OPAC allows to pick forbidden pick-up locations, but then places no hold -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #148812|0 |1 is obsolete| | --- Comment #33 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 149124 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149124&action=edit Bug 30860: Cache CanItemBeReserved return value This patch caches the return value of CanItemBeReserved that could be then returned *on demand* We don't want to introduce side-effects hard to catch from this simple change, so let's return the cache value only from the 2 scripts we are dealing with. This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nick@bywatersolutions.com |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA 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=30860 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #149124|0 |1 is obsolete| | --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 149377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149377&action=edit Bug 30860: Cache CanItemBeReserved return value This patch caches the return value of CanItemBeReserved that could be then returned *on demand* We don't want to introduce side-effects hard to catch from this simple change, so let's return the cache value only from the 2 scripts we are dealing with. This patch requests all item values from CanBookBeReserved on request.pl Before this we either: - Looped every item to find out that book could not be reserved - Looped until we found an item that could be reserved, then looped all items to get statuses In the worst case we avoid double processing a single item, in the best case we avoid double processing all items (if only last on record is holdable) To test: 1 - Find a record in staff client with several items 2 - Set AllowHoldsOnDamagedItems to 'Dont allow' 3 - Add a damaged item to record 4 - Set a hold rule to only allow holds form homebranch and ensure record has items from other branches 5 - Setup things to prevent more items from being held 6 - Attempt hold for patron 7 - Note item statuses 8 - Apply patch 9 - Confirm statuses are as they were before Signed-off-by: Nick Clemens <nick@bywatersolutions.com> 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=30860 --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Also ran prove t/db_dependent/Holds.t t/db_dependent/Circulation.t Circulation.t gave me some trouble in an unrelated test, but results between master and patched branch match. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.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=30860 --- Comment #36 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- This is so ugly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #37)
This is so ugly
Writing that about your own creation? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #38)
(In reply to Jonathan Druart from comment #37)
This is so ugly
Writing that about your own creation?
I would not date otherwise! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00 |23.11.00,23.05.02 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for all the hard work! Pushed to 23.05.x for the next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #41 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Enhancement. Not pushing to 22.11.x. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Severity|enhancement |minor Summary|Add option for |Performance: Add option for |CanBookBeReserved to return |CanBookBeReserved to return |all item values |all item values -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Bug 30860 depends on bug 33302, which changed state. Bug 33302 Summary: Placing item level holds in OPAC allows to pick forbidden pick-up locations, but then places no hold https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33302 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |7923 CC| |fridolin.somers@biblibre.co | |m Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7923 [Bug 7923] Performance omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30860 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32476 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org