Bug 27898 made PUT /holds/:hold_id handle x-koha-override correctly, but we didn't do that for the specific route.
Making it dependent on bug 27898 because of the test files.
Created attachment 120281 [details] [review] 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
Created attachment 120282 [details] [review] Bug 28254: Unit tests
Created attachment 120283 [details] [review] 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
Created attachment 120305 [details] [review] 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>
Created attachment 120306 [details] [review] Bug 28254: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 120307 [details] [review] 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>
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.
Created attachment 120337 [details] [review] 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>
Created attachment 120338 [details] [review] Bug 28254: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120339 [details] [review] 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>
Created attachment 120340 [details] [review] 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.
All works well and passes QA scripts. I added a followup for the consistency of parameter names after discussing it with Tomas. Passing QA
Created attachment 120457 [details] [review] 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.
Created attachment 120458 [details] [review] Bug 28254: Add missing tests for PUT and POST
Pushed to master for 21.05, thanks to everybody involved!
(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!
Depends on Bug 27898 not in 20.11.x