[Bug 33470] New: Don't calculate overridden values when placing a hold via the REST api
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Bug ID: 33470 Summary: Don't calculate overridden values when placing a hold via the REST api Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com CC: tomascohen@gmail.com When placing a hold via the REST api, we can pass an override to force the hold even if pickup locations are not valid, or hold is not allowed. When this is passed, the calculations that determine if the hold would be allowed are moot, and should not be performed -- 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=33470 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=33470 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 149378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149378&action=edit Bug 33470: Don't calculate overridden hold limits This patch can largely be checked in the code to ensure it makes sense, it moves a conditional up one level for the two different checks that could prevent holds To test: 1 - Attempt to place a hold via the REST API with an invalid pickup location Set pickup location as no in branches to make this easy 2 - Confirm you get an error 3 - Attempt again with header (bug 27760): x-koha-override: any 4 - Hold is placed 5 - Repeat 1-4 above but with an item that cannot be held 6 - Apply patch 7 - Repeat 1-4 above 8 - Results should be the same -- 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=33470 Nick Clemens <nick@bywatersolutions.com> 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=33470 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |33471 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33471 [Bug 33471] Improve performance of hold pickup location verification for next available holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #149378|0 |1 is obsolete| | --- Comment #2 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 149410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149410&action=edit Bug 33470: Don't calculate overridden hold limits This patch can largely be checked in the code to ensure it makes sense, it moves a conditional up one level for the two different checks that could prevent holds To test: 1 - Attempt to place a hold via the REST API with an invalid pickup location Set pickup location as no in branches to make this easy 2 - Confirm you get an error 3 - Attempt again with header (bug 27760): x-koha-override: any 4 - Hold is placed 5 - Repeat 1-4 above but with an item that cannot be held 6 - Apply patch 7 - Repeat 1-4 above 8 - Results should be the same Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off 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=33470 --- Comment #3 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Testing notes: Step 0 - Set AllowHoldPolicyOverride to "Allow" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Don't calculate overridden |Don't calculate overridden |values when placing a hold |values when placing a hold |via the REST api |via the REST API -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com Status|Signed Off |Failed QA --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Please provide tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Sorry, I misread there is no behaviour changes expected here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What's the point of the changes then? Be faster? There are behaviour changes, 400 instead of 403 in case of invalid pickup locations. Maybe that's why it has been written that way in the first place? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #6)
What's the point of the changes then? Be faster?
Yes, performance. Biblio->pickup_locations can take a long time when there are many items/branches
There are behaviour changes, 400 instead of 403 in case of invalid pickup locations. Maybe that's why it has been written that way in the first place?
(In reply to Jonathan Druart from comment #6)
What's the point of the changes then? Be faster?
There are behaviour changes, 400 instead of 403 in case of invalid pickup locations. Maybe that's why it has been written that way in the first place?
I get a 400 for an invalid pickup location before and after, can you describe the case you are seeing changed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Hum indeed, sorry! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #149410|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150581&action=edit Bug 33470: Don't calculate overridden hold limits This patch can largely be checked in the code to ensure it makes sense, it moves a conditional up one level for the two different checks that could prevent holds To test: 1 - Attempt to place a hold via the REST API with an invalid pickup location Set pickup location as no in branches to make this easy 2 - Confirm you get an error 3 - Attempt again with header (bug 27760): x-koha-override: any 4 - Hold is placed 5 - Repeat 1-4 above but with an item that cannot be held 6 - Apply patch 7 - Repeat 1-4 above 8 - Results should be the same Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> 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=33470 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. 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=33470 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00,22.11.06 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=33470 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Many hands makes light work, thankyou everyone! Pushed to 22.11.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=33470 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable Version(s)|23.05.00,22.11.06 |23.05.00,22.11.06,22.05.14 released in| | --- Comment #12 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for upcoming 22.05.14 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33470 Danyon Sewell <danyonsewell@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |danyonsewell@catalyst.net.n | |z --- Comment #13 from Danyon Sewell <danyonsewell@catalyst.net.nz> --- Does this need to be backported to 21.11.x? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org