From c4c187b384b776cd4e55a54cc50872784ed44b17 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 30 Oct 2013 10:50:13 +0100 Subject: [PATCH] Bug 9180: All branches should be returned if a default rule exists The C4::Overdues::GetBranchcodesWithOverdueRules routine has a bug. If a default rule *and* a specific rule exist, only the branchcode for the specific rule is returned. Test plan: prove t/db_dependent/Overdues.t and verify the unit tests are consistent. Signed-off-by: Kyle M Hall --- C4/Overdues.pm | 18 ++++++++++------- t/db_dependent/Overdues.t | 51 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 169b36c..91c2583 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -752,14 +752,18 @@ returns a list of branch codes for branches with overdue rules defined. sub GetBranchcodesWithOverdueRules { my $dbh = C4::Context->dbh; - my $rqoverduebranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode <> '' ORDER BY branchcode"); - $rqoverduebranches->execute; - my @branches = map { shift @$_ } @{ $rqoverduebranches->fetchall_arrayref }; - if (!$branches[0]) { - my $availbranches = C4::Branch::GetBranches(); - @branches = keys %$availbranches; + my $branchcodes = $dbh->selectcol_arrayref(q| + SELECT DISTINCT(branchcode) + FROM overduerules + WHERE delay1 IS NOT NULL + ORDER BY branchcode + |); + if ( $branchcodes->[0] eq '' ) { + # If a default rule exists, all branches should be returned + my $availbranches = C4::Branch::GetBranches(); + return keys %$availbranches; } - return @branches; + return @$branchcodes; } =head2 CheckItemNotify diff --git a/t/db_dependent/Overdues.t b/t/db_dependent/Overdues.t index 1a1d5f8..4fc8402 100644 --- a/t/db_dependent/Overdues.t +++ b/t/db_dependent/Overdues.t @@ -1,11 +1,13 @@ #!/usr/bin/perl; use Modern::Perl; -use Test::More;# tests => 3; +use Test::More tests => 16; use C4::Context; +use C4::Branch; use_ok('C4::Overdues'); can_ok('C4::Overdues', 'GetOverdueMessageTransportTypes'); +can_ok('C4::Overdues', 'GetBranchcodesWithOverdueRules'); my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; @@ -73,5 +75,50 @@ is_deeply( $mtts, ['email', 'sms'], 'GetOverdueMessageTransportTypes: second ove $mtts = C4::Overdues::GetOverdueMessageTransportTypes('', 'PT', 3); is_deeply( $mtts, ['print', 'sms', 'email'], 'GetOverdueMessageTransportTypes: third overdue is by print, sms and email for PT (default). With print in first.' ); +# Test GetBranchcodesWithOverdueRules +$dbh->do(q|DELETE FROM overduerules|); +$dbh->do(q| + INSERT INTO overduerules + ( branchcode,categorycode, delay1,letter1,debarred1, delay2,letter2,debarred2, delay3,letter3,debarred3 ) + VALUES + ( '', '', 1, 'LETTER_CODE1', 1, 5, 'LETTER_CODE2', 1, 10, 'LETTER_CODE3', 1 ) +|); + +my $all_branches = C4::Branch::GetBranches; +my @branchcodes = keys %$all_branches; + +my @overdue_branches = C4::Overdues::GetBranchcodesWithOverdueRules(); +is_deeply( [ sort @overdue_branches ], [ sort @branchcodes ], 'If a default rule exists, all branches should be returned' ); + +$dbh->do(q| + INSERT INTO overduerules + ( branchcode,categorycode, delay1,letter1,debarred1, delay2,letter2,debarred2, delay3,letter3,debarred3 ) + VALUES + ( 'CPL', '', 1, 'LETTER_CODE1', 1, 5, 'LETTER_CODE2', 1, 10, 'LETTER_CODE3', 1 ) +|); + +@overdue_branches = C4::Overdues::GetBranchcodesWithOverdueRules(); +is_deeply( [ sort @overdue_branches ], [ sort @branchcodes ], 'If a default rule exists and a specific rule exists, all branches should be returned' ); + +$dbh->do(q|DELETE FROM overduerules|); +$dbh->do(q| + INSERT INTO overduerules + ( branchcode,categorycode, delay1,letter1,debarred1, delay2,letter2,debarred2, delay3,letter3,debarred3 ) + VALUES + ( 'CPL', '', 1, 'LETTER_CODE1', 1, 5, 'LETTER_CODE2', 1, 10, 'LETTER_CODE3', 1 ) +|); + +@overdue_branches = C4::Overdues::GetBranchcodesWithOverdueRules(); +is_deeply( \@overdue_branches, ['CPL'] , 'If only a specific rule exist, only 1 branch should be returned' ); + +$dbh->do(q|DELETE FROM overduerules|); +$dbh->do(q| + INSERT INTO overduerules + ( branchcode,categorycode, delay1,letter1,debarred1, delay2,letter2,debarred2, delay3,letter3,debarred3 ) + VALUES + ( 'CPL', '', 1, 'LETTER_CODE1_CPL', 1, 5, 'LETTER_CODE2_CPL', 1, 10, 'LETTER_CODE3_CPL', 1 ), + ( 'MPL', '', 1, 'LETTER_CODE1_MPL', 1, 5, 'LETTER_CODE2_MPL', 1, 10, 'LETTER_CODE3_MPL', 1 ) +|); -done_testing; +@overdue_branches = C4::Overdues::GetBranchcodesWithOverdueRules(); +is_deeply( \@overdue_branches, ['CPL', 'MPL'] , 'If only 2 specific rules exist, 2 branches should be returned' ); -- 2.0.0.rc2