Bugzilla – Attachment 157445 Details for
Bug 33608
Allow to get statistics about found/recovered books
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33608: Polish Koha::Statistic further
Bug-33608-Polish-KohaStatistic-further.patch (text/plain), 11.14 KB, created by
Marcel de Rooy
on 2023-10-19 14:10:19 UTC
(
hide
)
Description:
Bug 33608: Polish Koha::Statistic further
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2023-10-19 14:10:19 UTC
Size:
11.14 KB
patch
obsolete
>From 57550c770f5aa9e18eed168fed37232b72b534e9 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Mon, 8 May 2023 09:17:32 +0000 >Subject: [PATCH] Bug 33608: Polish Koha::Statistic further >Content-Type: text/plain; charset=utf-8 > >Adding exceptions, removing croaks. >No exception in new for unknown hash keys, store will catch that. >Prepare switching amount parameter to value (db column name). > >Test plan: >Run t/db_dependent/Stats.t >Run t/db_dependent/Koha/Items.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Signed-off-by: Owen Leonard <oleonard@myacpl.org> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >[EDIT] Fixed the mandatory check in Stats.t. Removed key_or_default. > Additional tidy. >--- > Koha/Statistic.pm | 90 ++++++++++++++++-------------------------- > t/db_dependent/Stats.t | 28 ++++++------- > 2 files changed, 46 insertions(+), 72 deletions(-) > >diff --git a/Koha/Statistic.pm b/Koha/Statistic.pm >index c33abf5493..fb9c28a7c6 100644 >--- a/Koha/Statistic.pm >+++ b/Koha/Statistic.pm >@@ -1,5 +1,7 @@ > package Koha::Statistic; > >+# Copyright 2019, 2023 Koha development team >+# > # This file is part of Koha. > # > # Koha is free software; you can redistribute it and/or modify it >@@ -27,7 +29,7 @@ use base qw(Koha::Object); > > our @allowed_accounts_types = qw( writeoff payment ); > our @allowed_circulation_types = qw( renew issue localuse return onsite_checkout recall item_found item_lost ); >-our @mandatory_accounts_keys = qw( type branch borrowernumber amount ); >+our @mandatory_accounts_keys = qw( type branch borrowernumber value ); # note that amount is mapped to value > our @mandatory_circulation_keys = qw( type branch borrowernumber itemnumber ccode itemtype ); > > =head1 NAME >@@ -48,7 +50,8 @@ Koha::Statistic - Koha Statistic Object class > itemnumber : itemnumber > borrowernumber : borrowernumber > categorycode : patron category >- amount : transaction amount >+ amount : transaction amount (legacy parameter name) >+ value : transaction amount > other : sipmode > itemtype : itemtype > ccode : collection code >@@ -67,67 +70,42 @@ Koha::Statistic - Koha Statistic Object class > sub new { > my ( $class, $params ) = @_; > >- # make some controls >- return () if !defined $params; >- >- # change these arrays if new types of transaction or new parameters are allowed >- my @allowed_keys = >- qw (type branch amount other itemnumber itemtype borrowernumber ccode location categorycode interface); >+ Koha::Exceptions::BadParameter->throw( parameter => $params ) if !$params || ref($params) ne 'HASH'; >+ Koha::Exceptions::WrongParameter->throw( name => 'type', value => $params->{type} ) if !$params->{type}; > >- my @mandatory_keys = (); >- if ( !exists $params->{type} or !defined $params->{type} ) { >- croak("UpdateStats did not received type param"); >+ if ( exists $params->{amount} ) { # legacy amount parameter still supported >+ $params->{value} //= delete $params->{amount}; > } >- if ( grep ( $_ eq $params->{type}, @allowed_circulation_types ) ) { >- @mandatory_keys = @mandatory_circulation_keys; >- } elsif ( grep ( $_ eq $params->{type}, @allowed_accounts_types ) ) { >- @mandatory_keys = @mandatory_accounts_keys; >+ >+ my $category; >+ if ( grep { $_ eq $params->{type} } @allowed_circulation_types ) { >+ $category = 'circulation'; >+ } elsif ( grep { $_ eq $params->{type} } @allowed_accounts_types ) { >+ $category = 'accounts'; > } else { >- croak( "UpdateStats received forbidden type param: " . $params->{type} ); >- } >- my @missing_params = (); >- for my $mykey (@mandatory_keys) { >- push @missing_params, $mykey if !grep ( /^$mykey/, keys %$params ); >- } >- if ( scalar @missing_params > 0 ) { >- croak( "UpdateStats did not received mandatory param(s): " . join( ", ", @missing_params ) ); >- } >- my @invalid_params = (); >- for my $myparam ( keys %$params ) { >- push @invalid_params, $myparam unless grep { $_ eq $myparam } @allowed_keys; >- } >- if ( scalar @invalid_params > 0 ) { >- croak( "UpdateStats received invalid param(s): " . join( ", ", @invalid_params ) ); >+ Koha::Exceptions::WrongParameter->throw( name => 'type', value => $params->{type} ); > } > >- # get the parameters >- my $branch = $params->{branch}; >- my $type = $params->{type}; >- my $borrowernumber = exists $params->{borrowernumber} ? $params->{borrowernumber} : ''; >- my $itemnumber = exists $params->{itemnumber} ? $params->{itemnumber} : undef; >- my $amount = exists $params->{amount} ? $params->{amount} : 0; >- my $other = exists $params->{other} ? $params->{other} : ''; >- my $itemtype = exists $params->{itemtype} ? $params->{itemtype} : ''; >- my $location = exists $params->{location} ? $params->{location} : undef; >- my $ccode = exists $params->{ccode} ? $params->{ccode} : ''; >- my $categorycode = exists $params->{categorycode} ? $params->{categorycode} : undef; >- my $interface = exists $params->{interface} ? $params->{interface} : undef; >- >- my $dtf = Koha::Database->new->schema->storage->datetime_parser; >+ my @mandatory_keys = $category eq 'circulation' ? @mandatory_circulation_keys : @mandatory_accounts_keys; >+ my @missing = map { exists $params->{$_} ? () : $_ } @mandatory_keys; >+ Koha::Exceptions::MissingParameter->throw( parameter => join( ',', @missing ) ) if @missing; >+ >+ my $datetime = Koha::Database->new->schema->storage->datetime_parser->format_datetime( dt_from_string() ); > return $class->SUPER::new( > { >- datetime => $dtf->format_datetime( dt_from_string() ), >- branch => $branch, >- type => $type, >- value => $amount, >- other => $other, >- itemnumber => $itemnumber, >- itemtype => $itemtype, >- location => $location, >- borrowernumber => $borrowernumber, >- categorycode => $categorycode, >- ccode => $ccode, >- interface => $interface, >+ borrowernumber => $params->{borrowernumber}, # no longer sending empty string (changed 2023) >+ branch => $params->{branch}, >+ categorycode => $params->{categorycode}, >+ ccode => exists $params->{ccode} ? $params->{ccode} : q{}, >+ datetime => $datetime, >+ interface => $params->{interface}, >+ itemnumber => $params->{itemnumber}, >+ itemtype => exists $params->{itemtype} ? $params->{itemtype} : q{}, >+ location => $params->{location}, >+ other => exists $params->{other} ? $params->{other} : q{}, >+ type => $params->{type}, >+ value => exists $params->{value} ? $params->{value} : 0, >+ > } > ); > } >diff --git a/t/db_dependent/Stats.t b/t/db_dependent/Stats.t >index fba30415ba..fa04b826ce 100755 >--- a/t/db_dependent/Stats.t >+++ b/t/db_dependent/Stats.t >@@ -19,6 +19,7 @@ > > use Modern::Perl; > use Test::More tests => 1; >+use Test::Exception; > > use C4::Context; > use C4::Stats qw( UpdateStats ); >@@ -29,14 +30,15 @@ $schema->storage->txn_begin; > my $dbh = C4::Context->dbh; > > subtest 'UpdateStats' => sub { >- plan tests => 17; >- is (UpdateStats () ,undef, "UpdateStats returns undef if no params"); >+ plan tests => 16; >+ >+ throws_ok { UpdateStats() } 'Koha::Exceptions::BadParameter', 'UpdateStats called without params'; > > my $params = { > branch => "BRA", > itemnumber => 31, > borrowernumber => 5, >- amount =>5.1, >+ amount => 5.1, > other => "bla", > itemtype => "BK", > location => "LOC", >@@ -64,13 +66,14 @@ subtest 'UpdateStats' => sub { > 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 @allowed_circulation_types = @Koha::Statistic::allowed_circulation_types; >+ my @allowed_accounts_types = @Koha::Statistic::allowed_accounts_types; >+ my @circulation_mandatory_keys = @Koha::Statistic::mandatory_circulation_keys; >+ my @accounts_mandatory_keys = @Koha::Statistic::mandatory_accounts_keys; > > my @missing_errors = (); > foreach my $key (@circulation_mandatory_keys) { >+ next if $key eq 'type'; > my $value = $params->{$key}; > delete $params->{$key}; > foreach my $type (@allowed_circulation_types) { >@@ -82,6 +85,7 @@ subtest 'UpdateStats' => sub { > $params->{$key} = $value; > } > foreach my $key (@accounts_mandatory_keys) { >+ next if $key eq 'type'; > my $value = $params->{$key}; > delete $params->{$key}; > foreach my $type (@allowed_accounts_types) { >@@ -95,14 +99,6 @@ subtest 'UpdateStats' => sub { > } > is (join (", ", @missing_errors),'',"UpdateStats returns undef and croaks if mandatory params are missing"); > >- # returns undef and croaks if forbidden params are given >- $params -> {type} = "return"; >- $params -> {newparam} = "true"; >- eval {UpdateStats($params)}; >- $return_error = $@; >- isnt ($return_error,'',"UpdateStats returns undef and croaks if a forbidden param is given"); >- delete $params->{newparam}; >- > # save the params in the right database fields > $dbh->do(q|DELETE FROM statistics|); > $params = { >@@ -124,7 +120,7 @@ subtest 'UpdateStats' => sub { > 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"); >- cmp_ok($params->{amount},'==', $line->{value}, "UpdateStats save amount param in value 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"); >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
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