Attachment 84821 Details for Bug 20750 – Bug 20750: Allow logging of arbitrary actions

[patch] Bug 20750: Allow logging of arbitrary actions

Bug-20750-Allow-logging-of-arbitrary-actions.patch (text/plain), 19.72 KB, created by Andrew Isherwood on 2019-02-06 14:30:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2019-02-06 14:30:53 UTC

Size: 19.72 KB

patch

obsolete

>From 1116d7bb59332f4db40070c9292a8076de5cc608 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>
>---
> Koha/Illrequest.pm                                 |  72 +++++-
> 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, 369 insertions(+), 9 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 8bedf37f42..de1a303a26 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,14 +221,19 @@ 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) {
>+    my ( $self, $new_status) = @_;
>+
>+    my $current_status = $self->SUPER::status;
>+
>+    if ($new_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
>@@ -225,10 +241,25 @@ sub status {
>         # 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);
>+        #
>+        # 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) {
>+            $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 +283,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 +1141,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 be111b6a65..470e4cb2ae 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'),
> ('ILLOpacbackends',NULL,NULL,'ILL backends to enabled for OPAC initiated requests','multiple'),
>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 4adffa4382..2e917be9da 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
>@@ -320,6 +320,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">
>@@ -430,6 +434,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>
>@@ -468,7 +496,7 @@
>                                         </form>
>                                     </div>
>                                 </div>
>-                            </div>
>+                        </div>
>                     </div>
> 
>                 [% ELSIF query_type == 'illlist' %]
>@@ -836,6 +864,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 db17762656..07e4d5e229 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 %]
>@@ -102,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 %]
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 20750: 75304 | 75326 | 75329 | 75330 | 77757 | 77816 | 77817 | 77818 | 77931 | 79822 | 79823 | 79824 | 80860 | 82082 | 82083 | 82085 | 82096 | 82097 | 82098 | 82099 | 82500 | 82501 | 82502 | 84193 | 84194 | 84195 | 84203 | 84266 | 84497 | 84498 | 84821 | 84823 | 84824 | 84825 | 84826 | 84827 | 84973 | 84974 | 84975 | 84976 | 84977 | 84978 | 84979 | 85097 | 85098 | 85099 | 85101 | 85113 | 85238 | 85266 | 85697 | 85698 | 85699 | 85700 | 85701 | 85702 | 85703 | 85704 | 85705 | 85706 | 85707 | 85708 | 85709 | 85710 | 85756 | 85757 | 85765 | 85826 | 85827 | 85829 | 85830 | 85831 | 85832 | 85833 | 85834 | 85835 | 85836 | 85837 | 85838 | 85839 | 85840 | 85841 | 85842 | 85843 | 85844 | 85845 | 85846 | 85847 | 85848 | 86434 | 86435 | 86436 | 86534 | 86535 | 86536