From 092721c3d167be9ed9039982c8e9fc2d18e32c78 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. Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart Signed-off-by: Fridolin Somers --- Koha/REST/V1/Holds.pm | 4 ++-- t/db_dependent/api/v1/holds.t | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 3df81eb6ad..2162738d02 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 abf23d4fca..64485f01ac 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,8 @@ subtest 'edit() tests' => sub { branchcode => $library_3->branchcode, itemnumber => $item->itemnumber, priority => 1, + suspend => 0, + suspend_until => undef, } } ); @@ -1061,7 +1063,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 should not activate suspended status' ); + is( $item_hold->suspend_until, undef, 'Location change should keep suspended_until be undef' ); + $schema->storage->txn_rollback; + }; subtest 'add() tests' => sub { -- 2.39.5