From 011e58fd629739933e483c4ddd999c32da0a5404 Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Wed, 19 Feb 2020 11:43:08 +0200 Subject: [PATCH] Bug 24680: Fix PUT api/v1/holds/{hold_id} to work also when priority is not provided Before this fix the endpoint would accept the request but fail to actually update the hold if the request does not contain a priority parameter. Signed-off-by: David Nind --- Koha/REST/V1/Holds.pm | 13 +++++-------- t/db_dependent/api/v1/holds.t | 28 +++++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 047a69bd48..dcc46798ff 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -243,19 +243,16 @@ sub edit { my $body = $c->req->json; - my $pickup_library_id = $body->{pickup_library_id}; - my $priority = $body->{priority}; - my $suspended_until = $body->{suspended_until}; - - if ($suspended_until) { - $suspended_until = output_pref(dt_from_string($suspended_until, 'rfc3339')); - } + my $pickup_library_id = $body->{pickup_library_id} // $hold->branchcode; + my $priority = $body->{priority} // $hold->priority; + # suspended_until can also be set to undef + my $suspended_until = exists $body->{suspended_until} ? $body->{suspended_until} : $hold->suspend_until; my $params = { reserve_id => $hold_id, branchcode => $pickup_library_id, rank => $priority, - suspend_until => $suspended_until, + suspend_until => $suspended_until ? output_pref(dt_from_string($suspended_until, 'rfc3339')) : '', itemnumber => $hold->itemnumber }; diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index fda8047d33..bb9870079e 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -47,6 +47,7 @@ my $t = Test::Mojo->new('Koha::REST::V1'); my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; +my $branchcode2 = $builder->build({ source => 'Branch' })->{branchcode}; my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; # Generic password for everyone @@ -198,7 +199,7 @@ subtest "Test endpoints without permission" => sub { subtest "Test endpoints with permission" => sub { - plan tests => 44; + plan tests => 57; $t->get_ok( "//$userid_1:$password@/api/v1/holds" ) ->status_is(200) @@ -211,11 +212,32 @@ subtest "Test endpoints with permission" => sub { ->json_is('/0/patron_id', $patron_2->borrowernumber) ->json_hasnt('/1'); + # While suspended_until is date-time, it's always set to midnight. + my $expected_suspended_until = $suspended_until->strftime('%FT00:00:00%z'); + substr($expected_suspended_until, -2, 0, ':'); + $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => $put_data ) ->status_is(200) ->json_is( '/hold_id', $reserve_id ) - ->json_is( '/suspended_until', output_pref({ dt => $suspended_until, dateformat => 'rfc3339' }) ) - ->json_is( '/priority', 2 ); + ->json_is( '/suspended_until', $expected_suspended_until ) + ->json_is( '/priority', 2 ) + ->json_is( '/pickup_library_id', $branchcode ); + + # Change only pickup library, everything else should remain + $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => { pickup_library_id => $branchcode2 } ) + ->status_is(200) + ->json_is( '/hold_id', $reserve_id ) + ->json_is( '/suspended_until', $expected_suspended_until ) + ->json_is( '/priority', 2 ) + ->json_is( '/pickup_library_id', $branchcode2 ); + + # Reset suspended_until, everything else should remain + $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => { suspended_until => undef } ) + ->status_is(200) + ->json_is( '/hold_id', $reserve_id ) + ->json_is( '/suspended_until', undef ) + ->json_is( '/priority', 2 ) + ->json_is( '/pickup_library_id', $branchcode2 ); $t->delete_ok( "//$userid_3:$password@/api/v1/holds/$reserve_id" ) ->status_is(200); -- 2.11.0