From b457f54059312633b9295e5a2c18810e6b10df8d Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Fri, 11 May 2018 11:44:09 +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. --- C4/Log.pm | 1 + Koha/Illrequest.pm | 74 ++++++- Koha/Illrequest/Logger.pm | 242 +++++++++++++++++++++ Koha/Schema/Result/AuthorisedValue.pm | 19 +- Koha/Schema/Result/Illrequest.pm | 32 ++- Koha/Schema/Result/SocialData.pm | 7 +- .../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 | 34 +++ .../prog/en/modules/ill/log/status_change.tt | 7 + .../intranet-tmpl/prog/en/modules/tools/viewlog.tt | 5 +- t/Log.t | 1 + 15 files changed, 428 insertions(+), 10 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/C4/Log.pm b/C4/Log.pm index 0a14dfe7a8..2eec6d4bc9 100644 --- a/C4/Log.pm +++ b/C4/Log.pm @@ -142,6 +142,7 @@ sub GetLogStatus { $hash{CataloguingLog} = C4::Context->preference("CataloguingLog"); $hash{HoldsLog} = C4::Context->preference("HoldsLog"); $hash{IssueLog} = C4::Context->preference("IssueLog"); + $hash{IllLog} = C4::Context->preference("IllLog"); $hash{ReturnLog} = C4::Context->preference("ReturnLog"); $hash{SubscriptionLog} = C4::Context->preference("SubscriptionLog"); $hash{LetterLog} = C4::Context->preference("LetterLog"); diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm index 99b97bdbe4..34b5c3a842 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -30,6 +30,7 @@ use Koha::Database; use Koha::Email; use Koha::Exceptions::Ill; use Koha::Illrequestattributes; +use Koha::Illrequest::Logger; use Koha::Patron; use base qw(Koha::Object); @@ -119,6 +120,16 @@ sub illrequestattributes { ); } +=head3 logs + +=cut + +sub logs { + my ( $self ) = @_; + my $logger = Koha::Illrequest::Logger->new; + return $logger->get_request_logs($self); +} + =head3 patron =cut @@ -151,7 +162,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; } @@ -984,6 +998,64 @@ sub _censor { return $params; } +=head3 status + + $Illrequest->status('CANREQ'); + +Overloaded I method that, in addition to setting the request status +records the fact that the status has changed + +=cut + +sub status { + 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) { + my $logger = Koha::Illrequest::Logger->new; + $logger->log_status_change( + $self, + $new_status + ); + } else { + delete $self->{previous_status}; + } + return $ret; + } else { + return $current_status; + } +} + +=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/Koha/Schema/Result/AuthorisedValue.pm b/Koha/Schema/Result/AuthorisedValue.pm index 7b978eb3b8..88d003a087 100644 --- a/Koha/Schema/Result/AuthorisedValue.pm +++ b/Koha/Schema/Result/AuthorisedValue.pm @@ -129,9 +129,24 @@ __PACKAGE__->belongs_to( { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" }, ); +=head2 illrequests -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-04-26 16:13:07 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:YwvGOd/jzk71ekWfO56xrw +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "illrequests", + "Koha::Schema::Result::Illrequest", + { "foreign.status_alias" => "self.id" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + + +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2018-05-15 11:49:40 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:gDFr/88WVJNeXgUTkwFL7A # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/Koha/Schema/Result/Illrequest.pm b/Koha/Schema/Result/Illrequest.pm index 1b207ff002..76a88f1cca 100644 --- a/Koha/Schema/Result/Illrequest.pm +++ b/Koha/Schema/Result/Illrequest.pm @@ -119,6 +119,12 @@ __PACKAGE__->table("illrequests"); is_nullable: 1 size: 20 +=head2 status_alias + + data_type: 'integer' + is_foreign_key: 1 + is_nullable: 1 + =cut __PACKAGE__->add_columns( @@ -164,6 +170,8 @@ __PACKAGE__->add_columns( { data_type => "varchar", is_nullable => 1, size => 50 }, "backend", { data_type => "varchar", is_nullable => 1, size => 20 }, + "status_alias", + { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, ); =head1 PRIMARY KEY @@ -230,9 +238,29 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 status_alias + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "status_alias", + "Koha::Schema::Result::AuthorisedValue", + { id => "status_alias" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "SET NULL", + on_update => "RESTRICT", + }, +); + -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:53 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Rh8DSs3xj3KRmyd7WNGDAg +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2018-05-15 11:49:40 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:mHvzZfqlGJj3JbId6qBAAw # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/Koha/Schema/Result/SocialData.pm b/Koha/Schema/Result/SocialData.pm index b21e5088e4..40b52e0380 100644 --- a/Koha/Schema/Result/SocialData.pm +++ b/Koha/Schema/Result/SocialData.pm @@ -26,7 +26,6 @@ __PACKAGE__->table("social_data"); =head2 isbn data_type: 'varchar' - default_value: (empty string) is_nullable: 0 size: 30 @@ -65,7 +64,7 @@ __PACKAGE__->table("social_data"); __PACKAGE__->add_columns( "isbn", - { data_type => "varchar", default_value => "", is_nullable => 0, size => 30 }, + { data_type => "varchar", is_nullable => 0, size => 30 }, "num_critics", { data_type => "integer", is_nullable => 1 }, "num_critics_pro", @@ -93,8 +92,8 @@ __PACKAGE__->add_columns( __PACKAGE__->set_primary_key("isbn"); -# Created by DBIx::Class::Schema::Loader v0.07025 @ 2013-10-14 20:56:21 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Medc/ZzPVp5Eq+adFoqQoA +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2018-05-15 11:49:40 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ga2vc7qZoIWdh8G3f7nMCA # You can replace this text with custom content, and it will be preserved on regeneration 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 9f90163f89..1707cc47cb 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -203,6 +203,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 cb896c61a5..fe1631832d 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 70f31cad16..991841a2f1 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 @@ -193,6 +193,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(); @@ -496,6 +502,10 @@ [% END %] [% END %] + + + Display request log +
@@ -577,6 +587,30 @@
+ + [% ELSIF query_type == 'illlist' %]

View ILL requests

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 b183dcac28..c77ea30130 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ -27,6 +27,7 @@ [% CASE 'ACQUISITIONS' %]Acquisitions [% CASE 'SERIAL' %]Serials [% CASE 'HOLDS' %]Holds +[% CASE 'ILL' %]Interlibrary loans [% CASE 'CIRCULATION' %]Circulation [% CASE 'LETTER' %]Letter [% CASE 'FINES' %]Fines @@ -52,6 +53,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 %] [% END %] @@ -101,7 +104,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 %] diff --git a/t/Log.t b/t/Log.t index 601b909974..14a5673d9b 100755 --- a/t/Log.t +++ b/t/Log.t @@ -16,6 +16,7 @@ BEGIN { t::lib::Mocks::mock_preference('BorrowersLog', 1); t::lib::Mocks::mock_preference('CataloguingLog', 1); t::lib::Mocks::mock_preference('IssueLog', 1); +t::lib::Mocks::mock_preference('IllLog', 1); t::lib::Mocks::mock_preference('ReturnLog', 1); t::lib::Mocks::mock_preference('SubscriptionLog', 1); t::lib::Mocks::mock_preference('LetterLog', 1); -- 2.11.0