From 1b82ed58d120ae3359b197590305d792a2bc6994 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 8 May 2023 12:54:48 +0000 Subject: [PATCH] Bug 33608: Move older Stats tests to Koha/Statistics.t Test plan: Run t/db_dependent/Koha/Statistics.t Signed-off-by: Marcel de Rooy Signed-off-by: Owen Leonard Signed-off-by: Martin Renvoize --- t/db_dependent/Koha/Statistics.t | 144 +++++++++++++++++++++----- t/db_dependent/Stats.t | 168 ------------------------------- 2 files changed, 119 insertions(+), 193 deletions(-) delete mode 100755 t/db_dependent/Stats.t diff --git a/t/db_dependent/Koha/Statistics.t b/t/db_dependent/Koha/Statistics.t index d3f8a76c80..999d1b8567 100755 --- a/t/db_dependent/Koha/Statistics.t +++ b/t/db_dependent/Koha/Statistics.t @@ -1,6 +1,6 @@ #!/usr/bin/perl -# Copyright 2019 Koha Development team +# Copyright 2013, 2019, 2023 Koha Development team # # This file is part of Koha # @@ -19,24 +19,40 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 3; +use Test::Exception; +use C4::Context; use Koha::Database; use Koha::Statistics; -use C4::Context; -use C4::Stats qw( UpdateStats ); use t::lib::TestBuilder; -my $schema = Koha::Database->new->schema; -$schema->storage->txn_begin; +our $schema = Koha::Database->new->schema; +our $builder = t::lib::TestBuilder->new; + +our $test_params = { # No FK checks here + branch => "BRA", + itemnumber => 31, + borrowernumber => 5, + categorycode => 'S', + amount => 5.1, + other => "bla", + itemtype => "BK", + location => "LOC", + ccode => "CODE", + interface => 'INTERFACE', +}; + +subtest 'Basic Koha object tests' => sub { + plan tests => 4; + $schema->storage->txn_begin; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $item = $builder->build_sample_item; + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); -my $builder = t::lib::TestBuilder->new; -my $library = $builder->build_object( { class => 'Koha::Libraries' } ); -my $item = $builder->build_sample_item; -my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); -C4::Stats::UpdateStats( - { + Koha::Statistic->insert({ type => 'issue', branch => $library->branchcode, itemnumber => $item->itemnumber, @@ -44,16 +60,94 @@ C4::Stats::UpdateStats( itemtype => $item->effective_itemtype, location => $item->location, ccode => $item->ccode, - interface => C4::Context->interface - } -); - -my $stat = - Koha::Statistics->search( { itemnumber => $item->itemnumber } )->next; -is( $stat->borrowernumber, $patron->borrowernumber, 'Patron is there' ); -is( $stat->branch, $library->branchcode, 'Library is there' ); -is( ref( $stat->item ), 'Koha::Item', '->item returns a Koha::Item object' ); -is( $stat->item->itemnumber, $item->itemnumber, '->item works great' ); -is( $stat->interface, 'opac', 'Interface is recorded successfully' ); - -$schema->storage->txn_rollback; + interface => C4::Context->interface, + }); + + my $stat = Koha::Statistics->search( { itemnumber => $item->itemnumber } )->next; + is( $stat->borrowernumber, $patron->borrowernumber, 'Patron is there' ); + is( $stat->branch, $library->branchcode, 'Library is there' ); + is( ref( $stat->item ), 'Koha::Item', '->item returns a Koha::Item object' ); + is( $stat->item->itemnumber, $item->itemnumber, '->item works great' ); + + $schema->storage->txn_rollback; +}; + +subtest 'Test exceptions in ->new' => sub { + plan tests => 5; + $schema->storage->txn_begin; + + throws_ok { Koha::Statistic->new } 'Koha::Exceptions::BadParameter', '->new called without params'; + #FIXME Should we remove this for sake of consistency? + + # Type is missing + my $params = { %$test_params }; + throws_ok { Koha::Statistic->new($params) } 'Koha::Exceptions::WrongParameter', '->new called without type'; + + # Type is not allowed + $params = { %$test_params }; + $params ->{type} = "bla"; + throws_ok { Koha::Statistic->new($params) } 'Koha::Exceptions::WrongParameter', '->new called with wrong type'; + + # Test mandatory accounts/circulation keys + $params = { %$test_params }; + $params->{type} = 'payment'; + delete $params->{amount}; + throws_ok { Koha::Statistic->new($params) } 'Koha::Exceptions::MissingParameter', '->new called for accounts without amount'; + $params->{type} = 'issue'; + delete $params->{itemnumber}; + throws_ok { Koha::Statistic->new($params) } 'Koha::Exceptions::MissingParameter', '->new called for circulation without itemnumber'; + + $schema->storage->txn_rollback; +}; + +subtest 'Test ->insert (fka UpdateStats)' => sub { + plan tests => 15; + $schema->storage->txn_begin; + + # save the params in the right database fields + my $statistic = insert_and_fetch({ %$test_params, type => 'return' }); + is( $statistic->branch, $test_params->{branch}, "Check branch" ); + is( $statistic->type, 'return', "Check type" ); + is( $statistic->borrowernumber, $test_params->{borrowernumber}, "Check borrowernumber" ); + is( $statistic->value, $test_params->{amount}, "Check value" ); + is( $statistic->other, $test_params->{other}, "Check other" ); + is( $statistic->itemtype, $test_params->{itemtype}, "Check itemtype" ); + is( $statistic->location, $test_params->{location}, "Check location" ); + is( $statistic->ccode, $test_params->{ccode}, "Check ccode" ); + is( $statistic->interface, $test_params->{interface}, "Check interface" ); + + # Test location with undef and empty string + my $params = { %$test_params, type => 'return' }; + delete $params->{location}; + $statistic = insert_and_fetch($params); + is( $statistic->location, undef, "Location is NULL if not passed" ); + $params->{location} = q{}; + $statistic = insert_and_fetch($params); + is( $statistic->location, q{}, "Location is empty string if passed" ); + + # Test 'other' with undef and empty string (slightly different behavior from location, using _key_or_default) + $params = { %$test_params, type => 'return' }; + delete $params->{other}; + $statistic = insert_and_fetch($params); + is( $statistic->other, q{}, "Other is empty string if not passed" ); + $params->{other} = undef; + $statistic = insert_and_fetch($params); + is( $statistic->other, undef, "Other is NULL if passed undef" ); + + # Test amount versus value; value is the db column, amount is the legacy name (to be deprecated?) + $params = { %$test_params, type => 'return', value => 0 }; + $statistic = insert_and_fetch($params); + is( $statistic->value, 0, "Value is zero, overriding non-zero amount" ); + delete $params->{value}; + $statistic = insert_and_fetch($params); + is( $statistic->value, 5.1, "No value passed, amount used" ); + + $schema->storage->txn_rollback; +}; + +sub insert_and_fetch { + my $params = shift; + my $statistic = Koha::Statistic->insert($params); + return Koha::Statistics->search({ borrowernumber => $test_params->{borrowernumber} })->last; + # FIXME discard_changes would be nicer, but we dont have a PK (yet) +} diff --git a/t/db_dependent/Stats.t b/t/db_dependent/Stats.t deleted file mode 100755 index bd78e20b49..0000000000 --- a/t/db_dependent/Stats.t +++ /dev/null @@ -1,168 +0,0 @@ -#!/usr/bin/perl - -# Copyright 2013, 2023 Koha Development team -# -# This file is part of Koha -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -use Modern::Perl; -use Test::More tests => 1; -use Test::Exception; - -use C4::Context; -use C4::Stats qw( UpdateStats ); -use Koha::Database; - -my $schema = Koha::Database->new->schema; -$schema->storage->txn_begin; -my $dbh = C4::Context->dbh; - -subtest 'UpdateStats' => sub { - plan tests => 16; - - throws_ok { UpdateStats() } 'Koha::Exceptions::BadParameter', 'UpdateStats called without params'; - - my $params = { - branch => "BRA", - itemnumber => 31, - borrowernumber => 5, - amount =>5.1, - other => "bla", - itemtype => "BK", - location => "LOC", - ccode => "CODE", - interface => "INTERFACE", - }; - my $return_error; - - # returns undef and croaks if type is not allowed - $params -> {type} = "bla"; - eval {UpdateStats($params)}; - $return_error = $@; - isnt ($return_error,'',"UpdateStats returns undef and croaks if type is not allowed"); - - delete $params->{type}; - # returns undef and croaks if type is missing - eval {UpdateStats($params)}; - $return_error = $@; - isnt ($return_error,'',"UpdateStats returns undef and croaks if no type given"); - - $params -> {type} = undef; - # returns undef and croaks if type is undef - eval {UpdateStats($params)}; - $return_error = $@; - isnt ($return_error,'',"UpdateStats returns undef and croaks if type is undef"); - - # returns undef and croaks if mandatory params are missing - my @allowed_circulation_types = qw (renew issue localuse return onsite_checkout recall); - my @allowed_accounts_types = qw (writeoff payment); - my @circulation_mandatory_keys = qw (branch borrowernumber itemnumber ccode itemtype); #don't check type here - my @accounts_mandatory_keys = qw (branch borrowernumber amount); #don't check type here - - my @missing_errors = (); - foreach my $key (@circulation_mandatory_keys) { - my $value = $params->{$key}; - delete $params->{$key}; - foreach my $type (@allowed_circulation_types) { - $params->{type} = $type; - eval {UpdateStats($params)}; - $return_error = $@; - push @missing_errors, "key:$key for type:$type" unless $return_error; - } - $params->{$key} = $value; - } - foreach my $key (@accounts_mandatory_keys) { - my $value = $params->{$key}; - delete $params->{$key}; - foreach my $type (@allowed_accounts_types) { - $params->{type} = $type; - eval {UpdateStats($params)}; - $return_error = $@; - push @missing_errors, "key:$key for type:$type" unless $return_error; - } - $params->{$key} = $value; - - } - is (join (", ", @missing_errors),'',"UpdateStats returns undef and croaks if mandatory params are missing"); - - # save the params in the right database fields - $dbh->do(q|DELETE FROM statistics|); - $params = { - branch => "BRA", - itemnumber => 31, - borrowernumber => 5, - amount =>5.1, - other => "bla", - itemtype => "BK", - location => "LOC", - ccode => "CODE", - type => "return", - interface => "INTERFACE", - }; - UpdateStats ($params); - my $sth = $dbh->prepare("SELECT * FROM statistics"); - $sth->execute(); - my $line = ${ $sth->fetchall_arrayref( {} ) }[0]; - is ($params->{branch}, $line->{branch}, "UpdateStats save branch param in branch field of statistics table"); - is ($params->{type}, $line->{type}, "UpdateStats save type param in type field of statistics table"); - is ($params->{borrowernumber}, $line->{borrowernumber}, "UpdateStats save borrowernumber param in borrowernumber field of statistics table"); - is ($params->{value}, $line->{value}, "UpdateStats save amount param in value field of statistics table"); - is ($params->{other}, $line->{other}, "UpdateStats save other param in other field of statistics table"); - is ($params->{itemtype}, $line->{itemtype}, "UpdateStats save itemtype param in itemtype field of statistics table"); - is ($params->{location}, $line->{location}, "UpdateStats save location param in location field of statistics table"); - is ($params->{ccode}, $line->{ccode}, "UpdateStats save ccode param in ccode field of statistics table"); - is ($params->{interface}, $line->{interface}, "UpdateStats save interface param in interface field of statistics table"); - - $dbh->do(q|DELETE FROM statistics|); - $params = { - branch => "BRA", - itemnumber => 31, - borrowernumber => 5, - amount => 5.1, - other => "bla", - itemtype => "BK", - ccode => "CODE", - type => "return", - interface => "INTERFACE", - }; - UpdateStats($params); - $sth = $dbh->prepare("SELECT * FROM statistics"); - $sth->execute(); - $line = ${ $sth->fetchall_arrayref( {} ) }[0]; - is( $line->{location}, undef, - "UpdateStats sets location to NULL if no location is passed in." ); - - $dbh->do(q|DELETE FROM statistics|); - $params = { - branch => "BRA", - itemnumber => 31, - borrowernumber => 5, - amount => 5.1, - other => "bla", - itemtype => "BK", - location => undef, - ccode => "CODE", - type => "return", - interface => "interface" - }; - UpdateStats($params); - $sth = $dbh->prepare("SELECT * FROM statistics"); - $sth->execute(); - $line = ${ $sth->fetchall_arrayref( {} ) }[0]; - is( $line->{location}, undef, - "UpdateStats sets location to NULL if undef is passed in." ); -}; - -$schema->storage->txn_rollback; -- 2.41.0