Attachment 151921 Details for Bug 33608 – Bug 33608: Move older Stats tests to Koha/Statistics.t

[patch] Bug 33608: Move older Stats tests to Koha/Statistics.t

Bug-33608-Move-older-Stats-tests-to-KohaStatistics.patch (text/plain), 14.10 KB, created by Owen Leonard on 2023-06-01 12:24:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2023-06-01 12:24:56 UTC

Size: 14.10 KB

patch

obsolete

>From d5031dc0cb6d12c25fb1983b5f96e767150f81ee Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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 <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Owen Leonard <oleonard@myacpl.org>
>---
> 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 <http://www.gnu.org/licenses>.
>-
>-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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 33608: 150840 | 150841 | 150842 | 150843 | 150844 | 150845 | 150846 | 151051 | 151052 | 151053 | 151054 | 151055 | 151056 | 151057 | 151913 | 151915 | 151916 | 151917 | 151918 | 151919 | 151920 | 151921 | 151922 | 153653 | 153654 | 153655 | 153656 | 153657 | 153658 | 153659 | 153660 | 153661 | 153698 | 153699 | 153700 | 153701 | 153702 | 153703 | 153704 | 153705 | 153706 | 153707 | 153708 | 153709 | 153710 | 153711 | 153712 | 153713 | 153765 | 157440 | 157441 | 157442 | 157443 | 157444 | 157445 | 157446 | 157447 | 157448 | 157449