From 66f1605c59763613a4e955fd259a845e4fc06324 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 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Andreas Hedström Mace (fixed the introduction of a whitespace line and removed a double declare warning from the new tests as part of QA) Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- C4/Circulation.pm | 6 ++++ t/db_dependent/Circulation.t | 53 +++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index dcb18b3401..017e9d7df0 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3702,6 +3702,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 daafe4aaa3..e718224dc9 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 122; +use Test::More tests => 123; use Data::Dumper; use DateTime; @@ -2430,6 +2430,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'); + $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.39.5