Attachment 119361 Details for Bug 14233 – Bug 14233: Add logging support to notices and slips management

[patch] Bug 14233: Add logging support to notices and slips management

Bug-14233-Add-logging-support-to-notices-and-slips.patch (text/plain), 13.48 KB, created by Martin Renvoize (ashimema) on 2021-04-09 08:28:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-04-09 08:28:45 UTC

Size: 13.48 KB

patch

obsolete

>From 7a42a1d00ba09b734e56801d90f2af8a16754fc1 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Wed, 7 Apr 2021 15:02:17 +0200
>Subject: [PATCH] Bug 14233: Add logging support to notices and slips
> management
>
>This enhancements adds support for logging changes to notices and slips.
>Features include:
>- a system preference (NoticesLog) which allows you to enable logging changes for notices and slips
>- logging new notices, changes to notices, and deletion of notices
>- filtering in the log viewer so you can show all changes for a specific notice
>- a comparison feature, so you can see what changes were made to a notice.
>
>Test plan:
>1/ Add some 'Notice' action logs by adding a new notice or updateing an
>existing notice
>2/ Go to the 'Log viewer' under 'Tools'
>3/ Note you can filter by 'Notices'
>4/ Confirm the changes you made appear in the logs and that the 'Object'
>field links back to the updated notice ready for editing.
>
>1/ Make some changes to at least two different notices to trigger logs
>2/ Go to the 'Log viewer' from 'Tools' and filter on 'Notices'
>3/ Note, you can now see the 'Compare' checkbox option for these logs
>4/ Select the checkbox on one of the notices and note that the results
>are now filtered by that specific notice.
>5/ Select a second checkbox to expose the option to 'View comparison'
>6/ Click 'View comparison' to see the 'Diff view' of the two notices.
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> .../prog/en/modules/tools/viewlog.tt          | 48 +++++++++++--------
> tools/letter.pl                               | 41 ++++++++++++----
> tools/viewlog.pl                              | 10 ++++
> 3 files changed, 69 insertions(+), 30 deletions(-)
>
>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 97862f674b..a535488d26 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt
>@@ -96,6 +96,7 @@
> 
> [%        CASE 'SEARCHENGINE' %]Search engine
> 
>+[%        CASE 'NOTICES'      %]Notices[% UNLESS Koha.Preference('NoticesLog') %] <i class="fa fa-warning" title="Log not enabled"></i>[% END %]
> [%        CASE %][% module | html %]
> [%    END %]
> [% END %]
>@@ -190,7 +191,7 @@
>                                         [% ELSE %]
>                                             <label for="moduleALL" class="viewlog"><input type="checkbox" id="moduleALL" name="modules" value=""> All</label>
>                                         [% END %]
>-                                        [% FOREACH modx IN [ 'AUTH' 'CATALOGUING' 'AUTHORITIES' 'MEMBERS' 'ACQUISITIONS' 'SERIAL' 'HOLDS' 'ILL' 'CIRCULATION' 'LETTER' 'FINES' 'SYSTEMPREFERENCE' 'CRONJOBS', 'REPORTS', 'SEARCHENGINE' ] %]
>+                                        [% FOREACH modx IN [ 'AUTH' 'CATALOGUING' 'AUTHORITIES' 'MEMBERS' 'ACQUISITIONS' 'SERIAL' 'HOLDS' 'ILL' 'CIRCULATION' 'LETTER' 'FINES' 'SYSTEMPREFERENCE' 'CRONJOBS', 'REPORTS', 'SEARCHENGINE', 'NOTICES' ] %]
>                                             [% IF modules.grep(modx).size %]
>                                                 <label for="module[% modx | html %]" class="viewlog"><input type="checkbox" id="module[% modx | html %]" name="modules" value="[% modx | html %]" checked="checked"> [% PROCESS translate_log_module module=modx %]</label>
>                                             [% ELSE %]
>@@ -324,27 +325,27 @@
>                                             [% ELSE %]
>                                                 [% loopro.object | html %]
>                                             [% END %]
>+                                        [% ELSIF ( loopro.module == 'CATALOGUING' ) %]
>+                                            [% IF ( loopro.info.substr(0, 4) == 'item' ) %]
>+                                                <a href="/cgi-bin/koha/catalogue/moredetail.pl?item=[% loopro.object | uri %]&amp;biblionumber=[% loopro.biblionumber | uri %]&amp;bi=[% loopro.biblioitemnumber | uri %]#item[% loopro.object | uri %]">Item [% loopro.object | html %]</a> from
>+                                                <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loopro.biblionumber | uri %]" title="Display detail for this biblio">Biblio [% loopro.biblionumber | html %]</a>
>+                                            [% ELSIF ( loopro.info.substr(0, 6) == 'biblio' ) %]
>+                                                <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loopro.object | uri %]" title="Display detail for this biblio">Biblio [% loopro.object | html %]</a>
>+                                            [% ELSE %]
>+                                                [% loopro.object | html %]
>+                                            [% END %]
>+                                        [% ELSIF ( loopro.module == 'SERIAL' ) %]
>+                                            <a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% loopro.object | uri %]">Subscription [% loopro.object | html %] </a>
>+                                        [% ELSIF ( loopro.module == 'AUTHORITIES' ) %]
>+                                            <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% loopro.object | uri %]" title="Display detail for this authority">Authority [% loopro.object | html %]</a>
>+                                        [% ELSIF ( loopro.module == 'NOTICES' ) %]
>+                                            [% IF ( loopro.notice ) %]
>+                                                <a href="/cgi-bin/koha/tools/letter.pl?op=add_form&branchcode=[% loopro.notice.branchcode | uri %]&module=[% loopro.notice.module | uri %]&code=[% loopro.notice.code | uri %]&lang=[% loopro.notice.lang | uri %]">Notice [% loopro.notice.title | html %] ([% loopro.notice.code | html %])</a>
>+                                            [% ELSE %]
>+                                                [% loopro.object | html %]
>+                                            [% END %]
>                                         [% ELSE %]
>-                                                [% IF ( loopro.module == 'CATALOGUING' ) %]
>-                                                    [% IF ( loopro.info.substr(0, 4) == 'item' ) %]
>-                                                        <a href="/cgi-bin/koha/catalogue/moredetail.pl?item=[% loopro.object | uri %]&amp;biblionumber=[% loopro.biblionumber | uri %]&amp;bi=[% loopro.biblioitemnumber | uri %]#item[% loopro.object | uri %]">Item [% loopro.object | html %]</a> from
>-                                                        <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loopro.biblionumber | uri %]" title="Display detail for this biblio">Biblio [% loopro.biblionumber | html %]</a>
>-                                                    [% ELSIF ( loopro.info.substr(0, 6) == 'biblio' ) %]
>-                                                        <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loopro.object | uri %]" title="Display detail for this biblio">Biblio [% loopro.object | html %]</a>
>-                                                    [% ELSE %]
>-                                                        [% loopro.object | html %]
>-                                                    [% END %]
>-                                                [% ELSE %]
>-                                                    [% IF ( loopro.module == 'SERIAL' ) %]
>-                                                        <a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% loopro.object | uri %]">Subscription [% loopro.object | html %] </a>
>-                                                    [% ELSE %]
>-                                                        [% IF ( loopro.module == 'AUTHORITIES' ) %]
>-                                                            <a href="/cgi-bin/koha/authorities/detail.pl?authid=[% loopro.object | uri %]" title="Display detail for this authority">Authority [% loopro.object | html %]</a>
>-                                                        [% ELSE %]
>-                                                            [% loopro.object | html %]
>-                                                        [% END %]
>-                                                    [% END %]
>-                                                [% END %]
>+                                            [% loopro.object | html %]
>                                         [% END %]
>                                     </td>
>                                     <td>
>@@ -356,6 +357,11 @@
>                                                 <div class="compare_info" id="compare_info[% loopro.action_id | html %]">
>                                                     <label><input type="checkbox" name="diff" id="action_id[% loopro.action_id | html %]" data-actionid="[% loopro.action_id | html %]" data-filter="[% FOREACH info IN loopro.info.split(' \| ') %][% IF loop.first %][% info | html %][% END %][% END %]" class="compare" /> Compare</label>
>                                                 </div>
>+                                            [% ELSIF loopro.module == "NOTICES" %]
>+                                                <div class="loginfo" id="loginfo[% loopro.action_id | html %]">[% loopro.info | trim | html %]</div>
>+                                                <div class="compare_info" id="compare_info[% loopro.action_id | html %]">
>+                                                    <label><input type="checkbox" name="diff" id="action_id[% loopro.action_id | html %]" data-actionid="[% loopro.action_id | html %]" data-filter="[% IF loopro.notice %][% loopro.notice.title | html %][% ELSE %][% loopro.object | html %][% END %]" class="compare" /> Compare</label>
>+                                                </div>
>                                             [% ELSE %]
>                                                 <div class="loginfo" id="loginfo[% loopro.action_id | html %]">
>                                                     [% loopro.info | html %]
>diff --git a/tools/letter.pl b/tools/letter.pl
>index 20eb74771b..faf80e7d39 100755
>--- a/tools/letter.pl
>+++ b/tools/letter.pl
>@@ -46,6 +46,7 @@ use C4::Auth;
> use C4::Context;
> use C4::Output;
> use C4::Letters;
>+use C4::Log;
> 
> use Koha::Patron::Attribute::Types;
> 
>@@ -315,6 +316,10 @@ sub add_validate {
>             next;
>         }
>         elsif ( $letter and $letter->{message_transport_type} eq $mtt and $letter->{lang} eq $lang ) {
>+            logaction( 'NOTICES', 'MODIFY', $letter->{id}, $content,
>+                'Intranet' )
>+              if ( C4::Context->preference("NoticesLog")
>+                && $content ne $letter->{content} );
>             $dbh->do(
>                 q{
>                     UPDATE letter
>@@ -326,11 +331,22 @@ sub add_validate {
>                 $branchcode, $oldmodule, $code, $mtt, $lang
>             );
>         } else {
>-            $dbh->do(
>-                q{INSERT INTO letter (branchcode,module,code,name,is_html,title,content,message_transport_type, lang) VALUES (?,?,?,?,?,?,?,?,?)},
>-                undef,
>-                $branchcode || '', $module, $code, $name, $is_html || 0, $title, $content, $mtt, $lang
>-            );
>+            my $letter = Koha::Notice::Template->new(
>+                {
>+                    branchcode             => $branchcode,
>+                    module                 => $module,
>+                    code                   => $code,
>+                    name                   => $name,
>+                    is_html                => $is_html,
>+                    title                  => $title,
>+                    content                => $content,
>+                    message_transport_type => $mtt,
>+                    lang                   => $lang
>+                }
>+            )->store;
>+            logaction( 'NOTICES', 'CREATE', $letter->id, $letter->content,
>+                'Intranet' )
>+              if C4::Context->preference("NoticesLog");
>         }
>     }
>     # set up default display
>@@ -350,16 +366,23 @@ sub delete_confirm {
> }
> 
> sub delete_confirmed {
>-    my ($branchcode, $module, $code, $mtt, $lang) = @_;
>-    C4::Letters::DelLetter(
>+    my ( $branchcode, $module, $code, $mtt, $lang ) = @_;
>+    my $letters = Koha::Notice::Templates->search(
>         {
>             branchcode => $branchcode || '',
>             module     => $module,
>             code       => $code,
>-            mtt        => $mtt,
>-            lang       => $lang,
>+            ( $mtt ? ( message_transport_type => $mtt ) : () ),
>+            ( $lang ? ( lang => $lang ) : () ),
>         }
>     );
>+    while ( my $letter = $letters->next ) {
>+        logaction( 'NOTICES', 'DELETE', $letter->id, $letter->content,
>+            'Intranet' )
>+          if C4::Context->preference("NoticesLog");
>+        $letter->delete;
>+    }
>+
>     # setup default display for screen
>     default_display($branchcode);
>     return;
>diff --git a/tools/viewlog.pl b/tools/viewlog.pl
>index b58dcbacac..e67e911c40 100755
>--- a/tools/viewlog.pl
>+++ b/tools/viewlog.pl
>@@ -186,6 +186,16 @@ if ($do_it) {
>                 }
>             }
>         }
>+
>+        if ( $log->module eq 'NOTICES' ) {
>+            if ( $log->object ) {
>+                my $notice = Koha::Notice::Templates->find( { id => $log->object } );
>+                if ($notice && $output eq 'screen') {
>+                    $result->{notice} = $notice->unblessed;
>+                }
>+            }
>+        }
>+
>         push @data, $result;
>     }
>     if ( $output eq "screen" ) {
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 14233: 116649 | 116650 | 116651 | 116660 | 116661 | 116662 | 116668 | 116671 | 116672 | 116673 | 116674 | 116713 | 116760 | 116761 | 117251 | 117252 | 117253 | 117254 | 117255 | 117256 | 117257 | 117258 | 117259 | 117294 | 117295 | 117296 | 117297 | 117298 | 117299 | 117300 | 117301 | 117302 | 117303 | 117304 | 117311 | 117312 | 117313 | 117314 | 117315 | 117316 | 117317 | 117318 | 117319 | 117320 | 117321 | 117345 | 118132 | 118133 | 118134 | 118135 | 118136 | 118137 | 118138 | 118139 | 118140 | 118141 | 118142 | 118143 | 118155 | 118156 | 118157 | 118158 | 118159 | 118160 | 118161 | 118162 | 118163 | 118164 | 118165 | 118166 | 118167 | 118323 | 119291 | 119292 | 119293 | 119294 | 119312 | 119313 | 119314 | 119315 | 119359 | 119360 | 119361 | 119362 | 119510