From 58005f03ed69149020fa447cf5c6744bedef2aac Mon Sep 17 00:00:00 2001
From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
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 <Niamh.Walker-Headon@it-tallaght.ie>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
---
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<store> 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 <andrew.isherwood@ptfs-europe.com>
+
+=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 @@
<span class="fa fa-eye"></span>
Display supplier metadata
</a>
+ <a title="Display request log" id="ill-request-display-log" class="btn btn-sm btn-default pull-right" href="#">
+ <span class="fa fa-calendar"></span>
+ Display request log
+ </a>
</div>
<div class="ill-view-panel panel panel-default">
<div class="panel-heading">
@@ -429,6 +433,30 @@
</div>
</div>
+ <div id="requestLog" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="dataPreviewLabel" aria-hidden="true">
+ <div class="modal-dialog">
+ <div class="modal-content">
+ <div class="modal-header">
+ <button type="button" class="closebtn" data-dismiss="modal" aria-hidden="true">×</button>
+ <h3 id="requestLogLabel"> Request log</h3>
+ </div>
+ <div class="modal-body">
+ [% IF request.logs.size > 0 %]
+ [% FOREACH log IN request.logs %]
+ [% tpl = log.template %]
+ [% INCLUDE $tpl %]
+ [% END %]
+ [% ELSE %]
+ There are no recorded logs for this request
+ [% END %]
+ </div>
+ <div class="modal-footer">
+ <button class="btn btn-default" data-dismiss="modal" aria-hidden="true">Close</button>
+ </div>
+ </div>
+ </div>
+ </div>
+
<div class="ill-view-panel panel panel-default">
<div class="panel-heading">
<h3>[% request.illcomments.count | html %] comments</h3>
@@ -467,7 +495,7 @@
</form>
</div>
</div>
- </div>
+ </div>
</div>
[% 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 @@
+<p>
+[% log.timestamp | $KohaDates with_hours => 1 %] : <b>Status changed </b>
+[% IF log.info.status_before %]
+from "[% request.capabilities(log.info.status_before).name %]"
+[% END %]
+to "[% request.capabilities(log.info.status_after).name %]"
+</p>
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 b3e11059b5..f7f1ba667f 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
+++ b/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,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 %]
@@ -103,7 +106,7 @@
[% ELSE %]
<option value="">All</option>
[% 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 %]
<option value="[% modx | html %]" selected="selected">[% PROCESS translate_log_module module=modx %]</option>
[% ELSE %]
--
2.11.0