From 5f74170e68269360de848314a1e5ce0723ac672f Mon Sep 17 00:00:00 2001 From: Yohann Dufour 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 Signed-off-by: Jonathan Druart Amended patch: perltidy on t/db_dependent/Members/* Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- 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 13244e08bf..326344dc89 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -1181,6 +1181,9 @@ C 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 340a6c9951..36f7ccc9af 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 0000000000..c2791bc1d5 --- /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 0000000000..08051b19f1 --- /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(); -- 2.39.5