From 4a054c7d6c42592a8160e11be88d253e84a8a5fd Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Wed, 31 Oct 2018 10:27:41 +0000 Subject: [PATCH] Bug 21754: Automatically clean up outstanding transfers on lost items This is an alternative to bug 21732 as transfers are automatically cancelled on marking an item lost, and the items holding rbanch is set to the transfers source ('from') branch. When an item is marked as lost, the routine should also clean up any outstanding transfers. Also added tests to t/db_dependent/Circulation.t which check: * If transfer is automatically deleted when item is marked as lost * If the items holdingbranch automatically changes when item with transfers on it is marked as lost. Test plan: 1. Find a item which is in transfer, i.e. find an item with the text in the 'Status' field of the table in detail.pl that indicates it is in transfer 2. Set the item to 'Lost' either by clicking on Edit->Edit items from the detail.pl page OR clicking on the Items tab on the left side of the detail.pl page 3. Notice that the transfer is now cancelled for the item and the items holdingbranch is the transfers source ('from') branch 4. Run t/db_dependent/Circulation.t Sponsored-By: Brimbank Library, Australia --- C4/Circulation.pm | 6 +++++ t/db_dependent/Circulation.t | 54 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 23d0b5f..6443aa2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3701,6 +3701,12 @@ sub LostItem{ MarkIssueReturned($borrowernumber,$itemnumber,undef,undef,$patron->privacy) if $mark_returned; } + + #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch) + if (my ( $datesent,$frombranch,$tobranch ) = GetTransfers($itemnumber)) { + ModItem({holdingbranch => $frombranch}, undef, $itemnumber); + } + my $transferdeleted = DeleteTransfer($itemnumber); } sub GetOfflineOperations { diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 2d82c9f..9960d1f 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 119; +use Test::More tests => 120; use Data::Dumper; use DateTime; @@ -2279,6 +2279,7 @@ subtest '_FixAccountForLostAndReturned' => sub { 'The patron balance is the difference between the PF and the credit' ); }; + }; subtest '_FixOverduesOnReturn' => sub { @@ -2406,6 +2407,57 @@ subtest 'Set waiting flag' => sub { is( $status, 'Waiting', 'Now the hold is waiting'); }; +subtest 'Cancel transfers on lost items' => sub { + plan tests => 5; + my $library_1 = $builder->build( { source => 'Branch' } ); + my $patron_1 = $builder->build( { source => 'Borrower', value => { branchcode => $library_1->{branchcode}, categorycode => $patron_category->{categorycode} } } ); + my $library_2 = $builder->build( { source => 'Branch' } ); + my $patron_2 = $builder->build( { source => 'Borrower', value => { branchcode => $library_2->{branchcode}, categorycode => $patron_category->{categorycode} } } ); + my $biblio = $builder->build( { source => 'Biblio' } ); + my $biblioitem = $builder->build( { source => 'Biblioitem', value => { biblionumber => $biblio->{biblionumber} } } ); + my $item = $builder->build( + { + source => 'Item', + value => { + homebranch => $library_1->{branchcode}, + holdingbranch => $library_1->{branchcode}, + notforloan => 0, + itemlost => 0, + withdrawn => 0, + biblionumber => $biblioitem->{biblionumber}, + } + } + ); + + set_userenv( $library_2 ); + my $reserve_id = AddReserve( + $library_2->{branchcode}, $patron_2->{borrowernumber}, $biblioitem->{biblionumber}, '', 1, undef, undef, '', undef, $item->{itemnumber}, + ); + + #Return book and add transfer + set_userenv( $library_1 ); + my $do_transfer = 1; + my ( $res, $rr ) = AddReturn( $item->{barcode}, $library_1->{branchcode} ); + ModReserveAffect( $item->{itemnumber}, undef, $do_transfer, $reserve_id ); + C4::Circulation::transferbook( $library_2->{branchcode}, $item->{barcode} ); + my $hold = Koha::Holds->find( $reserve_id ); + is( $hold->found, 'T', 'Hold is in transit' ); + + #Check transfer exists and the items holding branch is the transfer destination branch before marking it as lost + my ($datesent,$frombranch,$tobranch) = GetTransfers($item->{itemnumber}); + is( $tobranch, $library_2->{branchcode}, 'The transfer record exists in the branchtransfers table'); + my $itemcheck = GetItem($item->{itemnumber}); + is( $itemcheck->{holdingbranch}, $library_2->{branchcode}, 'Items holding branch is the transfers destination branch before it is marked as lost' ); + + #Simulate item being marked as lost and confirm the transfer is deleted and the items holding branch is the transfers source branch + ModItem( { itemlost => 1 }, $biblio->{biblionumber}, $item->{itemnumber} ); + LostItem( $item->{itemnumber}, 'test', 1 ); + ($datesent,$frombranch,$tobranch) = GetTransfers($item->{itemnumber}); + is( $tobranch, undef, 'The transfer on the lost item has been deleted as the LostItemCancelOutstandingTransfer is enabled'); + my $itemcheck = GetItem($item->{itemnumber}); + is( $itemcheck->{holdingbranch}, $library_1->{branchcode}, 'Lost item with cancelled hold has holding branch equallying the transfers source branch' ); +}; + subtest 'CanBookBeIssued | is_overdue' => sub { plan tests => 3; -- 2.1.4