From 2d0cdc8f13cc13c93df488df88dd44e90811b8a1 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.
---
 C4/Log.pm                                          |   1 +
 Koha/Illrequest.pm                                 |  74 ++++++-
 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            |  39 ++++
 .../prog/en/modules/ill/log/status_change.tt       |   7 +
 .../intranet-tmpl/prog/en/modules/tools/viewlog.tt |   5 +-
 11 files changed, 382 insertions(+), 2 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<status> 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<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 a08ff3e5c8..73272a3a84 100644
--- a/installer/data/mysql/sysprefs.sql
+++ b/installer/data/mysql/sysprefs.sql
@@ -208,6 +208,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 b188945d12..0b88dc0bf2 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
@@ -194,6 +194,18 @@
             }
         };
 
+        // 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();
+            $('#requestattributes').toggleClass('content_hidden');
+        });
+
         // Filter partner list
         $('#partner_filter').keyup(function() {
             var needle = $('#partner_filter').val();
@@ -498,6 +510,9 @@
                         <a title="Display supplier metadata" id="ill-request-display-metadata" class="btn btn-sm btn-default pull-right" href="#">
                             <span class="fa fa-eye"></span>
                             Display supplier metadata
+                        <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 id="ill-view-panel" class="panel panel-default">
@@ -592,6 +607,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>
+
                 [% ELSIF query_type == 'illlist' %]
                     <!-- illlist -->
                     <h1>View ILL requests</h1>
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 &quot;[% request.capabilities(log.info.status_before).name %]&quot;
+[% END %]
+to &quot;[% request.capabilities(log.info.status_after).name %]&quot;
+</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 65569ffbc5..f7abb3735f 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 %]
@@ -101,7 +104,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