From 0bc2650421dfaa92cce63b271aaec4f9486fb28f Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Tue, 2 Oct 2018 14:55:58 +0100 Subject: [PATCH] Bug 20750: Allow logging of arbitrary actions This patch allows logging of arbitrary actions on request objects. A chronological summary of these actions can then be displayed when viewing a request. This patch also adds logging of request status changes. Additional work has been done on the BLDSS backend to log requests to the BLDSS request status check API. To test: - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's status. - Navigate to the "Manage ILL request" screen - Click the "Display request log" button - Observe that a modal opens displaying a summary of the status changes. Signed-off-by: Niamh Walker --- Koha/Illrequest.pm | 85 ++++++-- Koha/Illrequest/Logger.pm | 242 +++++++++++++++++++++ .../bug_20750-add_illlog_preference.perl | 7 + .../data/mysql/en/optional/ill_logging_pref.sql | 1 + .../data/mysql/en/optional/ill_logging_pref.txt | 1 + installer/data/mysql/sysprefs.sql | 1 + .../prog/en/modules/admin/preferences/logs.pref | 6 + .../prog/en/modules/ill/ill-requests.tt | 36 ++- .../prog/en/modules/ill/log/status_change.tt | 7 + .../intranet-tmpl/prog/en/modules/tools/viewlog.tt | 5 +- 10 files changed, 375 insertions(+), 16 deletions(-) create mode 100644 Koha/Illrequest/Logger.pm create mode 100644 installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl create mode 100644 installer/data/mysql/en/optional/ill_logging_pref.sql create mode 100644 installer/data/mysql/en/optional/ill_logging_pref.txt create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm index 98e4d707d4..375e7586a1 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -32,6 +32,7 @@ use Koha::Exceptions::Ill; use Koha::Illcomments; use Koha::Illrequestattributes; use Koha::AuthorisedValue; +use Koha::Illrequest::Logger; use Koha::Patron; use Koha::AuthorisedValues; @@ -157,6 +158,16 @@ sub illcomments { ); } +=head3 logs + +=cut + +sub logs { + my ( $self ) = @_; + my $logger = Koha::Illrequest::Logger->new; + return $logger->get_request_logs($self); +} + =head3 patron =cut @@ -210,25 +221,44 @@ sub status_alias { =head3 status + $Illrequest->status('CANREQ'); + Overloaded getter/setter for request status, -also nullifies status_alias +also nullifies status_alias and records the fact that the status has changed =cut sub status { - my ( $self, $newval) = @_; - if ($newval) { - # This is hackery to enable us to undefine - # status_alias, since we need to have an overloaded - # status_alias method to get us around the problem described - # here: - # https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c156 - # We need a way of passing implied undef to nullify status_alias - # so we pass -1, which is special cased in the overloaded setter - $self->status_alias("-1"); - return $self->SUPER::status($newval); + my ( $self, $new_status) = @_; + + my $current_status = $self->SUPER::status; + + if ($new_status) { + # Keep a record of the previous status before we change it, + # we might need it + $self->{previous_status} = $current_status; + my $ret = $self->SUPER::status($new_status)->store; + if ($ret) { + # This is hackery to enable us to undefine + # status_alias, since we need to have an overloaded + # status_alias method to get us around the problem described + # here: + # https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c156 + # We need a way of passing implied undef to nullify status_alias + # so we pass -1, which is special cased in the overloaded setter + $self->status_alias("-1"); + my $logger = Koha::Illrequest::Logger->new; + $logger->log_status_change( + $self, + $new_status + ); + } else { + delete $self->{previous_status}; + } + return $ret; + } else { + return $current_status; } - return $self->SUPER::status; } =head3 load_backend @@ -252,7 +282,10 @@ sub load_backend { my $location = join "/", @raw, $backend_name, "Base.pm"; # File to load my $backend_class = join "::", @raw, $backend_name, "Base"; # Package name require $location; - $self->{_my_backend} = $backend_class->new({ config => $self->_config }); + $self->{_my_backend} = $backend_class->new({ + config => $self->_config, + logger => Koha::Illrequest::Logger->new + }); return $self; } @@ -1107,6 +1140,30 @@ sub _censor { return $params; } +=head3 store + + $Illrequest->store; + +Overloaded I method that, in addition to performing the 'store', +possibly records the fact that something happened + +=cut + +sub store { + my ( $self, $attrs ) = @_; + + my $ret = $self->SUPER::store; + + $attrs->{log_origin} = 'core'; + + if ($ret && defined $attrs) { + my $logger = Koha::Illrequest::Logger->new; + $logger->log_maybe($self, $attrs); + } + + return $ret; +} + =head3 TO_JSON $json = $illrequest->TO_JSON diff --git a/Koha/Illrequest/Logger.pm b/Koha/Illrequest/Logger.pm new file mode 100644 index 0000000000..6cfa3031b3 --- /dev/null +++ b/Koha/Illrequest/Logger.pm @@ -0,0 +1,242 @@ +package Koha::Illrequest::Logger; + +# Copyright 2018 PTFS Europe Ltd +# +# 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, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; +use JSON qw( to_json from_json ); + +use C4::Context; +use C4::Templates; +use C4::Log qw( logaction GetLogs ); + +=head1 NAME + +Koha::Illrequest::Logger - Koha ILL Action / Event logger + +=head1 SYNOPSIS + +Object-oriented class that provides event logging functionality for +ILL requests + +=head1 DESCRIPTION + +This class provides the ability to log arbitrary actions or events +relating to Illrequest to the action log. + +=head1 API + +=head2 Class Methods + +=head3 new + + 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 +log entries we get back out + +=cut + +sub new { + my ( $class ) = @_; + my $self = {}; + + $self->{data} = { + modulename => 'ILL' + }; + + $self->{loggers} = { + status => sub { + $self->log_status_change(@_); + } + }; + + my ( $htdocs, $theme, $lang, $base ) = + C4::Templates::_get_template_file('ill/log/', 'intranet'); + + $self->{templates} = { + STATUS_CHANGE => $base . 'status_change.tt' + }; + + bless $self, $class; + + return $self; +} + +=head3 log_maybe + + Koha::IllRequest::Logger->log_maybe($attrs); + +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 + +=cut + +sub log_maybe { + my ($self, $req, $attrs) = @_; + + if (defined $req && defined $attrs) { + foreach my $key (keys %{ $attrs }) { + if (defined($self->{loggers}->{$key})) { + $self->{loggers}->{$key}($req, $attrs->{$key}); + } + } + } +} + +=head3 log_status_change + + Koha::IllRequest::Logger->log_status_change(); + +Log a request's status change + +=cut + +sub log_status_change { + my ( $self, $req, $new_status ) = @_; + + $self->set_data({ + actionname => 'STATUS_CHANGE', + objectnumber => $req->id, + infos => to_json({ + log_origin => 'core', + status_before => $req->{previous_status}, + status_after => $new_status + }) + }); + + $self->log_something(); +} + +=head3 log_something + + Koha::IllRequest::Logger->log_something(); + +If we have the required data set, log an action + +=cut + +sub log_something { + my ( $self ) = @_; + + if ( + defined $self->{data}->{modulename} && + defined $self->{data}->{actionname} && + defined $self->{data}->{objectnumber} && + defined $self->{data}->{infos} && + C4::Context->preference("IllLog") + ) { + logaction( + $self->{data}->{modulename}, + $self->{data}->{actionname}, + $self->{data}->{objectnumber}, + $self->{data}->{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); + +Given a log's origin and action, get the appropriate display template + +=cut + +sub get_log_template { + my ($self, $req, $params) = @_; + + my $origin = $params->{origin}; + my $action = $params->{action}; + + if ($origin eq 'core') { + # It's a core log, so we can just get the template path from + # the hashref above + return $self->{templates}->{$action}; + } 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}; + return $backend->get_log_template_path($action); + } +} + +=head3 get_request_logs + + $requestlogs = Koha::IllRequest::Logger->get_request_logs($request_id); + +Get all logged actions for a given request + +=cut + +sub get_request_logs { + my ( $self, $request ) = @_; + + my $logs = GetLogs( + undef, + undef, + undef, + ['ILL'], + undef, + $request->id, + undef, + undef + ); + foreach my $log(@{$logs}) { + $log->{info} = from_json($log->{info}); + $log->{template} = $self->get_log_template( + $request, + { + origin => $log->{info}->{log_origin}, + action => $log->{action} + } + ); + } + + my @sorted = sort {$$b{'timestamp'} <=> $$a{'timestamp'}} @{$logs}; + + return \@sorted; +} + +=head1 AUTHOR + +Andrew Isherwood + +=cut + +1; diff --git a/installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl b/installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl new file mode 100644 index 0000000000..b3317ee254 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl @@ -0,0 +1,7 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do( "INSERT IGNORE INTO systempreferences (variable, value, explanation, type) VALUES ('IllLog', 1, 'If ON, log information about ILL requests', 'YesNo')" ); + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 20750 - Allow timestamped auditing of ILL request events)\n"; +} diff --git a/installer/data/mysql/en/optional/ill_logging_pref.sql b/installer/data/mysql/en/optional/ill_logging_pref.sql new file mode 100644 index 0000000000..2316908dfe --- /dev/null +++ b/installer/data/mysql/en/optional/ill_logging_pref.sql @@ -0,0 +1 @@ +INSERT IGNORE INTO systempreferences (variable, value, explanation, type) VALUES ('IllLog', 1, 'If ON, log information about ILL requests', 'YesNo'); diff --git a/installer/data/mysql/en/optional/ill_logging_pref.txt b/installer/data/mysql/en/optional/ill_logging_pref.txt new file mode 100644 index 0000000000..eaa8f5f38e --- /dev/null +++ b/installer/data/mysql/en/optional/ill_logging_pref.txt @@ -0,0 +1 @@ +System preference to determine if logging of ILL requests should occur diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index d142b14a0e..bb36a0253c 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -216,6 +216,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('IDreamBooksResults','0','','Display IDreamBooks.com rating in search results','YesNo'), ('IDreamBooksReviews','0','','Display book review snippets from IDreamBooks.com','YesNo'), ('IdRef','0','','Disable/enable the IdRef webservice from the OPAC detail page.','YesNo'), +('IllLog', 1, '', 'If ON, log information about ILL requests', 'YesNo'), ('ILLModule','0','If ON, enables the interlibrary loans module.','','YesNo'), ('ILLModuleCopyrightClearance','','70|10','Enter text to enable the copyright clearance stage of request creation. Text will be displayed','Textarea'), ('ILS-DI','0','','Enables ILS-DI services at OPAC.','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref index 2a94ff6ab6..b3ca3c2433 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref @@ -37,6 +37,12 @@ Logging: off: "Don't log" - any actions on holds (create, cancel, suspend, resume, etc). - + - pref: IllLog + choices: + on: Log + off: "Don't log" + - when changes to ILL requests take place + - - pref: IssueLog choices: on: Log diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt index 2041cf0172..131509cb7b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ -319,6 +319,10 @@ Display supplier metadata + + + Display request log +
@@ -429,6 +433,30 @@
+ +

[% request.illcomments.count | html %] comments

@@ -467,7 +495,7 @@
- + [% ELSIF query_type == 'illlist' %] @@ -835,6 +863,12 @@ } }; + // Display the modal containing request supplier metadata + $('#ill-request-display-log').on('click', function(e) { + e.preventDefault(); + $('#requestLog').modal({show:true}); + }); + // Toggle request attributes in Illview $('#toggle_requestattributes').on('click', function(e) { e.preventDefault(); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt new file mode 100644 index 0000000000..ceb94fb73e --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt @@ -0,0 +1,7 @@ +

+[% log.timestamp | $KohaDates with_hours => 1 %] : Status changed +[% IF log.info.status_before %] +from "[% request.capabilities(log.info.status_before).name %]" +[% END %] +to "[% request.capabilities(log.info.status_after).name %]" +

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt index db17762656..07e4d5e229 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ -28,6 +28,7 @@ [% CASE 'ACQUISITIONS' %]Acquisitions [% CASE 'SERIAL' %]Serials [% CASE 'HOLDS' %]Holds +[% CASE 'ILL' %]Interlibrary loans [% CASE 'CIRCULATION' %]Circulation [% CASE 'LETTER' %]Letter [% CASE 'FINES' %]Fines @@ -53,6 +54,8 @@ [% CASE 'CHANGE PASS' %]Change password [% CASE 'ADDCIRCMESSAGE' %]Add circulation message [% CASE 'DELCIRCMESSAGE' %]Delete circulation message +[% CASE 'STATUS_CHANGE' %]Change ILL request status +[% CASE 'BLDSS_STATUS_CHECK' %]Check ILL request status with BLDSS [% CASE 'Run' %]Run [% CASE %][% action | html %] [% END %] @@ -102,7 +105,7 @@ [% ELSE %] [% END %] - [% FOREACH modx IN [ 'CATALOGUING' 'AUTHORITIES' 'MEMBERS' 'ACQUISITIONS' 'SERIAL' 'HOLDS' 'CIRCULATION' 'LETTER' 'FINES' 'SYSTEMPREFERENCE' 'CRONJOBS', 'REPORTS' ] %] + [% FOREACH modx IN [ 'CATALOGUING' 'AUTHORITIES' 'MEMBERS' 'ACQUISITIONS' 'SERIAL' 'HOLDS' 'ILL' 'CIRCULATION' 'LETTER' 'FINES' 'SYSTEMPREFERENCE' 'CRONJOBS', 'REPORTS' ] %] [% IF modules.grep(modx).size %] [% ELSE %] -- 2.11.0