From b7510c59f5bbbeafca4bcf664edfa71db57c6c1c Mon Sep 17 00:00:00 2001 From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Mon, 14 Sep 2015 11:35:56 +0100 Subject: [PATCH] Bug 12426: Simplify the code adding a new subroutine GetMessage The C4::Letters module does not have a GetMessage subroutine, which could be quite useful. This patch adds it and simplifies the code added by the previous patch. It also adds a few tests and fixes POD typos. Note that ResendNotice only resends failed messages. This will avoid to resend already sent messages (using an url from the browser history for instance). --- C4/Letters.pm | 41 +++++++++++++++++----- .../prog/en/modules/members/notices.tt | 2 +- members/notices.pl | 27 ++++++-------- t/db_dependent/Letters.t | 13 ++++--- 4 files changed, 53 insertions(+), 30 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index 7929506..f0d7aa3 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1060,12 +1060,29 @@ sub GetMessageTransportTypes { return $mtts; } +=head2 GetMessage + + my $message = C4::Letters::Message($message_id); + +=cut + +sub GetMessage { + my ( $message_id ) = @_; + return unless $message_id; + my $dbh = C4::Context->dbh; + return $dbh->selectrow_hashref(q| + SELECT message_id, borrowernumber, subject, content, metadata, letter_code, message_transport_type, status, time_queued, to_address, from_address, content_type + FROM message_queue + WHERE message_id = ? + |, {}, $message_id ); +} + =head2 ResendMessage - Attempt to resend a message. - - my $message_id = C4::Letters::ResendMessage(123); - + Attempt to resend a message which has failed previously. + + my $has_been_resent = C4::Letters::ResendMessage($message_id); + Updates the message to 'pending' status so that it will be resent later on. @@ -1075,11 +1092,17 @@ sub GetMessageTransportTypes { sub ResendMessage { my $message_id = shift; - - return ((C4::Letters::_set_message_status( { - message_id => $message_id, - status => 'pending', - } ) > 0) ? 1:0); + return unless $message_id; + + my $message = GetMessage( $message_id ); + return unless $message; + if ( $message->{status} eq 'failed' ) { + return ((C4::Letters::_set_message_status( { + message_id => $message_id, + status => 'pending', + } ) > 0) ? 1:0); + } + return 0; } =head2 _add_attachements diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt index d46438e..189209c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt @@ -70,7 +70,7 @@ <td> [% IF ( QUEUED_MESSAGE.status == 'sent' ) %]sent [% ELSIF ( QUEUED_MESSAGE.status == 'pending' ) %]pending - [% ELSIF ( QUEUED_MESSAGE.status == 'failed' ) %]failed <div class="notice"><a href="/cgi-bin/koha/members/notices.pl?borrowernumber=[% borrowernumber %]&resendnotice=[% QUEUED_MESSAGE.message_id %]" title="Attempt to resend the notice">Resend</a></div> + [% ELSIF ( QUEUED_MESSAGE.status == 'failed' ) %]failed <div class="notice"><a href="/cgi-bin/koha/members/notices.pl?borrowernumber=[% borrowernumber %]&op=resend_notice&message_id=[% QUEUED_MESSAGE.message_id %]" title="Attempt to resend the notice">Resend</a></div> [% ELSIF ( QUEUED_MESSAGE.status == 'deleted' ) %]deleted [% ELSE %][% QUEUED_MESSAGE.status %][% END %] </td> diff --git a/members/notices.pl b/members/notices.pl index dc021ed..2fdbe88 100755 --- a/members/notices.pl +++ b/members/notices.pl @@ -46,29 +46,24 @@ my ($template, $loggedinuser, $cookie) debug => 1, }); +my $op = $input->param('op') || q||; + $template->param( $borrower ); my ($picture, $dberror) = GetPatronImage($borrower->{'borrowernumber'}); $template->param( picture => 1 ) if $picture; -# Getting the messages -my $queued_messages = C4::Letters::GetQueuedMessages({borrowernumber => $borrowernumber}); - -# Bug 12426 - Allow resending of messages in Notices tab -if ($input->param('resendnotice')) { - foreach my $message (@$queued_messages){ - # resendnotice must be in this borrower's queue - we don't want to make it - # possible to change any message just by changing resendnotice id. - if ($message->{message_id} == $input->param('resendnotice')) { - # We also only want to resend messages in failed status - last unless $message->{status} eq "failed"; - - # Modify the message in $queued_message to have its new pending status - $message->{status} = 'pending' if (C4::Letters::ResendMessage($message->{message_id})); - last; - } +# Allow resending of messages in Notices tab +if ( $op eq 'resend_notice' ) { + my $message_id = $input->param('message_id'); + my $message = C4::Letters::GetMessage( $message_id ); + if ( $message->{borrowernumber} = $borrowernumber ) { + C4::Letters::ResendMessage( $message_id ); } } +# Getting the messages +my $queued_messages = C4::Letters::GetQueuedMessages({borrowernumber => $borrowernumber}); + if (C4::Context->preference('ExtendedPatronAttributes')) { my $attributes = GetBorrowerAttributes($borrowernumber); $template->param( diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 49b1320..78cb458 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -18,7 +18,7 @@ # along with Koha; if not, see <http://www.gnu.org/licenses>. use Modern::Perl; -use Test::More tests => 66; +use Test::More tests => 69; use Test::MockModule; use Test::Warn; @@ -137,9 +137,14 @@ is( ); # ResendMessage -C4::Letters::ResendMessage($messages->[0]->{message_id}); -$messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber }); -is($messages->[0]->{status},'pending', 'ResendMessage sets status to pending correctly (bug 12426)'); +my $resent = C4::Letters::ResendMessage($messages->[0]->{message_id}); +my $message = C4::Letters::GetMessage( $messages->[0]->{message_id}); +is( $resent, 1, 'The message should have been resent' ); +is($message->{status},'pending', 'ResendMessage sets status to pending correctly (bug 12426)'); +$resent = C4::Letters::ResendMessage($messages->[0]->{message_id}); +is( $resent, 0, 'The message should not have been resent again' ); +$resent = C4::Letters::ResendMessage(); +is( $resent, undef, 'ResendMessage should return undef if not message_id given' ); # GetLetters my $letters = C4::Letters::GetLetters(); -- 2.1.0