gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 28294: Remove updateWrongTransfer 

This patch removes the last remaining use of updateWrongTransfer and the
method itself.

We replace it with a call to request_transfer that passes the 'replace'
reason of 'WrongTransfer' through to Koha::Item::Transfer->cancel.

Test plan
1/ git grep updateWrongTransfer to confirm there are no more uses of the
method.
2/ Confirm an incorrect transfer still behaves as expected at
circulation returns.

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Martin Renvoize 2021-05-17 17:14:16 +01:00 committed by Katrin Fischer
parent 27b780ce62
commit 8b08d4f095
No known key found for this signature in database
GPG Key ID: 0EF6E2C03357A834
8 changed files with 52 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
C4/Circulation.pm
View File

@ -107,7 +107,6 @@ BEGIN {
transferbook
TooMany
GetTransfersFromTo
updateWrongTransfer
CalcDateDue
CheckValidBarcode
IsBranchTransferAllowed
@ -3838,36 +3837,6 @@ sub SendCirculationAlert {
return;
}
=head2 updateWrongTransfer
$items = updateWrongTransfer($itemNumber,$borrowernumber,$waitingAtLibrary,$FromLibrary);
This function validate the line of brachtransfer but with the wrong destination (mistake from a librarian ...), and create a new line in branchtransfer from the actual library to the original library of reservation
=cut
sub updateWrongTransfer {
my ( $itemNumber,$waitingAtLibrary,$FromLibrary ) = @_;
# first step: cancel the original transfer
my $item = Koha::Items->find($itemNumber);
my $transfer = $item->get_transfer;
$transfer->set({ datecancelled => dt_from_string, cancellation_reason => 'WrongTransfer' })->store();
# second step: create a new transfer to the right location
my $new_transfer = $item->request_transfer(
{
to => $transfer->to_library,
reason => $transfer->reason,
comment => $transfer->comments,
ignore_limits => 1,
enqueue => 1
}
);
return $new_transfer;
}
=head2 CalcDateDue
$newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower);

2
C4/Items.pm
View File

@ -401,7 +401,7 @@ sub ModItemTransfer {
to => $to_library,
reason => $trigger,
ignore_limits => 1,
replace => 1
replace => $trigger
}
);
}

2
C4/RotatingCollections.pm
View File

@ -484,7 +484,7 @@ sub TransferCollection {
to => $to_library,
reason => "RotatingCollection",
ignore_limits => 0,
replace => 1
replace => "RotatingCollection"
}
); # Replace transfer
# FIXME: If we just replaced a StockRotationAdvance,

6
Koha/Item.pm
View File

@ -759,7 +759,7 @@ sub check_booking {
{
to => $to_library,
reason => $reason,
[ ignore_limits => 0, enqueue => 1, replace => 1 ]
[ ignore_limits => 0, enqueue => 1, replace => 'reason' ]
}
);
@ -798,8 +798,8 @@ sub request_transfer {
Koha::Exceptions::Item::Transfer::InQueue->throw( transfer => $request )
if ( $request && !$params->{enqueue} && !$params->{replace} );
$request->cancel( { reason => $params->{reason}, force => 1 } )
if ( defined($request) && $params->{replace} );
$request->cancel( { reason => $params->{replace}, force => 1 } )
if ( defined($request) && $params->{replace} );
my $transfer = Koha::Item::Transfer->new(
{

2
Koha/StockRotationItem.pm
View File

@ -265,7 +265,7 @@ sub advance {
to => $new_stage->branchcode,
reason => "StockrotationAdvance",
ignore_limits => 1,
replace => 1
replace => "StockrotationAdvance"
}
); # Replace transfer
}

7
circ/returns.pl
View File

@ -35,7 +35,7 @@ use CGI qw ( -utf8 );
use DateTime;
use C4::Auth qw( get_template_and_user get_session haspermission );
use C4::Circulation qw( barcodedecode GetBranchItemRule AddReturn updateWrongTransfer LostItem );
use C4::Circulation qw( barcodedecode GetBranchItemRule AddReturn LostItem );
use C4::Context;
use C4::Members::Messaging;
use C4::Members;
@ -583,7 +583,10 @@ if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) {
);
# Update the transfer to reflect the new item holdingbranch
my $new_transfer = updateWrongTransfer($messages->{'WrongTransferItem'},$messages->{'WrongTransfer'}, $userenv_branch);
my $item = Koha::Items->find($messages->{'WrongTransferItem'});
my $old_transfer = $item->get_transfer;
my $new_transfer = $item->request_transfer(
{ to => $old_transfer->tobranch, reason => $old_transfer->reason, replace => 'WrongTransfer' } );
$template->param(
NewTransfer => $new_transfer->id
);

