From c6b01107070e2111eda83a22993f88f03cc3585a Mon Sep 17 00:00:00 2001 From: Andrew Isherwood 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 --- Koha/Illrequest.pm | 97 +++++++++++++++++++--- Koha/Illrequest/Logger.pm | 60 +++++++++++-- .../prog/en/modules/ill/ill-requests.tt | 10 ++- .../prog/en/modules/ill/log/status_change.tt | 16 ++++ 4 files changed, 162 insertions(+), 21 deletions(-) diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm index 57e47260dd..92f22637ab 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -196,11 +196,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 => '', additional => {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 @@ -208,14 +230,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}; @@ -248,17 +273,38 @@ also nullifies status_alias and records the fact that the status has changed 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 => '', additional => {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 @@ -267,12 +313,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}; @@ -430,12 +482,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' ], + next_actions => [ 'COMP', 'GENREQ' ], ui_method_icon => 'fa-send-o', }, REQREV => { @@ -1118,7 +1170,16 @@ EOF # Send it my $result = sendmail(%mail); 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', { @@ -1217,16 +1278,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 0625370237..657744a5bb 100644 --- a/Koha/Illrequest/Logger.pm +++ b/Koha/Illrequest/Logger.pm @@ -26,6 +26,7 @@ use C4::Context; use C4::Templates; use C4::Log qw( logaction ); use Koha::ActionLogs; +use Koha::Patron; =head1 NAME @@ -115,19 +116,63 @@ 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) { + 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({ @@ -218,6 +263,11 @@ sub get_request_logs { foreach my $log(@{$logs}) { $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 a096f32015..0689d2cc13 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 @@ -363,8 +363,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) %] + [% partner.email %] ([% partner.cardnumber %]) + [% IF loop.index() < loop.size() - 1 %];[% END %] + [% END %] [% END %] [% END %] @@ -448,7 +452,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 bde7367a0b..fc6b545556 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 %] + [% add | url %] + [% 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 %] + [% add | url %] + [% END %] + ) +[% END %]

-- 2.11.0