From 0abfc547aa65d3e9a79d9852c47d6c50828f03a8 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 Content-Type: text/plain; charset=utf-8 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: Marcel de Rooy --- 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 de3322a..77deba9 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1085,11 +1085,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. @@ -1100,11 +1117,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 c8beb6c..d4aa4b7 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 + [% ELSIF ( QUEUED_MESSAGE.status == 'failed' ) %]failed [% ELSIF ( QUEUED_MESSAGE.status == 'deleted' ) %]deleted [% ELSE %][% QUEUED_MESSAGE.status %][% END %] diff --git a/members/notices.pl b/members/notices.pl index 897da9e..1f7cce9 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 1dec6ef..626e1b5 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 => 70; +use Test::More tests => 73; 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(); -- 1.7.10.4