44
t/db_dependent/Circulation.t
View File

@ -18,7 +18,7 @@
use Modern::Perl;
use utf8;
use Test::More tests => 76;
use Test::More tests => 75;
use Test::Exception;
use Test::MockModule;
use Test::Deep qw( cmp_deeply );
@ -33,7 +33,7 @@ use t::lib::TestBuilder;
use C4::Accounts;
use C4::Calendar qw( new insert_single_holiday insert_week_day_holiday delete_holiday );
use C4::Circulation qw( AddIssue AddReturn CanBookBeRenewed GetIssuingCharges AddRenewal GetSoonestRenewDate GetLatestAutoRenewDate LostItem GetUpcomingDueIssues CanBookBeIssued AddIssuingCharge MarkIssueReturned ProcessOfflinePayment transferbook updateWrongTransfer );
use C4::Circulation qw( AddIssue AddReturn CanBookBeRenewed GetIssuingCharges AddRenewal GetSoonestRenewDate GetLatestAutoRenewDate LostItem GetUpcomingDueIssues CanBookBeIssued AddIssuingCharge MarkIssueReturned ProcessOfflinePayment transferbook );
use C4::Biblio;
use C4::Items qw( ModItemTransfer );
use C4::Log;
@ -6418,46 +6418,6 @@ subtest "Item's onloan value should be set if checked out item is checked out to
ok( $item->get_from_storage->onloan, "Item's onloan column is set after second checkout" );
};
subtest "updateWrongTransfer tests" => sub {
plan tests => 5;
my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
my $library2 = $builder->build_object( { class => 'Koha::Libraries' } );
my $library3 = $builder->build_object( { class => 'Koha::Libraries' } );
my $item = $builder->build_sample_item(
{
homebranch => $library1->branchcode,
holdingbranch => $library2->branchcode,
datelastseen => undef
}
);
my $transfer = $builder->build_object(
{
class => 'Koha::Item::Transfers',
value => {
itemnumber => $item->itemnumber,
frombranch => $library2->branchcode,
tobranch => $library1->branchcode,
daterequested => dt_from_string,
datesent => dt_from_string,
datecancelled => undef,
datearrived => undef,
reason => 'Manual'
}
}
);
is( ref($transfer), 'Koha::Item::Transfer', 'Mock transfer added' );
my $new_transfer = C4::Circulation::updateWrongTransfer($item->itemnumber, $library1->branchcode);
is(ref($new_transfer), 'Koha::Item::Transfer', "updateWrongTransfer returns a 'Koha::Item::Transfer' object");
ok( !$new_transfer->in_transit, "New transfer is NOT created as in transit (or cancelled)");
my $original_transfer = $transfer->get_from_storage;
ok( defined($original_transfer->datecancelled), "Original transfer was cancelled");
is( $original_transfer->cancellation_reason, 'WrongTransfer', "Original transfer cancellation reason is 'WrongTransfer'");
};
subtest "SendCirculationAlert" => sub {
plan tests => 3;

68
t/db_dependent/Koha/Item.t
View File

@ -852,7 +852,7 @@ subtest 'pickup_locations() tests' => sub {
};
subtest 'request_transfer' => sub {
plan tests => 13;
plan tests => 14;
$schema->storage->txn_begin;
my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
@ -867,15 +867,16 @@ subtest 'request_transfer' => sub {
# Mandatory fields tests
throws_ok { $item->request_transfer( { to => $library1 } ) }
'Koha::Exceptions::MissingParameter',
'Exception thrown if `reason` parameter is missing';
'Exception thrown if `reason` parameter is missing';
throws_ok { $item->request_transfer( { reason => 'Manual' } ) }
'Koha::Exceptions::MissingParameter',
'Exception thrown if `to` parameter is missing';
'Exception thrown if `to` parameter is missing';
# Successful request
my $transfer = $item->request_transfer({ to => $library1, reason => 'Manual' });
is( ref($transfer), 'Koha::Item::Transfer',
my $transfer = $item->request_transfer( { to => $library1, reason => 'Manual' } );
is(
ref($transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer should return a Koha::Item::Transfer object'
);
my $original_transfer = $transfer->get_from_storage;
@ -883,50 +884,59 @@ subtest 'request_transfer' => sub {
# Transfer already in progress
throws_ok { $item->request_transfer( { to => $library2, reason => 'Manual' } ) }
'Koha::Exceptions::Item::Transfer::InQueue',
'Exception thrown if transfer is already in progress';
'Exception thrown if transfer is already in progress';
my $exception = $@;
is( ref( $exception->transfer ),
is(
ref( $exception->transfer ),
'Koha::Item::Transfer',
'The exception contains the found Koha::Item::Transfer' );
'The exception contains the found Koha::Item::Transfer'
);
# Queue transfer
my $queued_transfer = $item->request_transfer(
{ to => $library2, reason => 'Manual', enqueue => 1 } );
is( ref($queued_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when enqueue is set' );
my $queued_transfer = $item->request_transfer( { to => $library2, reason => 'Manual', enqueue => 1 } );
is(
ref($queued_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when enqueue is set'
);
my $transfers = $item->get_transfers;
is($transfers->count, 2, "There are now 2 live transfers in the queue");
is( $transfers->count, 2, "There are now 2 live transfers in the queue" );
$transfer = $transfer->get_from_storage;
is_deeply($transfer->unblessed, $original_transfer->unblessed, "Original transfer unchanged");
is_deeply( $transfer->unblessed, $original_transfer->unblessed, "Original transfer unchanged" );
$queued_transfer->datearrived(dt_from_string)->store();
# Replace transfer
my $replaced_transfer = $item->request_transfer(
{ to => $library2, reason => 'Manual', replace => 1 } );
is( ref($replaced_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when replace is set' );
my $replaced_transfer =
$item->request_transfer( { to => $library2, reason => 'Manual', replace => 'WrongTransfer' } );
is(
ref($replaced_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when replace is set'
);
$original_transfer->discard_changes;
ok($original_transfer->datecancelled, "Original transfer cancelled");
ok( $original_transfer->datecancelled, "Original transfer cancelled" );
is( $original_transfer->cancellation_reason, "WrongTransfer", "Original cancellation_reason set correctly" );
$transfers = $item->get_transfers;
is($transfers->count, 1, "There is only 1 live transfer in the queue");
is( $transfers->count, 1, "There is only 1 live transfer in the queue" );
$replaced_transfer->datearrived(dt_from_string)->store();
# BranchTransferLimits
t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
my $limit = Koha::Item::Transfer::Limit->new({
fromBranch => $library2->branchcode,
toBranch => $library1->branchcode,
itemtype => $item->effective_itemtype,
})->store;
t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', 1 );
t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' );
my $limit = Koha::Item::Transfer::Limit->new(
{
fromBranch => $library2->branchcode,
toBranch => $library1->branchcode,
itemtype => $item->effective_itemtype,
}
)->store;
throws_ok { $item->request_transfer( { to => $library1, reason => 'Manual' } ) }
'Koha::Exceptions::Item::Transfer::Limit',
'Exception thrown if transfer is prevented by limits';
'Exception thrown if transfer is prevented by limits';
my $forced_transfer = $item->request_transfer( { to => $library1, reason => 'Manual', ignore_limits => 1 } );
is( ref($forced_transfer), 'Koha::Item::Transfer',
is(
ref($forced_transfer), 'Koha::Item::Transfer',
'Koha::Item->request_transfer allowed when ignore_limits is set'
);