Bug 42443: (QA follow-up) Prevent cancelling completed transfers
The delete endpoint was passing force => 1 to cancel() with no state guards, allowing already-received and already-cancelled transfers to be re-cancelled, corrupting datearrived/datecancelled audit fields. Add explicit 400 responses for both cases and tests to cover them. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
parent
c813fb2574
commit
f8d59a7b50
|
|
@ -60,6 +60,12 @@ sub delete {
|
|||
return $c->render_resource_not_found("Transfer")
|
||||
unless $transfer;
|
||||
|
||||
return $c->render( status => 400, openapi => { error => "Transfer has already been received" } )
|
||||
if $transfer->datearrived;
|
||||
|
||||
return $c->render( status => 400, openapi => { error => "Transfer has already been cancelled" } )
|
||||
if $transfer->datecancelled;
|
||||
|
||||
return try {
|
||||
my $body = $c->req->json;
|
||||
|
||||
|
|
|
|||
|
|
@ -117,7 +117,7 @@ subtest 'list() tests' => sub {
|
|||
|
||||
subtest 'delete() tests' => sub {
|
||||
|
||||
plan tests => 10;
|
||||
plan tests => 14;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
|
|
@ -165,6 +165,23 @@ subtest 'delete() tests' => sub {
|
|||
$t->delete_ok( "//$userid:$password@/api/v1/transfers/"
|
||||
. ( $transfer->id + 1000 ) => json => { cancellation_reason => 'Manual' } )->status_is(404);
|
||||
|
||||
# Cannot cancel an already-received transfer
|
||||
my $arrived_item = $builder->build_sample_item;
|
||||
my $arrived_transfer = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Item::Transfers',
|
||||
value => {
|
||||
itemnumber => $arrived_item->itemnumber,
|
||||
datesent => \'NOW()',
|
||||
datearrived => \'NOW()',
|
||||
datecancelled => undef,
|
||||
}
|
||||
}
|
||||
);
|
||||
$t->delete_ok(
|
||||
"//$userid:$password@/api/v1/transfers/" . $arrived_transfer->id => json =>
|
||||
{ cancellation_reason => 'Manual' } )->status_is(400);
|
||||
|
||||
# Cancel an in-transit transfer with the supplied reason
|
||||
$t->delete_ok(
|
||||
"//$userid:$password@/api/v1/transfers/" . $transfer->id => json => { cancellation_reason => 'WrongTransfer' } )
|
||||
|
|
@ -174,5 +191,10 @@ subtest 'delete() tests' => sub {
|
|||
ok( defined $transfer->datecancelled, 'The transfer has been cancelled' );
|
||||
is( $transfer->cancellation_reason, 'WrongTransfer', 'The supplied cancellation reason is recorded' );
|
||||
|
||||
# Cannot cancel an already-cancelled transfer
|
||||
$t->delete_ok(
|
||||
"//$userid:$password@/api/v1/transfers/" . $transfer->id => json => { cancellation_reason => 'Manual' } )
|
||||
->status_is(400);
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in New Issue