From 0453b3e87a54bbba28aa5de66386f26c91ad4c3b 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] 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
---
C4/Members.pm | 3 +
t/db_dependent/Members.t | 48 +++++++++++++++-
t/db_dependent/Members/GetAllIssues.t | 89 +++++++++++++++++++++++++++++
t/db_dependent/Members/GetPendingIssues.t | 94 +++++++++++++++++++++++++++++++
4 files changed, 232 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..d53498d
--- /dev/null
+++ b/t/db_dependent/Members/GetAllIssues.t
@@ -0,0 +1,89 @@
+#!/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..aeffe48
--- /dev/null
+++ b/t/db_dependent/Members/GetPendingIssues.t
@@ -0,0 +1,94 @@
+#!/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