@@ -, +, @@ - Apply patch - Ensure the Illlog logging preference is turned on - Create an ILL request and perform actions on it that change it's - 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. - Apply patch - Ensure the Illlog logging preference is turned on - Ensure you have some custom request statuses defined in the - Create an ILL request and perform actions on it that change it's - Edit the request and change status to a custom one - 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. --- Koha/Illrequest.pm | 124 +++++++-- Koha/Illrequest/Logger.pm | 237 ++++++++++++++++++ .../bug_20750-add_illlog_preference.perl | 7 + installer/data/mysql/sysprefs.sql | 1 + .../en/modules/admin/preferences/logs.pref | 6 + .../prog/en/modules/ill/ill-requests.tt | 36 ++- .../prog/en/modules/ill/log/status_change.tt | 11 + .../prog/en/modules/tools/viewlog.tt | 4 +- t/db_dependent/Illrequests.t | 4 +- 9 files changed, 405 insertions(+), 25 deletions(-) create mode 100644 Koha/Illrequest/Logger.pm create mode 100644 installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt --- a/Koha/Illrequest.pm +++ a/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 @@ -169,15 +180,26 @@ sub patron { } =head3 status_alias + + $Illrequest->status_alias(143); + Overloaded getter/setter for status_alias, that only returns authorised values from the -correct category +correct category and records the fact that the status has changed =cut sub status_alias { - my ($self, $newval) = @_; - if ($newval) { + my ($self, $new_status_alias) = @_; + + my $current_status_alias = $self->SUPER::status_alias; + + if ($new_status_alias) { + # Keep a record of the previous status before we change it, + # we might need it + $self->{previous_status} = $current_status_alias ? + $current_status_alias : + scalar $self->status; # 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 @@ -185,13 +207,19 @@ sub status_alias { # https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c156 # We need a way of accepting implied undef, so we can nullify # the status_alias column, when called from $self->status - my $val = $newval eq "-1" ? undef : $newval; - my $newval = $self->SUPER::status_alias($val); - if ($newval) { - return $newval; + my $val = $new_status_alias eq "-1" ? undef : $new_status_alias; + my $ret = $self->SUPER::status_alias($val); + my $val_to_log = $val ? $new_status_alias : scalar $self->status; + if ($ret) { + my $logger = Koha::Illrequest::Logger->new; + $logger->log_status_change({ + request => $self, + value => $val_to_log + }); } else { - return; + delete $self->{previous_status}; } + return $ret; } # We can't know which result is the right one if there are multiple # ILLSTATUS authorised values with the same authorised_value column value @@ -210,25 +238,47 @@ 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; + my $current_status_alias = $self->SUPER::status_alias; + + if ($new_status) { + # Keep a record of the previous status before we change it, + # we might need it + $self->{previous_status} = $current_status_alias ? + $current_status_alias : + $current_status; + my $ret = $self->SUPER::status($new_status)->store; + if ($current_status_alias) { + # 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"); + } else { + my $logger = Koha::Illrequest::Logger->new; + $logger->log_status_change({ + request => $self, + value => $new_status + }); + } + delete $self->{previous_status}; + return $ret; + } else { + return $current_status; } - return $self->SUPER::status; } =head3 load_backend @@ -252,7 +302,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 +1160,33 @@ 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({ + request => $self, + attrs => $attrs + }); + } + + return $ret; +} + =head3 TO_JSON $json = $illrequest->TO_JSON --- a/Koha/Illrequest/Logger.pm +++ a/Koha/Illrequest/Logger.pm @@ -0,0 +1,237 @@ +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 Time::Local; + +use C4::Koha; +use C4::Context; +use C4::Templates; +use C4::Log qw( logaction ); +use Koha::ActionLogs; + +=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. +We also set up 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->{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($params); + +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, $params) = @_; + + if (defined $params->{request} && defined $params->{attrs}) { + foreach my $key (keys %{ $params->{attrs} }) { + if (defined($self->{loggers}->{$key})) { + $self->{loggers}->{$key}( + $params->{request}, + $params->{attrs}->{$key} + ); + } + } + } +} + +=head3 log_status_change + + Koha::IllRequest::Logger->log_status_change($params); + +Receive a hashref containing a request object and a status to log, +and log it + +=cut + +sub log_status_change { + 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 + + 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 passed, log an action + +=cut + +sub log_something { + my ( $self, $to_log ) = @_; + + if ( + defined $to_log->{modulename} && + defined $to_log->{actionname} && + defined $to_log->{objectnumber} && + defined $to_log->{infos} && + C4::Context->preference("IllLog") + ) { + logaction( + $to_log->{modulename}, + $to_log->{actionname}, + $to_log->{objectnumber}, + $to_log->{infos} + ); + } +} + +=head3 get_log_template + + $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, $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 =$params->{request}->{_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 = Koha::ActionLogs->search( + { + module => 'ILL', + object => $request->id + }, + { order_by => { -desc => "timestamp" } } + )->unblessed; + + # Populate a lookup table for status aliases + my $aliases = GetAuthorisedValues('ILLSTATUS'); + my $alias_hash; + foreach my $alias(@{$aliases}) { + $alias_hash->{$alias->{authorised_value}} = $alias; + } + foreach my $log(@{$logs}) { + $log->{aliases} = $alias_hash; + $log->{info} = from_json($log->{info}); + $log->{template} = $self->get_log_template({ + request => $request, + origin => $log->{info}->{log_origin}, + action => $log->{action} + }); + } + + return $logs; +} + +=head1 AUTHOR + +Andrew Isherwood + +=cut + +1; --- a/installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl +++ a/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', 0, '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"; +} --- a/installer/data/mysql/sysprefs.sql +++ a/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', 0, '', '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'), ('ILLOpacbackends',NULL,NULL,'ILL backends to enabled for OPAC initiated requests','multiple'), --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref @@ -36,6 +36,12 @@ Logging: on: Log 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: --- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt @@ -319,6 +319,10 @@ Display supplier metadata + + + ILL 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(); --- a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt @@ -0,0 +1,11 @@ +

+[% log.timestamp | $KohaDates with_hours => 1 %] : Status changed +[% IF log.info.status_before %] +[% before = log.info.status_before %] +[% display_before = log.aliases.$before ? log.aliases.$before.lib : request.capabilities.$before.name %] +from "[% display_before | html %]" +[% END %] +[% after = log.info.status_after %] +[% display_after = log.aliases.$after ? log.aliases.$after.lib : request.capabilities.$after.name %] +to "[% display_after | html %]" +

--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ -29,6 +29,7 @@ [% CASE 'ACQUISITIONS' %]Acquisitions [% CASE 'SERIAL' %]Serials [% CASE 'HOLDS' %]Holds +[% CASE 'ILL' %]Interlibrary loans [% CASE 'CIRCULATION' %]Circulation [% CASE 'LETTER' %]Letter [% CASE 'FINES' %]Fines @@ -54,6 +55,7 @@ [% CASE 'CHANGE PASS' %]Change password [% CASE 'ADDCIRCMESSAGE' %]Add circulation message [% CASE 'DELCIRCMESSAGE' %]Delete circulation message +[% CASE 'STATUS_CHANGE' %]Change ILL request status [% CASE 'Run' %]Run [% CASE %][% action | html %] [% END %] @@ -103,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 %] --- a/t/db_dependent/Illrequests.t +++ a/t/db_dependent/Illrequests.t @@ -39,7 +39,7 @@ use_ok('Koha::Illrequests'); subtest 'Basic object tests' => sub { - plan tests => 24; + plan tests => 25; $schema->storage->txn_begin; @@ -62,6 +62,8 @@ subtest 'Basic object tests' => sub { "Branchcode getter works."); is($illrq_obj->status, $illrq->{status}, "Status getter works."); + is($illrq_obj->status_alias, $illrq->{status_alias}, + "Status_alias getter works."); is($illrq_obj->placed, $illrq->{placed}, "Placed getter works."); is($illrq_obj->replied, $illrq->{replied}, --