Bug 10693: CreateBranchTransferLimit's return value in C4::Circulation.pm should...

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

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 2c568b4..d120dfd 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3251,13 +3251,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 927b87c..5724f4f 100644 (file)
--- 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 );