From b6b06c15a3ba1c275e9185223aaff8ae552eda9b Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 22 Jan 2021 11:11:53 +0000 Subject: [PATCH] Bug 24446: (QA follow-up) Correction to datecancelled for ModItemTransfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModItemTransfer always replaces any existing transfers, including those in transit.. so we needed to add a 'force' option to Koha::Item::Transfer->cancel(); Signed-off-by: Kathleen Milne Signed-off-by: Joonas Kylmälä --- Koha/Item.pm | 2 +- Koha/Item/Transfer.pm | 10 +++++----- t/db_dependent/Circulation/transfers.t | 6 +++--- t/db_dependent/Koha/Item/Transfer.t | 16 +++++++++++----- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index d894fb517d..0794974b1a 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -447,7 +447,7 @@ sub request_transfer { Koha::Exceptions::Item::Transfer::Found->throw( transfer => $request ) if ( $request && !$params->{enqueue} && !$params->{replace} ); - $request->cancel( $params->{reason} ) + $request->cancel( { reason => $params->{reason}, force => 1 } ) if ( defined($request) && $params->{replace} ); my $transfer = Koha::Item::Transfer->new( diff --git a/Koha/Item/Transfer.pm b/Koha/Item/Transfer.pm index 9c8d7abb0b..c9e3206b53 100644 --- a/Koha/Item/Transfer.pm +++ b/Koha/Item/Transfer.pm @@ -118,25 +118,25 @@ sub receive { =head3 cancel - $transfer->cancel($reason); + $transfer->cancel({ reason => $reason, [force => 1]}); Cancel the transfer by setting the datecancelled time and recording the reason. =cut sub cancel { - my ( $self, $reason ) = @_; + my ( $self, $params ) = @_; Koha::Exceptions::MissingParameter->throw( error => "The 'reason' parameter is mandatory" ) - unless defined($reason); + unless defined($params->{reason}); # Throw exception if item is in transit already - Koha::Exceptions::Item::Transfer::Transit->throw() if ( $self->in_transit ); + Koha::Exceptions::Item::Transfer::Transit->throw() if ( !$params->{force} && $self->in_transit ); # Update the cancelled date $self->set( - { datecancelled => dt_from_string, cancellation_reason => $reason } ) + { datecancelled => dt_from_string, cancellation_reason => $params->{reason} } ) ->store; return $self; diff --git a/t/db_dependent/Circulation/transfers.t b/t/db_dependent/Circulation/transfers.t index 7ad80f31d6..c22992d381 100755 --- a/t/db_dependent/Circulation/transfers.t +++ b/t/db_dependent/Circulation/transfers.t @@ -173,7 +173,7 @@ is(CreateBranchTransferLimit(undef,$branchcode_2),undef, my @transfers = GetTransfers($item_id1); cmp_deeply( \@transfers, - [ re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), $branchcode_1, $branchcode_2, re('[0-9]*'), re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), undef ], + [ re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), $branchcode_1, $branchcode_2, re('[0-9]*'), re('^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$'), 'Manual' ], "Transfers of the item1" ); #NOTE: Only the first transfer is returned @transfers = GetTransfers; @@ -255,8 +255,8 @@ ModItemTransfer( $trigger ); $transfer->{_result}->discard_changes; -ok( $transfer->datearrived, 'Date arrived is set when new transfer is initiated' ); -is( $transfer->comments, "Canceled, new transfer from $branchcode_1 to $branchcode_2 created", 'Transfer comment is set as expected when new transfer is initiated' ); +ok( $transfer->datecancelled, 'Date cancelled is set when new transfer is initiated' ); +is( $transfer->cancellation_reason, "Manual", 'Cancellation reason is set correctly when new transfer is initiated' ); $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Item/Transfer.t b/t/db_dependent/Koha/Item/Transfer.t index 1bc6d7fa9f..ac782e28fb 100755 --- a/t/db_dependent/Koha/Item/Transfer.t +++ b/t/db_dependent/Koha/Item/Transfer.t @@ -208,7 +208,7 @@ subtest 'in_transit tests' => sub { }; subtest 'cancel tests' => sub { - plan tests => 5; + plan tests => 7; $schema->storage->txn_begin; @@ -245,15 +245,21 @@ subtest 'cancel tests' => sub { 'Exception thrown if a reason is not passed to cancel'; # Item in transit should result in failure - throws_ok { $transfer->cancel($cancellation_reason) } + throws_ok { $transfer->cancel({ reason => $cancellation_reason }) } 'Koha::Exceptions::Item::Transfer::Transit', 'Exception thrown if item is in transit'; - $transfer->datesent(undef)->store(); + $transfer->cancel({ reason => $cancellation_reason, force => 1}); + ok( $transfer->datecancelled, 'Forced cancellation, cancellation date set' ); + is( $transfer->cancellation_reason, 'Manual', 'Forced cancellation, cancellation reason is set'); + + $transfer->datecancelled(undef); + $transfer->cancellation_reason(undef); + $transfer->datesent(undef); # Transit state unset - $transfer->discard_changes; - $transfer->cancel($cancellation_reason); + $transfer->store()->discard_changes; + $transfer->cancel({ reason => $cancellation_reason }); ok( $transfer->datecancelled, 'Cancellation date set upon call to cancel' ); is( $transfer->cancellation_reason, 'Manual', 'Cancellation reason is set'); -- 2.20.1