Attachment 29548 Details for Bug 9180 – Bug 9180: All branches should be returned if a default rule exists

[patch] Bug 9180: All branches should be returned if a default rule exists

Bug-9180-All-branches-should-be-returned-if-a-defa.patch (text/plain), 4.91 KB, created by Jonathan Druart on 2014-07-07 14:01:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2014-07-07 14:01:29 UTC

Size: 4.91 KB

patch

obsolete

>From c4c187b384b776cd4e55a54cc50872784ed44b17 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>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 <kyle@bywatersolutions.com>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 9180: 13799 | 22574 | 25947 | 27427 | 29548 | 30392