From a2385271f56e88b43f31e431dd70783550a621c1 Mon Sep 17 00:00:00 2001
From: Kenza Zaki <kenza.zaki@biblibre.com>
Date: Wed, 7 Aug 2013 16:35:35 +0200
Subject: [PATCH] Bug 10693: CreateBranchTransferLimit's return value in
 C4::Circulation.pm should be more explicit

This patch test if the parameters $toBranch and $fromBranch are given. If not, CreateBranchTransferLimit now returns undef.
This patch also fixes and adds some regression tests in t/db_dependent/Circulation_transfers.t

NOTE:
Currently, we can add a transferlimit to nonexistent branches because in the database branch_transfer_limits.toBranch
and branch_transfer_limits.fromBranch aren't foreign keys.

To test:
prove t/db_dependent/Circulation_transfers.t
t/db_dependent/Circulation_transfers.t .. ok
All tests successful.
Files=1, Tests=15, 18 wallclock secs ( 0.02 usr  0.01 sys +  0.42 cusr  0.00 csys =  0.45 CPU)
Result: PASS
---
 C4/Circulation.pm                      |    4 ++--
 t/db_dependent/Circulation_transfers.t |   13 ++++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index ca66931..7831b80 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3248,13 +3248,13 @@ $code is either itemtype or collection code depending on what the pref BranchTra
 
 sub CreateBranchTransferLimit {
    my ( $toBranch, $fromBranch, $code ) = @_;
-
+   return unless ($toBranch && $fromBranch);
    my $limitType = C4::Context->preference("BranchTransferLimitsType");
    
    my $dbh = C4::Context->dbh;
    
    my $sth = $dbh->prepare("INSERT INTO branch_transfer_limits ( $limitType, toBranch, fromBranch ) VALUES ( ?, ?, ? )");
-   $sth->execute( $code, $toBranch, $fromBranch );
+   return $sth->execute( $code, $toBranch, $fromBranch );
 }
 
 =head2 DeleteBranchTransferLimits
diff --git a/t/db_dependent/Circulation_transfers.t b/t/db_dependent/Circulation_transfers.t
index 9a356c7..43e8e4b 100644
--- a/t/db_dependent/Circulation_transfers.t
+++ b/t/db_dependent/Circulation_transfers.t
@@ -9,7 +9,7 @@ use C4::Circulation;
 use Koha::DateUtils;
 use DateTime::Duration;
 
-use Test::More tests => 12;
+use Test::More tests => 15;
 
 BEGIN {
     use_ok('C4::Circulation');
@@ -129,8 +129,15 @@ is(
     1,
     "A Branch TransferLimit has been added"
 );
-#FIXME :The following test should pass but doesn't because currently the routine CreateBranchTransferLimit returns nothing
-#is(CreateBranchTransferLimit(),undef,"Without parameters CreateBranchTransferLimit returns undef");
+is(CreateBranchTransferLimit(),undef,
+    "Without parameters CreateBranchTransferLimit returns undef");
+is(CreateBranchTransferLimit($samplebranch2->{branchcode}),undef,
+    "With only tobranch CreateBranchTransferLimit returns undef");
+is(CreateBranchTransferLimit(undef,$samplebranch2->{branchcode}),undef,
+    "With only frombranch CreateBranchTransferLimit returns undef");
+#FIXME: Currently, we can add a transferlimit even to nonexistent branches because in the database,
+#branch_transfer_limits.toBranch and branch_transfer_limits.fromBranch aren't foreign keys
+#is(CreateBranchTransferLimit(-1,-1,'CODE'),0,"With wrong CreateBranchTransferLimit returns 0 - No transfertlimit added");
 
 #Test GetTransfers
 my $dt_today = dt_from_string( undef, 'sql', undef );
-- 
1.7.9.5