Bug 23695: Set holding branch to transferring branch when manually initiating a transfer

To test:
 1 - Go to Circulation->Transfer
 2 - Note your signed in branch
 3 - Find an item from your branch and create a transfer to branch B
 4 - Confirm the item is marked as held at your current branch and is being transferred to B
 5 - Find an item from a third branch, branch C
 6 - Transfer that item to brnach B
 7 - Confirm the item is held at your current branch and is being transferred to B
 8 - prove -v t/db_dependent/Circulation.t
 9 - prove -v t/db_dependent/Koha/Items.t
10 - prove -v t/db_dependent/RotatingCollections.t

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 8474df4..7f75e89 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -251,12 +251,18 @@ sub decode {
 
 =head2 transferbook
 
-  ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, 
-                                            $barcode, $ignore_reserves, $trigger);
+  ($dotransfer, $messages, $iteminformation) = &transferbook({
+                                                   from_branch => $frombranch
+                                                   to_branch => $tobranch,
+                                                   barcode => $barcode,
+                                                   ignore_reserves => $ignore_reserves,
+                                                   trigger => $trigger
+                                                });
 
 Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer.
 
-C<$newbranch> is the code for the branch to which the item should be transferred.
+C<$fbr> is the code for the branch initiating the transfer.
+C<$tbr> is the code for the branch to which the item should be transferred.
 
 C<$barcode> is the barcode of the item to be transferred.
 
@@ -306,7 +312,12 @@ The item was eligible to be transferred. Barring problems communicating with the
 =cut
 
 sub transferbook {
-    my ( $tbr, $barcode, $ignoreRs, $trigger ) = @_;
+    my $params = shift;
+    my $tbr      = $params->{to_branch};
+    my $fbr      = $params->{from_branch};
+    my $ignoreRs = $params->{ignore_reserves};
+    my $barcode  = $params->{barcode};
+    my $trigger  = $params->{trigger};
     my $messages;
     my $dotransfer      = 1;
     my $item = Koha::Items->find( { barcode => $barcode } );
@@ -321,7 +332,6 @@ sub transferbook {
     my $itemnumber = $item->itemnumber;
     # get branches of book...
     my $hbr = $item->homebranch;
-    my $fbr = $item->holdingbranch;
 
     # if using Branch Transfer Limits
     if ( C4::Context->preference("UseBranchTransferLimits") == 1 ) {

diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm
index a4477fa..c41d578 100644 (file)
--- a/C4/RotatingCollections.pm
+++ b/C4/RotatingCollections.pm
@@ -446,7 +446,13 @@ sub TransferCollection {
     while ( my $item = $sth->fetchrow_hashref ) {
         my ($status) = CheckReserves( $item->{itemnumber} );
         my @transfers = C4::Circulation::GetTransfers( $item->{itemnumber} );
-        C4::Circulation::transferbook( $colBranchcode, $item->{barcode}, 1, 'RotatingCollection' ) unless ( $status eq 'Waiting' || @transfers );
+        C4::Circulation::transferbook({
+            from_branch => $item->holdingbranch,
+            to_branch => $colBranchcode,
+            barcode => $item->{barcode},
+            ignore_reserves => 1,
+            trigger => 'RotatingCollection'
+        }) unless ( $status eq 'Waiting' || @transfers );
     }
 
     return 1;

diff --git a/circ/branchtransfers.pl b/circ/branchtransfers.pl
index 8cd01d5..1e9f48c 100755 (executable)
--- a/circ/branchtransfers.pl
+++ b/circ/branchtransfers.pl
@@ -127,7 +127,14 @@ defined $barcode and $barcode =~ s/^\s*|\s*$//g;  # FIXME: barcodeInputFilter
 if ($barcode) {
 
     ( $transferred, $messages ) =
-      transferbook( $tobranchcd, $barcode, $ignoreRs, 'Manual' );
+
+        transferbook({
+            from_branch => C4::Context->userenv->{'branch'},
+            to_branch => $tobranchcd,
+            barcode => $barcode,
+            ignore_reserves => $ignoreRs,
+            trigger => 'Manual'
+        });
     my $item = Koha::Items->find({ barcode => $barcode });
     $found = $messages->{'ResFound'};
     if ($transferred) {

diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index bb2ff5f..f3f2671 100755 (executable)
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -3244,7 +3244,7 @@ subtest 'Set waiting flag' => sub {
 };
 
 subtest 'Cancel transfers on lost items' => sub {
-    plan tests => 5;
+    plan tests => 6;
     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' } );
@@ -3271,15 +3271,20 @@ subtest 'Cancel transfers on lost items' => sub {
     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 );
+    C4::Circulation::transferbook({
+        from_branch => $library_1->{branchcode},
+        to_branch => $library_2->{branchcode},
+        barcode   => $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');
+    is( $frombranch, $library_1->{branchcode}, 'The transfer is generated from the correct library');
+    is( $tobranch, $library_2->{branchcode}, 'The transfer is generated to the correct library');
     my $itemcheck = Koha::Items->find($item->itemnumber);
-    is( $itemcheck->holdingbranch, $library_1->{branchcode}, 'Items holding branch is the transfers origin branch before it is marked as lost' );
+    is( $itemcheck->holdingbranch, $library_1->{branchcode}, 'Items holding branch is the transfers origination 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
     $item->itemlost(1)->store;

diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
index d04559a..acc57a0 100644 (file)
--- a/t/db_dependent/Koha/Items.t
+++ b/t/db_dependent/Koha/Items.t
@@ -92,7 +92,11 @@ subtest 'get_transfer' => sub {
 
     my $library_to = $builder->build( { source => 'Branch' } );
 
-    C4::Circulation::transferbook( $library_to->{branchcode}, $new_item_1->barcode );
+    C4::Circulation::transferbook({
+        from_branch => $new_item_1->holdingbranch,
+        to_branch => $library_to->{branchcode},
+        barcode => $new_item_1->barcode,
+    });
 
     $transfer = $new_item_1->get_transfer();
     is( ref($transfer), 'Koha::Item::Transfer', 'Koha::Item->get_transfer should return a Koha::Item::Transfers object' );