From c76c28a881430a34d8b9696f74dbe18dcbac20bb Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Wed, 27 Feb 2019 10:57:20 +0000 Subject: [PATCH] Bug 20750: (follow-up) Don't use object variable We were unecessarily setting a property on the Logger object containing the data to be logged, prior to calling log_something. This was overcomplicated things and could be problematic if an unexpected call to log_something was made. Now we just explicitly pass the data to log_something. As per comment #99 here: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20750#c99 --- Koha/Illrequest/Logger.pm | 65 ++++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/Koha/Illrequest/Logger.pm b/Koha/Illrequest/Logger.pm index da4f65947b..566e8f6fca 100644 --- a/Koha/Illrequest/Logger.pm +++ b/Koha/Illrequest/Logger.pm @@ -49,8 +49,8 @@ relating to Illrequest to the action log. my $config = Koha::Illrequest::Logger->new(); -Create a new Koha::Illrequest::Logger object, with skeleton logging data -We also set up data of what can be logged, how to do it and how to display +Create a new Koha::Illrequest::Logger object. +We also set up what can be logged, how to do it and how to display log entries we get back out =cut @@ -59,10 +59,6 @@ sub new { my ( $class ) = @_; my $self = {}; - $self->{data} = { - modulename => 'ILL' - }; - $self->{loggers} = { status => sub { $self->log_status_change(@_); @@ -105,7 +101,7 @@ sub log_maybe { =head3 log_status_change - Koha::IllRequest::Logger->log_status_change(); + Koha::IllRequest::Logger->log_status_change($req, $new_status); Log a request's status change @@ -114,7 +110,8 @@ Log a request's status change sub log_status_change { my ( $self, $req, $new_status ) = @_; - $self->set_data({ + $self->log_something({ + modulename => 'ILL', actionname => 'STATUS_CHANGE', objectnumber => $req->id, infos => to_json({ @@ -123,56 +120,44 @@ sub log_status_change { status_after => $new_status }) }); - - $self->log_something(); } =head3 log_something - Koha::IllRequest::Logger->log_something(); + Koha::IllRequest::Logger->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 + }) + }); -If we have the required data set, log an action +If we have the required data passed, log an action =cut sub log_something { - my ( $self ) = @_; + my ( $self, $to_log ) = @_; if ( - defined $self->{data}->{modulename} && - defined $self->{data}->{actionname} && - defined $self->{data}->{objectnumber} && - defined $self->{data}->{infos} && + defined $to_log->{modulename} && + defined $to_log->{actionname} && + defined $to_log->{objectnumber} && + defined $to_log->{infos} && C4::Context->preference("IllLog") ) { logaction( - $self->{data}->{modulename}, - $self->{data}->{actionname}, - $self->{data}->{objectnumber}, - $self->{data}->{infos} + $to_log->{modulename}, + $to_log->{actionname}, + $to_log->{objectnumber}, + $to_log->{infos} ); } } -=head3 set_data - - Koha::IllRequest::Logger->set_data({ - key => 'value', - key2 => 'value2' - }); - -Set arbitrary data propert(ies) on the logger object - -=cut - -sub set_data { - my ( $self, $data ) = @_; - - foreach my $key (keys %{ $data }) { - $self->{data}->{$key} = $data->{$key}; - } -} - =head3 get_log_template $template_path = get_log_template($origin, $action); -- 2.11.0