Bugzilla – Attachment 47957 Details for
Bug 12426
Allow resending of emails from the notices tab in the patron account
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12426: Simplify the code adding a new subroutine GetMessage
Bug-12426-Simplify-the-code-adding-a-new-subroutin.patch (text/plain), 6.75 KB, created by
Marcel de Rooy
on 2016-02-12 10:32:00 UTC
(
hide
)
Description:
Bug 12426: Simplify the code adding a new subroutine GetMessage
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2016-02-12 10:32:00 UTC
Size:
6.75 KB
patch
obsolete
>From 0abfc547aa65d3e9a79d9852c47d6c50828f03a8 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 >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 <m.de.rooy@rijksmuseum.nl> >--- > 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 @@ > <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 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 <http://www.gnu.org/licenses>. > > 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
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 12426
:
42295
|
42346
|
42401
|
42409
|
42418
|
42512
|
42615
|
42618
|
42620
|
42621
|
47563
|
47564
|
47565
|
47588
|
47589
|
47590
|
47956
|
47957
|
47958
|
48049
|
48570
|
48571
|
48572
|
48573