Attachment 157444 Details for Bug 33608 – Bug 33608: Redirect UpdateStats to Koha namespace

[patch] Bug 33608: Redirect UpdateStats to Koha namespace

Bug-33608-Redirect-UpdateStats-to-Koha-namespace.patch (text/plain), 13.30 KB, created by Marcel de Rooy on 2023-10-19 14:10:17 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2023-10-19 14:10:17 UTC

Size: 13.30 KB

patch

obsolete

>From b58f12732bea393afb8e5a712ea30d8723f9094c Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Wed, 26 Apr 2023 15:58:13 +0200
>Subject: [PATCH] Bug 33608: Redirect UpdateStats to Koha namespace
>Content-Type: text/plain; charset=utf-8
>
>Moving code to Koha::Statistic->new and ->insert.
>Polishing code a bit further in next patch.
>
>Test plan:
>Run t/db_dependent/Stats.t
>Run t/db_dependent/Koha/Statistics.t
>Run t/db_dependent/Koha/Pseudonymization.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] POD headers, use plural module and tidy Koha::Statistic
>---
> C4/Stats.pm                            | 108 ++-----------------
> Koha/Statistic.pm                      | 142 ++++++++++++++++++++++++-
> t/db_dependent/Koha/Pseudonymization.t |   1 +
> 3 files changed, 146 insertions(+), 105 deletions(-)
>
>diff --git a/C4/Stats.pm b/C4/Stats.pm
>index 7d67c8a18b..bfd2ad52e2 100644
>--- a/C4/Stats.pm
>+++ b/C4/Stats.pm
>@@ -1,6 +1,5 @@
> package C4::Stats;
> 
>-
> # Copyright 2000-2002 Katipo Communications
> #
> # This file is part of Koha.
>@@ -19,12 +18,8 @@ package C4::Stats;
> # along with Koha; if not, see <http://www.gnu.org/licenses>.
> 
> use Modern::Perl;
>-use Carp qw( croak );
>-use C4::Context;
> 
>-use Koha::DateUtils qw( dt_from_string );
> use Koha::Statistics;
>-use Koha::PseudonymizedTransactions;
> 
> our (@ISA, @EXPORT_OK);
> BEGIN {
>@@ -35,14 +30,13 @@ BEGIN {
>     );
> }
> 
>-
> =head1 NAME
> 
> C4::Stats - Update Koha statistics (log)
> 
> =head1 SYNOPSIS
> 
>-  use C4::Stats;
>+    use C4::Stats;
> 
> =head1 DESCRIPTION
> 
>@@ -52,105 +46,16 @@ The functions of this module deals with statistics table of Koha database.
> 
> =head2 UpdateStats
> 
>-  &UpdateStats($params);
>-
>-Adds an entry to the statistics table in the Koha database, which acts as an activity log.
>+    C4::Stats::UpdateStats($params);
> 
>-C<$params> is an hashref whose expected keys are:
>-    branch             : the branch where the transaction occurred
>-    type               : the type of transaction (renew, issue, localuse, return, writeoff, payment
>-    itemnumber         : the itemnumber of the item
>-    borrowernumber     : the borrowernumber of the patron
>-    amount             : the amount of the transaction
>-    other              : sipmode
>-    itemtype           : the type of the item
>-    ccode              : the collection code of the item
>-    categorycode       : the categorycode of the patron
>-    interface          : the context this action was taken in
>-
>-type key is mandatory.
>-For types used in C4::Circulation (renew,issue,localuse,return), the following other keys are mandatory:
>-branch, borrowernumber, itemnumber, ccode, itemtype
>-For types used in C4::Accounts (writeoff, payment), the following other keys are mandatory:
>-branch, borrowernumber, itemnumber, ccode, itemtype
>-If an optional key is not provided, the value '' is used for this key.
>-
>-Returns undef if no C<$param> is given
>+    This is a (legacy) alias for Koha::Statistic->insert($params).
>+    Please see Koha::Statistic module.
> 
> =cut
> 
> sub UpdateStats {
>-    my ($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);
>-    my @allowed_circulation_types = qw (renew issue localuse return onsite_checkout recall item_found item_lost);
>-    my @allowed_accounts_types = qw (writeoff payment);
>-    my @circulation_mandatory_keys = qw (type branch borrowernumber itemnumber ccode itemtype);
>-    my @accounts_mandatory_keys = qw (type branch borrowernumber amount);
>-
>-    my @mandatory_keys = ();
>-    if (! exists $params->{type} or ! defined $params->{type}) {
>-        croak ("UpdateStats did not received type param");
>-    }
>-    if (grep ($_ eq $params->{type}, @allowed_circulation_types  )) {
>-        @mandatory_keys = @circulation_mandatory_keys;
>-    } elsif (grep ($_ eq $params->{type}, @allowed_accounts_types )) {
>-        @mandatory_keys = @accounts_mandatory_keys;
>-    } 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 ));
>-    }
>-# 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 $statistic = Koha::Statistic->new(
>-        {
>-            datetime       => $dtf->format_datetime( dt_from_string ),
>-            branch         => $branch,
>-            type           => $type,
>-            value          => $amount,
>-            other          => $other,
>-            itemnumber     => $itemnumber,
>-            itemtype       => $itemtype,
>-            location       => $location,
>-            borrowernumber => $borrowernumber,
>-            ccode          => $ccode,
>-            categorycode   => $categorycode,
>-            interface      => $interface,
>-        }
>-    )->store;
>-
>-    Koha::PseudonymizedTransaction->new_from_statistic($statistic)->store
>-      if C4::Context->preference('Pseudonymization')
>-        && $borrowernumber # Not a real transaction if the patron does not exist
>-                           # For instance can be a transfer, or hold trigger
>-        && grep { $_ eq $params->{type} } qw(renew issue return onsite_checkout);
>+    my $params = shift;
>+    Koha::Statistic->insert($params);
> }
> 
> 1;
>@@ -161,4 +66,3 @@ __END__
> Koha Development Team <http://koha-community.org/>
> 
> =cut
>-
>diff --git a/Koha/Statistic.pm b/Koha/Statistic.pm
>index 397e440cdb..c33abf5493 100644
>--- a/Koha/Statistic.pm
>+++ b/Koha/Statistic.pm
>@@ -17,12 +17,19 @@ package Koha::Statistic;
> 
> use Modern::Perl;
> 
>-
>+use C4::Context;
> use Koha::Database;
>+use Koha::DateUtils qw/dt_from_string/;
> use Koha::Items;
>+use Koha::PseudonymizedTransaction;
> 
> 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_circulation_keys = qw( type branch borrowernumber itemnumber ccode itemtype );
>+
> =head1 NAME
> 
> Koha::Statistic - Koha Statistic Object class
>@@ -31,11 +38,140 @@ Koha::Statistic - Koha Statistic Object class
> 
> =head2 Class methods
> 
>+=head3 new
>+
>+    my $record = Koha::Statistic->new( $params );
>+
>+    C<$params> is an hashref whose expected keys are:
>+    branch             : transaction branch
>+    type               : transaction type
>+    itemnumber         : itemnumber
>+    borrowernumber     : borrowernumber
>+    categorycode       : patron category
>+    amount             : transaction amount
>+    other              : sipmode
>+    itemtype           : itemtype
>+    ccode              : collection code
>+    interface          : the context this action was taken in
>+
>+    The type key is mandatory, see @Koha::Statistic::allowed_account_types or
>+    @Koha::Statistic::allowed_circulation_types.
>+    Some keys are mandatory, depending on type. See @mandatory_accounts_keys
>+    or @mandatory_circulation_keys.
>+
>+    The method throws an exception when given bad data, otherwise returns a
>+    Koha::Statistic object, which is not yet stored.
>+
>+=cut
>+
>+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);
>+
>+    my @mandatory_keys = ();
>+    if ( !exists $params->{type} or !defined $params->{type} ) {
>+        croak("UpdateStats did not received type param");
>+    }
>+    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;
>+    } 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 ) );
>+    }
>+
>+    # 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;
>+    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,
>+        }
>+    );
>+}
>+
>+=head3 store
>+
>+    $statistic->store;
>+
>+    This call includes pseudonymization if enabled.
>+
>+=cut
>+
>+sub store {
>+    my ($self) = @_;
>+    $self->SUPER::store;
>+    Koha::PseudonymizedTransaction->new_from_statistic($self)->store
>+        if C4::Context->preference('Pseudonymization')
>+        && $self->borrowernumber    # Not a real transaction if the patron does not exist
>+                                    # For instance can be a transfer, or hold trigger
>+        && grep { $_ eq $self->type } qw(renew issue return onsite_checkout);
>+    return $self;
>+}
>+
>+=head3 insert
>+
>+    Koha::Statistic->insert( $params );
>+
>+    This is a shorthand for ->new($params)->store.
>+    It is the new equivalent for the legacy C4::Stats::UpdateStats call.
>+
>+=cut
>+
>+sub insert {
>+    my ( $class, $params ) = @_;
>+    my $statistic = $class->new($params) or return;
>+    $statistic->store;
>+    return $statistic;
>+}
>+
> =head3 item
> 
>-my $item = $statistic->item;
>+    my $item = $statistic->item;
> 
>-Return the item associated to this statistic.
>+    Return the item associated to this statistic.
> 
> =cut
> 
>diff --git a/t/db_dependent/Koha/Pseudonymization.t b/t/db_dependent/Koha/Pseudonymization.t
>index 93cea32f21..ee7dd5732c 100755
>--- a/t/db_dependent/Koha/Pseudonymization.t
>+++ b/t/db_dependent/Koha/Pseudonymization.t
>@@ -29,6 +29,7 @@ use Koha::Database;
> use Koha::DateUtils qw( dt_from_string );
> use Koha::Patrons;
> use Koha::PseudonymizedTransactions;
>+use Koha::Statistics;
> 
> use t::lib::TestBuilder;
> use t::lib::Mocks;
>-- 
>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