From 5c997b8af5cc5758147af914883ca8acf86543d2 Mon Sep 17 00:00:00 2001
From: Yohann Dufour <dufour.yohann@gmail.com>
Date: Mon, 23 Jun 2014 16:00:58 +0200
Subject: [PATCH] [PASSED QA] Bug 12457: Adding unit tests for Members.pm

Adding unit tests for the routines AddMessage, GetMessages, GetMessagesCount and DeleteMessage in t/db_dependent/Members.t
Adding unit tests for the routines GetPendingIssues and GetAllIssues in separate files : t/db_dependent/Members/GetPendingIssues.t and t/db_dependent/Members/GetAllIssues.t
The routine GetAllIssues has been modified because it does not test if the arguments was defined :
- the borrowernumber argument is required
- if the order argument is not given, it takes a value by default : 'date_due desc'
- the limit argument is optional

Test plan:
1/ Apply the patch
2/ Execute : prove t/db_dependent/Members.t t/db_dependent/Members/GetAllIssues.t t/db_dependent/Members/GetPendingIssues.t
3/ The result has to be a success without error or warning :
t/db_dependent/Members.t ................... ok
t/db_dependent/Members/GetAllIssues.t ...... ok
t/db_dependent/Members/GetPendingIssues.t .. ok
All tests successful.
Files=3, Tests=83,  5 wallclock secs ( 0.06 usr  0.01 sys +  4.68 cusr  0.26 csys =  5.01 CPU)
Result: PASS

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Amended patch: perltidy on t/db_dependent/Members/*

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
---
 C4/Members.pm                             |   3 +
 t/db_dependent/Members.t                  |  48 ++++++++++++-
 t/db_dependent/Members/GetAllIssues.t     |  95 ++++++++++++++++++++++++++
 t/db_dependent/Members/GetPendingIssues.t | 110 ++++++++++++++++++++++++++++++
 4 files changed, 254 insertions(+), 2 deletions(-)
 create mode 100644 t/db_dependent/Members/GetAllIssues.t
 create mode 100644 t/db_dependent/Members/GetPendingIssues.t

diff --git a/C4/Members.pm b/C4/Members.pm
index 13244e0..326344d 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -1181,6 +1181,9 @@ C<items> tables of the Koha database.
 sub GetAllIssues {
     my ( $borrowernumber, $order, $limit ) = @_;
 
+    return unless $borrowernumber;
+    $order = 'date_due desc' unless $order;
+
     my $dbh = C4::Context->dbh;
     my $query =
 'SELECT *, issues.timestamp as issuestimestamp, issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
index 340a6c9..36f7ccc 100755
--- a/t/db_dependent/Members.t
+++ b/t/db_dependent/Members.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 34;
+use Test::More tests => 55;
 use Data::Dumper;
 use C4::Context;
 
@@ -204,6 +204,50 @@ ModMember(borrowernumber => $member->{'borrowernumber'}, dateexpiry => '2001-01-
 $member = GetMemberDetails($member->{'borrowernumber'});
 ok($member->{is_expired}, "GetMemberDetails() indicates that patron is expired");
 
+
+my $message_type = 'B';
+my $message = 'my message';
+my $messages_count = GetMessagesCount($member->{borrowernumber}, $message_type, $BRANCHCODE);
+is( $messages_count, 0, 'GetMessagesCount returns the number of messages correclty' );
+
+is( AddMessage(), undef, 'AddMessage without argument returns undef' );
+is( AddMessage(undef, $message_type, $message, $BRANCHCODE), undef,  'AddMessage without the borrower number returns undef' );
+is( AddMessage($member->{borrowernumber}, undef, $message, $BRANCHCODE), undef,  'AddMessage without the message type returns undef' );
+is( AddMessage($member->{borrowernumber}, $message_type, undef, $BRANCHCODE), undef,  'AddMessage without the message returns undef' );
+is( AddMessage($member->{borrowernumber}, $message_type, $message, undef), undef,  'AddMessage without the branch code returns undef' );
+is( AddMessage($member->{borrowernumber}, $message_type, $message, $BRANCHCODE), 1,  'AddMessage functions correctly' );
+
+$messages_count = GetMessagesCount();
+is( $messages_count, 0, 'GetMessagesCount without argument returns 0' );
+$messages_count = GetMessagesCount(undef, $message_type, $BRANCHCODE);
+is( $messages_count, '0', 'GetMessagesCount without the borrower number returns the number of messages' );
+$messages_count = GetMessagesCount($member->{borrowernumber}, undef, $BRANCHCODE);
+is( $messages_count, '1', 'GetMessagesCount without the message type returns the total number of messages' );
+$messages_count = GetMessagesCount($member->{borrowernumber}, $message_type, undef);
+is( $messages_count, '1', 'GetMessagesCount without the branchcode returns the total number of messages' );
+$messages_count = GetMessagesCount($member->{borrowernumber}, $message_type, $BRANCHCODE);
+is( $messages_count, '1', 'GetMessagesCount returns the number of messages correctly' );
+
+my $messages = GetMessages();
+is( @$messages, 0, 'GetMessages without argument returns 0' );
+$messages = GetMessages($member->{borrowernumber}, $message_type, $BRANCHCODE);
+is( @$messages, 1, 'GetMessages returns the correct number of messages' );
+is( $messages->[0]->{borrowernumber}, $member->{borrowernumber}, 'GetMessages returns the borrower number correctly' );
+is( $messages->[0]->{message_type}, $message_type, 'GetMessages returns the message type correclty' );
+is( $messages->[0]->{message}, $message, 'GetMessages returns the message correctly' );
+is( $messages->[0]->{branchcode}, $BRANCHCODE, 'GetMessages returns the branch code correctly' );
+
+$messages_count = GetMessagesCount($member->{borrowernumber}, $message_type, $BRANCHCODE);
+is( $messages_count, 1, 'GetMessagesCount returns the number of messages correclty' );
+
+DeleteMessage();
+$messages = GetMessages($member->{borrowernumber}, $message_type, $BRANCHCODE);
+is( @$messages, 1, 'DeleteMessage without message id does not delete messages' );
+DeleteMessage($messages->[0]->{message_id});
+$messages = GetMessages($member->{borrowernumber}, $message_type, $BRANCHCODE);
+is( @$messages, 0, 'DeleteMessage deletes a message correctly' );
+
+
 # clean up 
 DelMember($member->{borrowernumber});
 $results = Search($CARDNUMBER,undef,undef,undef,["cardnumber"]);
@@ -252,4 +296,4 @@ sub _find_member {
     return $found;
 }
 
-1;
\ No newline at end of file
+1;
diff --git a/t/db_dependent/Members/GetAllIssues.t b/t/db_dependent/Members/GetAllIssues.t
new file mode 100644
index 0000000..c2791bc
--- /dev/null
+++ b/t/db_dependent/Members/GetAllIssues.t
@@ -0,0 +1,95 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+
+use Test::More tests => 16;
+use Test::MockModule;
+
+use C4::Biblio;
+use C4::Items;
+use C4::Members;
+use C4::Branch;
+use C4::Category;
+use C4::Circulation;
+use MARC::Record;
+
+my $dbh = C4::Context->dbh;
+$dbh->{AutoCommit} = 0;
+$dbh->{RaiseError} = 1;
+
+$dbh->do(q|DELETE FROM borrowers|);
+$dbh->do(q|DELETE FROM branches|);
+$dbh->do(q|DELETE FROM biblio|);
+$dbh->do(q|DELETE FROM items|);
+$dbh->do(q|DELETE FROM categories|);
+
+my $branchcode = 'B';
+ModBranch( { add => 1, branchcode => $branchcode, branchname => 'Branch' } );
+
+my $categorycode = 'C';
+$dbh->do( "INSERT INTO categories(categorycode) VALUES(?)",
+    undef, $categorycode );
+
+my %item_branch_infos = (
+    homebranch    => $branchcode,
+    holdingbranch => $branchcode,
+);
+
+my ($biblionumber1) = AddBiblio( MARC::Record->new, '' );
+my $itemnumber1 =
+  AddItem( { barcode => '0101', %item_branch_infos }, $biblionumber1 );
+my $itemnumber2 =
+  AddItem( { barcode => '0102', %item_branch_infos }, $biblionumber1 );
+
+my ($biblionumber2) = AddBiblio( MARC::Record->new, '' );
+my $itemnumber3 =
+  AddItem( { barcode => '0203', %item_branch_infos }, $biblionumber2 );
+
+my $borrowernumber1 =
+  AddMember( categorycode => $categorycode, branchcode => $branchcode );
+my $borrowernumber2 =
+  AddMember( categorycode => $categorycode, branchcode => $branchcode );
+my $borrower1 = GetMember( borrowernumber => $borrowernumber1 );
+my $borrower2 = GetMember( borrowernumber => $borrowernumber2 );
+
+my $module = new Test::MockModule('C4::Context');
+$module->mock( 'userenv', sub { { branch => $branchcode } } );
+
+my $issues = C4::Members::GetAllIssues();
+is( $issues, undef, 'GetAllIssues without borrower number returns undef' );
+
+$issues = C4::Members::GetAllIssues($borrowernumber1);
+is( @$issues, 0, 'GetAllIssues returns the correct number of elements' );
+$issues = C4::Members::GetAllIssues($borrowernumber2);
+is( @$issues, 0, 'GetAllIssues returns the correct number of elements' );
+
+AddIssue( $borrower1, '0101' );
+$issues = C4::Members::GetAllIssues($borrowernumber1);
+my $issues_with_order =
+  C4::Members::GetAllIssues( $borrowernumber1, 'date_due desc' );
+is_deeply( $issues, $issues_with_order,
+'The value by default for the argument order in GellAllIssues is date_due_desc'
+);
+is( @$issues, 1, 'GetAllIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber}, $itemnumber1, '' );
+$issues = C4::Members::GetAllIssues($borrowernumber2);
+is( @$issues, 0, 'GetAllIssues returns the correct number of elements' );
+
+AddIssue( $borrower1, '0102' );
+$issues = C4::Members::GetAllIssues($borrowernumber1);
+is( @$issues, 2, 'GetAllIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber}, $itemnumber1, '' );
+is( $issues->[1]->{itemnumber}, $itemnumber2, '' );
+$issues = C4::Members::GetAllIssues($borrowernumber2);
+is( @$issues, 0, 'GetAllIssues returns the correct number of elements' );
+
+AddIssue( $borrower2, '0203' );
+$issues = C4::Members::GetAllIssues($borrowernumber1);
+is( @$issues, 2, 'GetAllIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber}, $itemnumber1, '' );
+is( $issues->[1]->{itemnumber}, $itemnumber2, '' );
+$issues = C4::Members::GetAllIssues($borrowernumber2);
+is( @$issues, 1, 'GetAllIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber}, $itemnumber3, '' );
+
+$dbh->rollback();
diff --git a/t/db_dependent/Members/GetPendingIssues.t b/t/db_dependent/Members/GetPendingIssues.t
new file mode 100644
index 0000000..08051b1
--- /dev/null
+++ b/t/db_dependent/Members/GetPendingIssues.t
@@ -0,0 +1,110 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+
+use Test::More tests => 20;
+use Test::MockModule;
+
+use C4::Biblio;
+use C4::Items;
+use C4::Members;
+use C4::Branch;
+use C4::Category;
+use C4::Circulation;
+use MARC::Record;
+
+my $dbh = C4::Context->dbh;
+$dbh->{AutoCommit} = 0;
+$dbh->{RaiseError} = 1;
+
+$dbh->do(q|DELETE FROM borrowers|);
+$dbh->do(q|DELETE FROM branches|);
+$dbh->do(q|DELETE FROM biblio|);
+$dbh->do(q|DELETE FROM items|);
+$dbh->do(q|DELETE FROM categories|);
+
+my $branchcode = 'B';
+ModBranch( { add => 1, branchcode => $branchcode, branchname => 'Branch' } );
+
+my $categorycode = 'C';
+$dbh->do( "INSERT INTO categories(categorycode) VALUES(?)",
+    undef, $categorycode );
+
+my %item_branch_infos = (
+    homebranch    => $branchcode,
+    holdingbranch => $branchcode,
+);
+
+my ($biblionumber1) = AddBiblio( MARC::Record->new, '' );
+my $itemnumber1 =
+  AddItem( { barcode => '0101', %item_branch_infos }, $biblionumber1 );
+my $itemnumber2 =
+  AddItem( { barcode => '0102', %item_branch_infos }, $biblionumber1 );
+
+my ($biblionumber2) = AddBiblio( MARC::Record->new, '' );
+my $itemnumber3 =
+  AddItem( { barcode => '0203', %item_branch_infos }, $biblionumber2 );
+
+my $borrowernumber1 =
+  AddMember( categorycode => $categorycode, branchcode => $branchcode );
+my $borrowernumber2 =
+  AddMember( categorycode => $categorycode, branchcode => $branchcode );
+my $borrower1 = GetMember( borrowernumber => $borrowernumber1 );
+my $borrower2 = GetMember( borrowernumber => $borrowernumber2 );
+
+my $module = new Test::MockModule('C4::Context');
+$module->mock( 'userenv', sub { { branch => $branchcode } } );
+
+my $issues =
+  C4::Members::GetPendingIssues( $borrowernumber1, $borrowernumber2 );
+is( @$issues, 0, 'GetPendingIssues returns the correct number of elements' );
+
+AddIssue( $borrower1, '0101' );
+$issues = C4::Members::GetPendingIssues($borrowernumber1);
+is( @$issues, 1, 'GetPendingIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber},
+    $itemnumber1, 'GetPendingIssues returns the itemnumber correctly' );
+my $issues_bis =
+  C4::Members::GetPendingIssues( $borrowernumber1, $borrowernumber2 );
+is_deeply( $issues, $issues_bis, 'GetPendingIssues functions correctly' );
+$issues = C4::Members::GetPendingIssues($borrowernumber2);
+is( @$issues, 0, 'GetPendingIssues returns the correct number of elements' );
+
+AddIssue( $borrower1, '0102' );
+$issues = C4::Members::GetPendingIssues($borrowernumber1);
+is( @$issues, 2, 'GetPendingIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber},
+    $itemnumber1, 'GetPendingIssues returns the itemnumber correctly' );
+is( $issues->[1]->{itemnumber},
+    $itemnumber2, 'GetPendingIssues returns the itemnumber correctly' );
+$issues_bis =
+  C4::Members::GetPendingIssues( $borrowernumber1, $borrowernumber2 );
+is_deeply( $issues, $issues_bis, 'GetPendingIssues functions correctly' );
+$issues = C4::Members::GetPendingIssues($borrowernumber2);
+is( @$issues, 0, 'GetPendingIssues returns the correct number of elements' );
+
+AddIssue( $borrower2, '0203' );
+$issues = C4::Members::GetPendingIssues($borrowernumber2);
+is( @$issues, 1, 'GetAllIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber},
+    $itemnumber3, 'GetPendingIssues returns the itemnumber correctly' );
+$issues = C4::Members::GetPendingIssues($borrowernumber1);
+is( @$issues, 2, 'GetPendingIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber},
+    $itemnumber1, 'GetPendingIssues returns the itemnumber correctly' );
+is( $issues->[1]->{itemnumber},
+    $itemnumber2, 'GetPendingIssues returns the itemnumber correctly' );
+$issues = C4::Members::GetPendingIssues( $borrowernumber1, $borrowernumber2 );
+is( @$issues, 3, 'GetPendingIssues returns the correct number of elements' );
+is( $issues->[0]->{itemnumber},
+    $itemnumber1, 'GetPendingIssues returns the itemnumber correctly' );
+is( $issues->[1]->{itemnumber},
+    $itemnumber2, 'GetPendingIssues returns the itemnumber correctly' );
+is( $issues->[2]->{itemnumber},
+    $itemnumber3, 'GetPendingIssues returns the itemnumber correctly' );
+
+$issues = C4::Members::GetPendingIssues();
+is( @$issues, 0,
+    'GetPendingIssues without borrower numbers returns an empty array' );
+
+$dbh->rollback();
-- 
1.9.1