From c73c4c5c5e0dce69218ecafa3475b3336d1c7ed7 Mon Sep 17 00:00:00 2001 From: Kenza Zaki 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 Signed-off-by: Chris Cormack --- 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 310cefa..5513890 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3232,13 +3232,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.8.3.1