gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>

Since this bug now is dependent on Bug 20581, it should be aware of the
custom statuses provided in 20581. This patch enables logging of request
statuses being changed to custom ones. As such the test plan is
modified:

To test:
- Apply patch
- Ensure the Illlog logging preference is turned on
- Ensure you have some custom request statuses defined in the
Authorised Values ILLSTATUS category
- Create an ILL request and perform actions on it that change it's
status.
- 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.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Andrew Isherwood 2018-10-02 14:55:58 +01:00 committed by Nick Clemens
parent a13dff4b88
commit 79d64a37a0
9 changed files with 405 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

124
Koha/Illrequest.pm
View File

@ -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<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({
request => $self,
attrs => $attrs
});
}
return $ret;
}
=head3 TO_JSON
$json = $illrequest->TO_JSON

237
Koha/Illrequest/Logger.pm Normal file
View File

@ -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 <andrew.isherwood@ptfs-europe.com>
=cut
1;

7
installer/data/mysql/atomicupdate/bug_20750-add_illlog_preference.perl Normal file
View File

@ -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";
}

1
installer/data/mysql/sysprefs.sql
View File

@ -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'),

6
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/logs.pref
View File

@ -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:

36
koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
View File

@ -319,6 +319,10 @@
<span class="fa fa-eye"></span>
Display supplier metadata
</a>
<a title="ILL request log" id="ill-request-display-log" class="btn btn-sm btn-default pull-right" href="#">
<span class="fa fa-calendar"></span>
ILL 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 log=log %]
[% 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();

11
koha-tmpl/intranet-tmpl/prog/en/modules/ill/log/status_change.tt Normal file
View File

@ -0,0 +1,11 @@
<p>
[% log.timestamp | $KohaDates with_hours => 1 %] : <b>Status changed</b>
[% IF log.info.status_before %]
[% before = log.info.status_before %]
[% display_before = log.aliases.$before ? log.aliases.$before.lib : request.capabilities.$before.name %]
from &quot;[% display_before | html %]&quot;
[% END %]
[% after = log.info.status_after %]
[% display_after = log.aliases.$after ? log.aliases.$after.lib : request.capabilities.$after.name %]
to &quot;[% display_after | html %]&quot;
</p>

4
koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
View File

@ -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 %]
<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 %]

4
t/db_dependent/Illrequests.t
View File

@ -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},