From 3e8a4e7bb5099eca1e6e678808dc6ace3bee803c Mon Sep 17 00:00:00 2001 From: Petro Vashchuk Date: Wed, 19 Jan 2022 18:01:15 +0200 Subject: [PATCH] Bug 29906: fix when hold record forcibly gets unwanted "suspended" state This code allows for "Perl false" to pass through to $suspended_until if it is in $hold->suspend_until and in $body->{suspended_until} But in case of "Perl truth" it calls dt_from_string for that value. Reproduction: 1. run provided test in kshell: prove t/db_dependent/api/v1/holds.t 2. see the test fails with something like: # Failed test 'Location change shouldn't touch suspended status' # at t/db_dependent/api/v1/holds.t line 1067. # got: '1' # expected: '0' # Failed test 'suspended_until should be undef' # at t/db_dependent/api/v1/holds.t line 1068. # got: '2022-01-20 00:00:00' # expected: undef # Looks like you failed 2 tests of 39. 3. apply the changes in this patch to Koha/REST/V1/Holds.pm 4. run provided test in kshell: prove t/db_dependent/api/v1/holds.t 5. test will pass. --- Koha/REST/V1/Holds.pm | 4 ++-- t/db_dependent/api/v1/holds.t | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 0ca60b1459..f5cfb1d3af 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -282,8 +282,8 @@ sub edit { # suspended_until can also be set to undef my $suspended_until = exists $body->{suspended_until} - ? dt_from_string( $body->{suspended_until}, 'rfc3339' ) - : dt_from_string( $hold->suspend_until, 'iso' ); + ? $body->{suspended_until} && dt_from_string( $body->{suspended_until}, 'rfc3339' ) + : $hold->suspend_until && dt_from_string( $hold->suspend_until, 'iso' ); my $params = { reserve_id => $hold_id, diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 006c61659a..be52cb2593 100755 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -892,7 +892,7 @@ subtest 'pickup_locations() tests' => sub { subtest 'edit() tests' => sub { - plan tests => 37; + plan tests => 39; $schema->storage->txn_begin; @@ -1010,6 +1010,9 @@ subtest 'edit() tests' => sub { branchcode => $library_3->branchcode, itemnumber => $item->itemnumber, priority => 1, + suspend => 0, + suspend_until => undef, + waitingdate => undef } } ); @@ -1061,7 +1064,11 @@ subtest 'edit() tests' => sub { $item_hold->discard_changes; is( $item_hold->branchcode, $library_2->id, 'Pickup location changed correctly' ); + is( $item_hold->suspend, 0, 'Location change shouldn\'t touch suspended status' ); + is( $item_hold->suspend_until, undef, 'suspended_until should be undef' ); + $schema->storage->txn_rollback; + }; subtest 'add() tests' => sub { -- 2.32.0 (Apple Git-132)