[Bug 28254] New: Make it possible to override rules in PUT /holds/:hold_id/pickup_location
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Bug ID: 28254 Summary: Make it possible to override rules in PUT /holds/:hold_id/pickup_location 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: tomascohen@gmail.com Bug 27898 made PUT /holds/:hold_id handle x-koha-override correctly, but we didn't do that for the specific route. -- 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=28254 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Blocks| |27865 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27865 [Bug 27865] Hold pickup location dropdown on patron pages should respect hold fulfillment policies -- 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=28254 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27898 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Making it dependent on bug 27898 because of the test files. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27898 [Bug 27898] Make PUT /holds/:hold_id handle x-koha-override for pickup locations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 120281 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120281&action=edit Bug 28254: Add 'override' parameter to Koha::Hold->set_pickup_location This patch adds a new parameter to the method. If passed+true it makes the method skip the pickup validation test. Tests and POD added for this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 120282 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120282&action=edit Bug 28254: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 120283 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120283&action=edit Bug 28254: Make PUT /holds/:hold_id/pickup_location honour x-koha-override This patch adds a new parameter (x-koha-override header) to the route, and makes the controller pass this information (override requested) when the AllowHoldPolicyOverride syspref is set, to Koha::Hold->set_pickup_location. This way, under certain conditions, we can be sure the update wont' fail. To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Kyle M Hall <kyle@bywatersolutions.com> 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=28254 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120281|0 |1 is obsolete| | Attachment #120282|0 |1 is obsolete| | Attachment #120283|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120305 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120305&action=edit Bug 28254: Add 'override' parameter to Koha::Hold->set_pickup_location This patch adds a new parameter to the method. If passed+true it makes the method skip the pickup validation test. Tests and POD added for this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120306 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120306&action=edit Bug 28254: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120307 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120307&action=edit Bug 28254: Make PUT /holds/:hold_id/pickup_location honour x-koha-override This patch adds a new parameter (x-koha-override header) to the route, and makes the controller pass this information (override requested) when the AllowHoldPolicyOverride syspref is set, to Koha::Hold->set_pickup_location. This way, under certain conditions, we can be sure the update wont' fail. To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- In the Transfers work I ended up with a 'force' parameter in a few places.. which was used to force an action that would normally be prevented by policy.. often with a check in the caller prior to calling with force. I wonder if we should converge on that term or 'override' as implemented here. Just some food for thought. The code looks good. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120305|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 120337 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120337&action=edit Bug 28254: Add 'override' parameter to Koha::Hold->set_pickup_location This patch adds a new parameter to the method. If passed+true it makes the method skip the pickup validation test. Tests and POD added for this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120306|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 120338 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120338&action=edit Bug 28254: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120307|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 120339 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120339&action=edit Bug 28254: Make PUT /holds/:hold_id/pickup_location honour x-koha-override This patch adds a new parameter (x-koha-override header) to the route, and makes the controller pass this information (override requested) when the AllowHoldPolicyOverride syspref is set, to Koha::Hold->set_pickup_location. This way, under certain conditions, we can be sure the update wont' fail. To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 120340 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120340&action=edit Bug 28254: (QA follow-up) override -> force After some discussion we decided that the parameter should be called 'force' for pushign through an action against policy. (as per the Transfers work). We decided to stick to 'override' at the API level however as it highlights the 'human interaction' element of the call. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All works well and passes QA scripts. I added a followup for the consistency of parameter names after discussing it with Tomas. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 120457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120457&action=edit Bug 28254: Simplify tests This is only cosmetic, and maybe personal preferences. It seems easier to create the library object with the pickup_library we want to use. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 120458 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120458&action=edit Bug 28254: Add missing tests for PUT and POST -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.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=28254 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.05, 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=28254 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #16)
Pushed to master for 21.05, thanks to everybody involved!
Thanks, and many thanks for the follow-ups! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28254 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 27898 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org