Attachment 85765 Details for Bug 20750 – Bug 20750: (follow-up) Pass hashrefs to functions

[patch] Bug 20750: (follow-up) Pass hashrefs to functions

Bug-20750-follow-up-Pass-hashrefs-to-functions.patch (text/plain), 6.43 KB, created by Andrew Isherwood on 2019-02-27 11:41:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2019-02-27 11:41:08 UTC

Size: 6.43 KB

patch

obsolete

>From 511c40c26bda07593db9c71e2cc397b9529c2a42 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>Date: Wed, 27 Feb 2019 11:40:12 +0000
>Subject: [PATCH] Bug 20750: (follow-up) Pass hashrefs to functions
>
>As per comment #100 here:
>https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c100
>---
> Koha/Illrequest.pm                 | 21 +++++++-----
> Koha/Illrequest/Logger.pm          | 68 ++++++++++++++++++++------------------
> t/db_dependent/Illrequest/Logger.t |  9 ++---
> 3 files changed, 53 insertions(+), 45 deletions(-)
>
>diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm
>index 3d7bc37bb0..8bec6074f1 100644
>--- a/Koha/Illrequest.pm
>+++ b/Koha/Illrequest.pm
>@@ -212,10 +212,10 @@ sub status_alias {
>         my $val_to_log = $val ? $new_status_alias : scalar $self->status;
>         if ($ret) {
>             my $logger = Koha::Illrequest::Logger->new;
>-            $logger->log_status_change(
>-                $self,
>-                $val_to_log
>-            );
>+            $logger->log_status_change({
>+                request => $self,
>+                value   => $val_to_log
>+            });
>         } else {
>             delete $self->{previous_status};
>         }
>@@ -269,10 +269,10 @@ sub status {
>             $self->status_alias("-1");
>         } else {
>             my $logger = Koha::Illrequest::Logger->new;
>-            $logger->log_status_change(
>-                $self,
>-                $new_status
>-            );
>+            $logger->log_status_change({
>+                request => $self,
>+                value   => $new_status
>+            });
>         }
>         delete $self->{previous_status};
>         return $ret;
>@@ -1178,7 +1178,10 @@ sub store {
> 
>     if ($ret && defined $attrs) {
>         my $logger = Koha::Illrequest::Logger->new;
>-        $logger->log_maybe($self, $attrs);
>+        $logger->log_maybe({
>+            request => $self,
>+            attrs   => $attrs
>+        });
>     }
> 
>     return $ret;
>diff --git a/Koha/Illrequest/Logger.pm b/Koha/Illrequest/Logger.pm
>index 566e8f6fca..0625370237 100644
>--- a/Koha/Illrequest/Logger.pm
>+++ b/Koha/Illrequest/Logger.pm
>@@ -79,21 +79,24 @@ sub new {
> 
> =head3 log_maybe
> 
>-    Koha::IllRequest::Logger->log_maybe($attrs);
>+    Koha::IllRequest::Logger->log_maybe($params);
> 
>-Receive request object and an attributes hashref (which may or may
>-not be defined) If the hashref contains a key matching our "loggers" hashref
>-then we want to log it
>+Receive params hashref, containing a request object and an attrs
>+hashref (which may or may not be defined) If the attrs hashref contains
>+a key matching our "loggers" hashref then we want to log it
> 
> =cut
> 
> sub log_maybe {
>-    my ($self, $req, $attrs) = @_;
>+    my ($self, $params) = @_;
> 
>-    if (defined $req && defined $attrs) {
>-        foreach my $key (keys %{ $attrs }) {
>+    if (defined $params->{request} && defined $params->{attrs}) {
>+        foreach my $key (keys %{ $params->{attrs} }) {
>             if (defined($self->{loggers}->{$key})) {
>-                $self->{loggers}->{$key}($req, $attrs->{$key});
>+                $self->{loggers}->{$key}(
>+                    $params->{request},
>+                    $params->{attrs}->{$key}
>+                );
>             }
>         }
>     }
>@@ -101,25 +104,28 @@ sub log_maybe {
> 
> =head3 log_status_change
> 
>-    Koha::IllRequest::Logger->log_status_change($req, $new_status);
>+    Koha::IllRequest::Logger->log_status_change($params);
> 
>-Log a request's status change
>+Receive a hashref containing a request object and a status to log,
>+and log it
> 
> =cut
> 
> sub log_status_change {
>-    my ( $self, $req, $new_status ) = @_;
>-
>-    $self->log_something({
>-        modulename   => 'ILL',
>-        actionname   => 'STATUS_CHANGE',
>-        objectnumber => $req->id,
>-        infos        => to_json({
>-            log_origin    => 'core',
>-            status_before => $req->{previous_status},
>-            status_after  => $new_status
>-        })
>-    });
>+    my ( $self, $params ) = @_;
>+
>+    if (defined $params->{request} && defined $params->{value}) {
>+        $self->log_something({
>+            modulename   => 'ILL',
>+            actionname   => 'STATUS_CHANGE',
>+            objectnumber => $params->{request}->id,
>+            infos        => to_json({
>+                log_origin    => 'core',
>+                status_before => $params->{request}->{previous_status},
>+                status_after  => $params->{value}
>+            })
>+        });
>+    }
> }
> 
> =head3 log_something
>@@ -160,14 +166,14 @@ sub log_something {
> 
> =head3 get_log_template
> 
>-    $template_path = get_log_template($origin, $action);
>+    $template_path = get_log_template($params);
> 
> Given a log's origin and action, get the appropriate display template
> 
> =cut
> 
> sub get_log_template {
>-    my ($self, $req, $params) = @_;
>+    my ($self, $params) = @_;
> 
>     my $origin = $params->{origin};
>     my $action = $params->{action};
>@@ -179,7 +185,7 @@ sub get_log_template {
>     } else {
>         # It's probably a backend log, so we need to get the path to the
>         # template from the backend
>-        my $backend =$req->{_my_backend};
>+        my $backend =$params->{request}->{_my_backend};
>         return $backend->get_log_template_path($action);
>     }
> }
>@@ -212,13 +218,11 @@ sub get_request_logs {
>     foreach my $log(@{$logs}) {
>         $log->{aliases} = $alias_hash;
>         $log->{info} = from_json($log->{info});
>-        $log->{template} = $self->get_log_template(
>-        $request,
>-            {
>-                origin => $log->{info}->{log_origin},
>-                action => $log->{action}
>-            }
>-        );
>+        $log->{template} = $self->get_log_template({
>+            request => $request,
>+            origin => $log->{info}->{log_origin},
>+            action => $log->{action}
>+        });
>     }
> 
>     return $logs;
>diff --git a/t/db_dependent/Illrequest/Logger.t b/t/db_dependent/Illrequest/Logger.t
>index 7beb8d62ab..e3b5670221 100644
>--- a/t/db_dependent/Illrequest/Logger.t
>+++ b/t/db_dependent/Illrequest/Logger.t
>@@ -103,10 +103,11 @@ subtest 'Basics' => sub {
>     # get_log_template()
>     #
>     is(
>-        $logger->get_log_template(
>-            {},
>-            { origin => 'core', 'action' => 'STATUS_CHANGE' }
>-        ),
>+        $logger->get_log_template({
>+            request => {},
>+            origin => 'core',
>+            action => 'STATUS_CHANGE'
>+        }),
>         'base/status_change.tt',
>         'get_log_template() fetches correct core template'
>     );
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 20750: 75304 | 75326 | 75329 | 75330 | 77757 | 77816 | 77817 | 77818 | 77931 | 79822 | 79823 | 79824 | 80860 | 82082 | 82083 | 82085 | 82096 | 82097 | 82098 | 82099 | 82500 | 82501 | 82502 | 84193 | 84194 | 84195 | 84203 | 84266 | 84497 | 84498 | 84821 | 84823 | 84824 | 84825 | 84826 | 84827 | 84973 | 84974 | 84975 | 84976 | 84977 | 84978 | 84979 | 85097 | 85098 | 85099 | 85101 | 85113 | 85238 | 85266 | 85697 | 85698 | 85699 | 85700 | 85701 | 85702 | 85703 | 85704 | 85705 | 85706 | 85707 | 85708 | 85709 | 85710 | 85756 | 85757 | 85765 | 85826 | 85827 | 85829 | 85830 | 85831 | 85832 | 85833 | 85834 | 85835 | 85836 | 85837 | 85838 | 85839 | 85840 | 85841 | 85842 | 85843 | 85844 | 85845 | 85846 | 85847 | 85848 | 86434 | 86435 | 86436 | 86534 | 86535 | 86536