We need a clean and fully tested method to set a hold's pickup location. It should throw exceptions if: - No pickup location passed - The passed pickup location is not a valid one
Created attachment 114356 [details] [review] Bug 27209: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 114357 [details] [review] Bug 27209: Add Koha::Hold->set_pickup_location This patch introduces a method to safely update a hold's pickup location. It will raise exceptionis if the passed parameters are invalid or absent. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS; Tests pass 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 114374 [details] [review] Bug 27209: (follow-up) Add ->is_pickup_location_valid This patch simply refactors the pickup location check into a method that can be called on its own. Tests are added, and the tests for ->set_pickup_location should pass unmodified. To test: 1. Apply the first two patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: tests pass 3. Apply this refactoring patch 4. Notice the tests are similar, but check for boolean output 5. Repeat 2 => SUCCESS: New tests pass, set_pickup_location() behavior unchanged. 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 114378 [details] [review] Bug 27209: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 114379 [details] [review] Bug 27209: Add Koha::Hold->set_pickup_location This patch introduces a method to safely update a hold's pickup location. It will raise exceptionis if the passed parameters are invalid or absent. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.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>
Created attachment 114380 [details] [review] Bug 27209: (follow-up) Add ->is_pickup_location_valid This patch simply refactors the pickup location check into a method that can be called on its own. Tests are added, and the tests for ->set_pickup_location should pass unmodified. To test: 1. Apply the first two patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: tests pass 3. Apply this refactoring patch 4. Notice the tests are similar, but check for boolean output 5. Repeat 2 => SUCCESS: New tests pass, set_pickup_location() behavior unchanged. 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 114443 [details] [review] Bug 27209: Unit tests 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>
Created attachment 114444 [details] [review] Bug 27209: Add Koha::Hold->set_pickup_location This patch introduces a method to safely update a hold's pickup location. It will raise exceptionis if the passed parameters are invalid or absent. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.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>
Created attachment 114445 [details] [review] Bug 27209: (follow-up) Add ->is_pickup_location_valid This patch simply refactors the pickup location check into a method that can be called on its own. Tests are added, and the tests for ->set_pickup_location should pass unmodified. To test: 1. Apply the first two patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: tests pass 3. Apply this refactoring patch 4. Notice the tests are similar, but check for boolean output 5. Repeat 2 => SUCCESS: New tests pass, set_pickup_location() behavior unchanged. 6. 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>
Created attachment 114448 [details] [review] Bug 27209: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 114449 [details] [review] Bug 27209: Add Koha::Hold->set_pickup_location This patch introduces a method to safely update a hold's pickup location. It will raise exceptionis if the passed parameters are invalid or absent. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.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: Josef Moravec <josef.moravec@gmail.com>
Created attachment 114450 [details] [review] Bug 27209: (follow-up) Add ->is_pickup_location_valid This patch simply refactors the pickup location check into a method that can be called on its own. Tests are added, and the tests for ->set_pickup_location should pass unmodified. To test: 1. Apply the first two patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t => SUCCESS: tests pass 3. Apply this refactoring patch 4. Notice the tests are similar, but check for boolean output 5. Repeat 2 => SUCCESS: New tests pass, set_pickup_location() behavior unchanged. 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.01
Pushed to 20.05.x for 20.05.07
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.
Tried to backport to 19.11.x it because it's the dependency of a major bug (bug 27205) but the tests don't pass. Help needed here in order to backport bug 27205. kohadev-koha@6a2e5bab0d87:/kohadevbox/koha$ prove t/db_dependent/Koha/Hold.t t/db_dependent/Koha/Hold.t .. # No tests run! t/db_dependent/Koha/Hold.t .. 1/3 # Failed test 'No tests run for subtest "patron() tests"' # at t/db_dependent/Koha/Hold.t line 55. The method Koha::Hold->patron is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 661 Koha::Object::AUTOLOAD('Koha::Hold=HASH(0x556f6652aa00)') called at t/db_dependent/Koha/Hold.t line 50 main::__ANON__ at /usr/share/perl5/Test/Builder.pm line 310 eval {...} at /usr/share/perl5/Test/Builder.pm line 310 Test::Builder::subtest('Test::Builder=HASH(0x556f5d6558b0)', 'patron() tests', 'CODE(0x556f5d5c2918)') called at /usr/share/perl5/Test/More.pm line 807 Test::More::subtest('patron() tests', 'CODE(0x556f5d5c2918)') called at t/db_dependent/Koha/Hold.t line 55 # Looks like your test exited with 255 just after 1. t/db_dependent/Koha/Hold.t .. Dubious, test returned 255 (wstat 65280, 0xff00)