[Bug 27205] New: Hold routes are not dealing with invalid pickup locations
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Bug ID: 27205 Summary: Hold routes are not dealing with invalid pickup locations Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com It's possible to PUT or POST /holds with invalid pickup locations. As the staff cannot overwrite invalid pickup locations (to confirmed), we should raise an exception from AddReserve and ModReserve if called with invalid pickup locations. -- 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=27205 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18729 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18729 [Bug 18729] Librarian unable to update hold pickup library from patron pages without "modify_holds_priority" permission -- 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=27205 Tomás Cohen Arazi <tomascohen@gmail.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=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master -- 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=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED -- 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=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27209 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27209 [Bug 27209] Add Koha::Hold->set_pickup_location -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|27209 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27209 [Bug 27209] Add Koha::Hold->set_pickup_location -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114358&action=edit Bug 27205: Check valid pickup location on PUT /holds/:hold_id This patch adds a test for valid pickup locations when updating a hold through the API. Tests are adjusted to reflect this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I submit this patch for review. If the approach is accepted, I will adjust the POST route too. The tests are obviously the difficult bit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #3 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Even after restarting the services, the tests don't pass: kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. 10/11 # Failed test '400 Bad Request' # at t/db_dependent/api/v1/holds.t line 817. # got: '500' # expected: '400' # Failed test '400 Bad Request' # at t/db_dependent/api/v1/holds.t line 850. # got: '500' # expected: '400' # Looks like you failed 2 tests of 12. t/db_dependent/api/v1/holds.t .. 11/11 # Failed test 'edit() tests' # at t/db_dependent/api/v1/holds.t line 868. # Looks like you failed 1 test of 11. t/db_dependent/api/v1/holds.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/11 subtests Test Summary Report ------------------- t/db_dependent/api/v1/holds.t (Wstat: 256 Tests: 11 Failed: 1) Failed test: 11 Non-zero exit status: 1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27209 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27209 [Bug 27209] Add Koha::Hold->set_pickup_location -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114358|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |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=27205 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #3)
Even after restarting the services, the tests don't pass:
Sorry for that, it was a missing dependency on the bug. Anyway, I updated the dependency, and this bug, so better start over. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114375 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114375&action=edit Bug 27205: Check valid pickup location on PUT /holds/:hold_id This patch adds a test for valid pickup locations when updating a hold through the API. Tests are adjusted to reflect this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114375|0 |1 is obsolete| | --- Comment #6 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 114377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114377&action=edit Bug 27205: Check valid pickup location on PUT /holds/:hold_id This patch adds a test for valid pickup locations when updating a hold through the API. Tests are adjusted to reflect this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114382&action=edit Bug 27205: Check valid pickup location on POST /holds This patch adds a check for valid pickup location to the POST /holds route. A 400 code is returned if the supplied pickup library is not valid. To test: 1. Apply this patch 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=27205 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114382|0 |1 is obsolete| | --- Comment #8 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 114384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114384&action=edit Bug 27205: Check valid pickup location on POST /holds This patch adds a check for valid pickup location to the POST /holds route. A 400 code is returned if the supplied pickup library is not valid. To test: 1. Apply this patch 2. Run; $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Victor Grousset/tuxayo <victor@tuxayo.net> 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=27205 Kyle M Hall <kyle@bywatersolutions.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=27205 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114377|0 |1 is obsolete| | Attachment #114384|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 114446 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114446&action=edit Bug 27205: Check valid pickup location on PUT /holds/:hold_id This patch adds a test for valid pickup locations when updating a hold through the API. Tests are adjusted to reflect this change. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=27205 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 114447 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114447&action=edit Bug 27205: Check valid pickup location on POST /holds This patch adds a check for valid pickup location to the POST /holds route. A 400 code is returned if the supplied pickup library is not valid. To test: 1. Apply this patch 2. Run; $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=27205 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 114512 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114512&action=edit Bug 27205: Improve logic readability in conditional -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Tomas, do you agree it reads better like that? - unless ( - !defined $pickup_library_id - or $hold->is_pickup_location_valid( - { library_id => $pickup_library_id } - ) + if ( + defined $pickup_library_id + and not $hold->is_pickup_location_valid({ library_id => $pickup_library_id }) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #12)
Tomas, do you agree it reads better like that?
+1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=27205 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 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=27205 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|21.05.00 |21.05.00,20.11.01 released in| | --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.01 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable Version(s)|21.05.00,20.11.01 |21.05.00,20.11.01,20.05.07 released in| | --- Comment #16 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Bug 27205 depends on bug 27209, which changed state. Bug 27209 Summary: Add Koha::Hold->set_pickup_location https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27209 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |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=27205 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED --- Comment #17 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Missing dependencies for 19.11.x, can't backport. Send help to bug 27209 if you need this one in 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27205 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27865 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 watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org