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 &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 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