Bugzilla – Attachment 131234 Details for
Bug 22531
Allow for multiple requests to partners and display partners in audit log
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22531: Add logging & display of req partners
Bug-22531-Add-logging--display-of-req-partners.patch (text/plain), 15.05 KB, created by
Martin Renvoize (ashimema)
on 2022-03-02 10:44:34 UTC
(
hide
)
Description:
Bug 22531: Add logging & display of req partners
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-03-02 10:44:34 UTC
Size:
15.05 KB
patch
obsolete
>From 1603882c4317ec73a12053d7e5145b96a2c6564f Mon Sep 17 00:00:00 2001 >From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> >Date: Mon, 18 Mar 2019 16:31:32 +0000 >Subject: [PATCH] Bug 22531: Add logging & display of req partners > >This patch adds: > >- The ability to send request to partners multiple times. >- Logging of requested partners >- Display of requested partners in "ILL Request Log" display > >Signed-ff-by: Barry Cannon <bc@interleaf.ie> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/Illrequest.pm | 97 ++++++++++++++++--- > Koha/Illrequest/Logger.pm | 65 ++++++++++++- > .../prog/en/modules/ill/ill-requests.tt | 10 +- > .../prog/en/modules/ill/log/status_change.tt | 16 +++ > 4 files changed, 167 insertions(+), 21 deletions(-) > >diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm >index 6d26418cd9..e61eac0da5 100644 >--- a/Koha/Illrequest.pm >+++ b/Koha/Illrequest.pm >@@ -202,11 +202,33 @@ sub status_alias { > my $current_status_alias = $self->SUPER::status_alias; > > if ($new_status_alias) { >+ # $new_status_alias may be one of two things, it may be a simple string >+ # thereby just a drop in replacement for the native status_alias >+ # method, but it may also be a hashref, as per: >+ # >+ # { status => '<new_status>', additional => {<arbitrary_hashref} } >+ # >+ # The purpose of 'additional' is to allow additional data to be >+ # sent for logging when the status change is logged, in addition to >+ # the new status. I'm not 100% happy with this solution, but since >+ # we implcitly log when $request->status_alias(123) is called, I'm not >+ # sure how else we can achieve it without explicitly calling logging >+ # each time a status is changed, which would be grim >+ # > # Keep a record of the previous status before we change it, > # we might need it > $self->{previous_status} = $current_status_alias ? > $current_status_alias : > scalar $self->status; >+ my $actual_status_alias; >+ my $additional; >+ if (ref $new_status_alias eq 'HASH') { >+ $actual_status_alias = $new_status_alias->{status}; >+ $additional = $new_status_alias->{additional} >+ if exists $new_status_alias->{additional}; >+ } else { >+ $actual_status_alias = $new_status_alias; >+ } > # 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 >@@ -214,14 +236,17 @@ sub status_alias { > # https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20581#c156 > # We need a way of accepting implied undef, so we can nullify > # the status_alias column, when called from $self->status >- my $val = $new_status_alias eq "-1" ? undef : $new_status_alias; >+ my $val = $actual_status_alias eq "-1" ? undef : $actual_status_alias; > my $ret = $self->SUPER::status_alias($val); >- my $val_to_log = $val ? $new_status_alias : scalar $self->status; >+ my $val_to_log = $val ? $actual_status_alias : scalar $self->status; > if ($ret) { > my $logger = Koha::Illrequest::Logger->new; > $logger->log_status_change({ > request => $self, >- value => $val_to_log >+ value => { >+ status => $val_to_log, >+ additional => $additional >+ } > }); > } else { > delete $self->{previous_status}; >@@ -259,17 +284,38 @@ and sends a notice if appropriate > > sub status { > my ( $self, $new_status) = @_; >- > my $current_status = $self->SUPER::status; > my $current_status_alias = $self->SUPER::status_alias; > > if ($new_status) { >+ # $new_status may be one of two things, it may be a simple string >+ # thereby just a drop in replacement for the native status method, >+ # but it may also be a hashref, as per: >+ # >+ # { status => '<new_status>', additional => {<arbitrary_hashref} } >+ # >+ # The purpose of 'additional' is to allow additional data to be >+ # sent for logging when the status change is logged, in addition to >+ # the new status. I'm not 100% happy with this solution, but since >+ # we implcitly log when $request->status('XYZ') is called, I'm not >+ # sure how else we can achieve it without explicitly calling logging >+ # each time a status is changed, which would be grim >+ # > # Keep a record of the previous status before we change it, > # we might need it > $self->{previous_status} = $current_status_alias ? > $current_status_alias : > $current_status; >- my $ret = $self->SUPER::status($new_status)->store; >+ my $actual_status; >+ my $additional; >+ if (ref $new_status eq 'HASH') { >+ $actual_status = $new_status->{status}; >+ $additional = $new_status->{additional} >+ if exists $new_status->{additional}; >+ } else { >+ $actual_status = $new_status; >+ } >+ my $ret = $self->SUPER::status($actual_status)->store; > if ($current_status_alias) { > # This is hackery to enable us to undefine > # status_alias, since we need to have an overloaded >@@ -278,12 +324,18 @@ 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"); >+ $self->status_alias({ >+ status => "-1", >+ additional => $additional >+ }); > } else { > my $logger = Koha::Illrequest::Logger->new; > $logger->log_status_change({ > request => $self, >- value => $new_status >+ value => { >+ status => $actual_status, >+ additional => $additional >+ } > }); > } > delete $self->{previous_status}; >@@ -445,12 +497,12 @@ sub _core_status_graph { > ui_method_icon => 'fa-check', > }, > GENREQ => { >- prev_actions => [ 'NEW', 'REQREV' ], >+ prev_actions => [ 'NEW', 'REQREV', 'GENREQ' ], > id => 'GENREQ', > name => 'Requested from partners', > ui_method_name => 'Place request with partners', > method => 'generic_confirm', >- next_actions => [ 'COMP', 'CHK' ], >+ next_actions => [ 'COMP', 'CHK', 'GENREQ' ], > ui_method_icon => 'fa-send-o', > }, > REQREV => { >@@ -1363,7 +1415,16 @@ sub generic_confirm { > if ($letter) { > my $result = C4::Letters::EnqueueLetter($params); > if ( $result ) { >- $self->status("GENREQ")->store; >+ # This request may previously have been sent to partners >+ # so we want to check who >+ my $previous_partners = $self->requested_partners; >+ $self->status({ >+ status => "GENREQ", >+ additional => { >+ partner_email_now => $to, >+ partner_email_previous => $previous_partners >+ } >+ })->store; > $self->_backend_capability( > 'set_requested_partners', > { >@@ -1654,16 +1715,26 @@ sub store { > my $partners_string = $illRequest->requested_partners; > > Return the string representing the email addresses of the partners to >-whom a request has been sent >+whom a request has been sent or, alternatively, the full (unblessed) >+patron objects in an arrayref > > =cut > > sub requested_partners { >- my ( $self ) = @_; >- return $self->_backend_capability( >+ my ( $self, $full ) = @_; >+ my $email = $self->_backend_capability( > 'get_requested_partners', > { request => $self } > ); >+ return $email if (!$full); >+ >+ # The string may contain multiple addressed delimited by '; ' >+ my @email_split = split(/; /, $email); >+ # Get the appropriate patron objects >+ return Koha::Patrons->search({ >+ email => { -in => \@email_split }, >+ categorycode => $self->_config->partner_code >+ })->unblessed; > } > > =head3 TO_JSON >diff --git a/Koha/Illrequest/Logger.pm b/Koha/Illrequest/Logger.pm >index 31f1704446..cf7d2b6384 100644 >--- a/Koha/Illrequest/Logger.pm >+++ b/Koha/Illrequest/Logger.pm >@@ -26,6 +26,7 @@ use C4::Templates; > use C4::Log qw( logaction ); > use Koha::ActionLogs; > use Koha::Notice::Template; >+use Koha::Patron; > > =head1 NAME > >@@ -145,19 +146,68 @@ sub log_status_change { > my ( $self, $params ) = @_; > > if (defined $params->{request} && defined $params->{value}) { >+ >+ my $infos = { >+ log_origin => 'core', >+ status_before => $params->{request}->{previous_status}, >+ status_after => $params->{value}->{status} >+ }; >+ >+ if ( >+ defined $params->{value}->{additional} && >+ $params->{value}->{additional} >+ ) { >+ $infos->{additional} = $params->{value}->{additional}; >+ } >+ > $self->log_something({ > modulename => 'ILL', > actionname => 'STATUS_CHANGE', > objectnumber => $params->{request}->id, >- infos => to_json({ >- log_origin => 'core', >- status_before => $params->{request}->{previous_status}, >- status_after => $params->{value} >- }) >+ infos => to_json($infos) > }); > } > } > >+=head3 logged_requested_partners >+ >+ Koha::Illrequest::Logger->logged_requested_partners($log); >+ >+Receive an request's log and return a hashref of all >+patron objects associated with it, keyed on email >+address. This enables us to display full information about them >+and link to their patron page >+ >+=cut >+ >+sub _logged_requested_partners { >+ my ( $params ) = @_; >+ my @to = (); >+ my @potential = ('partner_email_previous', 'partner_email_now'); >+ foreach my $partner(@potential) { >+ if ($params->{info}->{additional}->{$partner}) { >+ my @addresses = split( >+ /; /, >+ $params->{info}->{additional}->{$partner} >+ ); >+ push @to, @addresses if scalar @addresses > 0; >+ } >+ } >+ my $to_return = {}; >+ if (scalar @to > 0) { >+ my $patrons = Koha::Patrons->search({ >+ email => { -in => \@to }, >+ categorycode => $params->{request}->_config->partner_code >+ })->unblessed; >+ foreach my $patron(@{$patrons}) { >+ $to_return->{$patron->{email}} = $patron; >+ } >+ return $to_return; >+ } else { >+ return {}; >+ } >+} >+ > =head3 log_something > > Koha::IllRequest::Logger->log_something({ >@@ -261,6 +311,11 @@ sub get_request_logs { > $log->{notice_types} = $notice_hash; > $log->{aliases} = $alias_hash; > $log->{info} = from_json($log->{info}); >+ my $full_partners = _logged_requested_partners({ >+ info => $log->{info}, >+ request => $request >+ }); >+ $log->{requested_partners} = $full_partners if keys %{$full_partners}; > $log->{template} = $self->get_log_template({ > request => $request, > origin => $log->{info}->{log_origin}, >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 5029e5f858..6296670ec4 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 >@@ -637,8 +637,12 @@ > [% request.statusalias.lib | html %] > [% ELSE %] > [% request.capabilities.$req_status.name | html%] >- [% IF request.requested_partners.length > 0 %] >- ([% request.requested_partners | html %]) >+ [% IF request.status == 'GENREQ' && request.requested_partners.length > 0 %] >+ : >+ [% FOREACH partner IN request.requested_partners(1) %] >+ <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% partner.borrowernumber %]">[% partner.email %] ([% partner.cardnumber %])</a> >+ [% IF loop.index() < loop.size() - 1 %];[% END %] >+ [% END %] > [% END %] > [% END %] > </li> >@@ -722,7 +726,7 @@ > [% IF request.logs.size > 0 %] > [% FOREACH log IN request.logs %] > [% tpl = log.template %] >- [% INCLUDE $tpl log=log %] >+ [% INCLUDE $tpl log=log request=request %] > [% END %] > [% ELSE %] > There are no recorded logs for this request >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 >index 5c18443165..1bef45e991 100644 >--- 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 >@@ -5,8 +5,24 @@ > [% before = log.info.status_before %] > [% display_before = log.aliases.$before ? log.aliases.$before.lib : request.capabilities.$before.name %] > from "[% display_before | html %]" >+[% IF before == "GENREQ" && log.info.additional.partner_email_previous %] >+ ( >+ [% FOREACH add IN log.info.additional.partner_email_previous.split('; ') %] >+ [% borrowernumber = log.requested_partners.$add.borrowernumber %] >+ <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | html %]">[% add | url %]</a> >+ [% END %] >+ ) >+[% END %] > [% END %] > [% after = log.info.status_after %] > [% display_after = log.aliases.$after ? log.aliases.$after.lib : request.capabilities.$after.name %] > to "[% display_after | html %]" >+[% IF after == "GENREQ" && log.info.additional.partner_email_now %] >+ ( >+ [% FOREACH add IN log.info.additional.partner_email_now.split('; ') %] >+ [% borrowernumber = log.requested_partners.$add.borrowernumber %] >+ <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber | html %]">[% add | url %]</a> >+ [% END %] >+ ) >+[% END %] > </p> >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 22531
:
86723
|
86744
|
118843
|
118844
|
122643
|
122644
|
124979
|
124980
|
131234
|
131235
|
131236
|
131237
|
133708
|
133709
|
133710
|
133711
|
145726
|
145727
|
145728
|
145729
|
145730