Summary: | Don't calculate overridden values when placing a hold via the REST API | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | REST API | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | danyonsewell, emily.lamancusa, jonathan.druart, lucas, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.06,22.05.14
|
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 33471 | ||
Attachments: |
Bug 33470: Don't calculate overridden hold limits
Bug 33470: Don't calculate overridden hold limits Bug 33470: Don't calculate overridden hold limits |
Description
Nick Clemens (kidclamp)
2023-04-09 18:24:54 UTC
Created attachment 149378 [details] [review] 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 Created attachment 149410 [details] [review] 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> Testing notes: Step 0 - Set AllowHoldPolicyOverride to "Allow" Please provide tests. Sorry, I misread there is no behaviour changes expected here. 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? (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? Hum indeed, sorry! Created attachment 150581 [details] [review] 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> Pushed to master for 23.05. Nice work everyone, thanks! Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release Backported to 22.05.x for upcoming 22.05.14 Does this need to be backported to 21.11.x? |