From 0813c72b1fe8880f319a8259e1f0b3efff80fa32 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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). Signed-off-by: Brendan A Gallagher --- C4/Letters.pm | 37 +++++++++++++++---- .../prog/en/modules/members/notices.tt | 2 +- members/notices.pl | 26 +++++-------- t/db_dependent/Letters.t | 13 +++++-- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index e953ccb242..8a49010673 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1094,11 +1094,28 @@ 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. + Attempt to resend a message which has failed previously. - my $message_id = C4::Letters::ResendMessage(123); + my $has_been_resent = C4::Letters::ResendMessage($message_id); Updates the message to 'pending' status so that it will be resent later on. @@ -1109,11 +1126,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 c8beb6c42d..d4aa4b72fb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt @@ -71,7 +71,7 @@ [% IF ( QUEUED_MESSAGE.status == 'sent' ) %]sent [% ELSIF ( QUEUED_MESSAGE.status == 'pending' ) %]pending - [% ELSIF ( QUEUED_MESSAGE.status == 'failed' ) %]failed
Resend
+ [% ELSIF ( QUEUED_MESSAGE.status == 'failed' ) %]failed
Resend
[% ELSIF ( QUEUED_MESSAGE.status == 'deleted' ) %]deleted [% ELSE %][% QUEUED_MESSAGE.status %][% END %] diff --git a/members/notices.pl b/members/notices.pl index 897da9e332..1f7cce9828 100755 --- a/members/notices.pl +++ b/members/notices.pl @@ -49,25 +49,19 @@ $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 +my $op = $input->param('op') || q{}; +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 f229726f26..abf7d883fb 100644 --- a/t/db_dependent/Letters.t +++ b/t/db_dependent/Letters.t @@ -18,7 +18,7 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 69; +use Test::More tests => 72; use Test::MockModule; use Test::Warn; @@ -138,9 +138,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.20.1