Bugzilla – Attachment 73751 Details for
Bug 18725
process_message_queue.pl sends duplicate emails if message_queue is not writable
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18725: (QA follow-up) Restore status in Letters, adjust test
Bug-18725-QA-follow-up-Restore-status-in-Letters-a.patch (text/plain), 3.96 KB, created by
Marcel de Rooy
on 2018-04-06 09:17:13 UTC
(
hide
)
Description:
Bug 18725: (QA follow-up) Restore status in Letters, adjust test
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2018-04-06 09:17:13 UTC
Size:
3.96 KB
patch
obsolete
>From bcffcaffac1cbd51ec4e1ff3783643973b6ca365 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 6 Apr 2018 09:35:26 +0200 >Subject: [PATCH] Bug 18725: (QA follow-up) Restore status in Letters, adjust > test >Content-Type: text/plain; charset=utf-8 > >We need to restore the pending status. Otherwise we will loose some >messages. (Just look at the code.) > >Correct number of tests, do not rely on number processed. > >The return of SendQueuedMessage is unreliable (just look again at >the code). Viewing this as the number of processed/sent messages seems >a bit useless. It merely serves as an indication now if we started >processing. > >Test plan: >Run Letters.t again. > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Letters.pm | 4 +++- > t/db_dependent/Letters.t | 25 +++++++++++++++++-------- > 2 files changed, 20 insertions(+), 9 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 6ec0608..f67a303 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -1061,9 +1061,11 @@ sub SendQueuedMessages { > my $unsent_messages = _get_unsent_messages( $which_unsent_messages ); > MESSAGE: foreach my $message ( @$unsent_messages ) { > my $message_object = Koha::Notice::Messages->find( $message->{message_id} ); >- $message_object->status('processing'); >+ > # If this fails the database is unwritable and we won't manage to send a message that continues to be marked 'pending' >+ $message_object->status('processing'); # enum: same as empty string > return unless $message_object->store(); >+ $message_object->status('pending')->store; # We need to restore the original status in order to prevent possible loss, but this implicitly assumes that the second store worked too. > > # warn Data::Dumper->Dump( [ $message ], [ 'message' ] ); > warn sprintf( 'sending %s message to patron: %s', >diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t >index 8ceddfb..1215c8a 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 => 77; >+use Test::More tests => 78; > use Test::MockModule; > use Test::Warn; > >@@ -48,6 +48,8 @@ use Koha::Acquisition::Bookseller::Contacts; > use Koha::Acquisition::Orders; > use Koha::Libraries; > use Koha::Notice::Templates; >+use Koha::Notice::Messages; >+ > my $schema = Koha::Database->schema; > $schema->storage->txn_begin(); > >@@ -753,7 +755,7 @@ subtest 'Test limit parameter for SendQueuedMessages' => sub { > }; > > subtest 'Test canary call for SendQueuedMessages' => sub { >- plan tests => 2; >+ plan tests => 4; > > my $letter = { > 'letter' => { >@@ -768,14 +770,21 @@ subtest 'Test canary call for SendQueuedMessages' => sub { > 'message_transport_type' => 'email', > 'from_address' => 'from@example.com' > }; >- C4::Letters::EnqueueLetter($letter); >+ my $queued_msg = Koha::Notice::Messages->find( >+ C4::Letters::EnqueueLetter($letter), >+ ); >+ is( $queued_msg->status, 'pending', 'Status should be pending now' ); > > my $mocked_storing = Test::MockModule->new('Koha::Object'); > $mocked_storing->mock('store' => sub { return; }); >- $messages_processed = C4::Letters::SendQueuedMessages(); >- is( $messages_processed, undef, 'Bad store should prevent sending any.'); >+ is( C4::Letters::SendQueuedMessages(), undef, 'Bad store should prevent handling any messages.' ); >+ $queued_msg->discard_changes; # refresh >+ is( $queued_msg->status, 'pending', 'Status still pending' ); >+ > $mocked_storing->unmock('store'); >- $messages_processed = C4::Letters::SendQueuedMessages(); >- is( $messages_processed, 1, 'Good store should send.'); >+ C4::Letters::SendQueuedMessages(); >+ $queued_msg->discard_changes; # refresh again >+ is( $queued_msg->status, 'failed', 'Good store and mocked sendmail should trigger status change.' ); >+}; > >-} >+$schema->storage->txn_rollback; >-- >2.1.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 18725
:
64803
|
65625
|
68602
|
68603
|
70805
|
70826
|
70853
|
70876
|
73277
|
73278
|
73749
|
73750
|
73751
|
74143
|
74247