From 19c6390aa184b21f397868f6fd4dce151926526e Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 5 Jul 2017 10:33:16 -0400 Subject: [PATCH] Bug 18894 - Add ability to limit the number of messages sent by misc/cronjobs/process_message_queue.pl at a time Having the ability to limit the number of messages sent by process_message_queue.pl on a single run would be very useful for controlling home many messages are sent at a given time. This can help prevent too many messages being sent out at once and getting flagged as a spammer. Test Plan: 1) Apply this patch 2) Generate some number of messages in the message queue 3) Run process_message_queue.pl with the new --limit option, set limit to a number smaller than the number of pending messages 4) After the script has run, check the database and note that only a number of pending messages were sent, and that the remaining amount of pending messages is the original amount less the number specified as the limit --- C4/Letters.pm | 14 +++++----- misc/cronjobs/process_message_queue.pl | 13 +++++++++- t/db_dependent/Letters.t | 47 +++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/C4/Letters.pm b/C4/Letters.pm index ada2794..19055a2 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1034,7 +1034,7 @@ returns number of messages sent. sub SendQueuedMessages { my $params = shift; - my $unsent_messages = _get_unsent_messages(); + my $unsent_messages = _get_unsent_messages( { limit => $params->{limit} } ); MESSAGE: foreach my $message ( @$unsent_messages ) { # warn Data::Dumper->Dump( [ $message ], [ 'message' ] ); warn sprintf( 'sending %s message to patron: %s', @@ -1268,12 +1268,12 @@ sub _get_unsent_messages { my $params = shift; my $dbh = C4::Context->dbh(); - my $statement = << 'ENDSQL'; -SELECT mq.message_id, mq.borrowernumber, mq.subject, mq.content, mq.message_transport_type, mq.status, mq.time_queued, mq.from_address, mq.to_address, mq.content_type, b.branchcode, mq.letter_code - FROM message_queue mq - LEFT JOIN borrowers b ON b.borrowernumber = mq.borrowernumber - WHERE status = ? -ENDSQL + my $statement = qq{ + SELECT mq.message_id, mq.borrowernumber, mq.subject, mq.content, mq.message_transport_type, mq.status, mq.time_queued, mq.from_address, mq.to_address, mq.content_type, b.branchcode, mq.letter_code + FROM message_queue mq + LEFT JOIN borrowers b ON b.borrowernumber = mq.borrowernumber + WHERE status = ? + }; my @query_params = ('pending'); if ( ref $params ) { diff --git a/misc/cronjobs/process_message_queue.pl b/misc/cronjobs/process_message_queue.pl index 3325c8a..adb11bd 100755 --- a/misc/cronjobs/process_message_queue.pl +++ b/misc/cronjobs/process_message_queue.pl @@ -31,6 +31,7 @@ use Getopt::Long; my $username = undef; my $password = undef; +my $limit = undef; my $method = 'LOGIN'; my $help = 0; my $verbose = 0; @@ -38,6 +39,7 @@ my $verbose = 0; GetOptions( 'u|username:s' => \$username, 'p|password:s' => \$password, + 'l|limit:s' => \$limit, 'm|method:s' => \$method, 'h|help|?' => \$help, 'v|verbose' => \$verbose, @@ -53,6 +55,7 @@ advance_notices.pl script. This script has the following parameters : -u --username: username of mail account -p --password: password of mail account + -l --limit: The maximum number of messages to process for this run -m --method: authentication method required by SMTP server (See perldoc Sendmail.pm for supported authentication types.) -h --help: this message -v --verbose: provides verbose output to STDOUT @@ -63,5 +66,13 @@ die $usage if $help; cronlogaction(); -C4::Letters::SendQueuedMessages( { verbose => $verbose, username => $username, password => $password, method => $method } ); +C4::Letters::SendQueuedMessages( + { + verbose => $verbose, + username => $username, + password => $password, + method => $method, + limit => $limit, + } +); diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t index 7950750..73191c6 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 => 82; +use Test::More tests => 83; use Test::MockModule; use Test::Warn; @@ -109,6 +109,7 @@ $my_message->{letter} = { code => 'TEST_MESSAGE', content_type => 'text/plain', }; + $message_id = C4::Letters::EnqueueLetter($my_message); is( $message_id, undef, 'EnqueueLetter without the message type argument argument returns undef' ); @@ -628,3 +629,47 @@ subtest 'SendQueuedMessages' => sub { is( $sms_message_address, '5555555555@kidclamp.rocks', 'SendQueuedMessages populates the to address correctly for SMS by email' ); }; + +subtest 'Test limit parameter for SendQueuedMessages' => sub { + plan tests => 3; + + my $dbh = C4::Context->dbh; + + my $borrowernumber = AddMember( + firstname => 'Jane', + surname => 'Smith', + categorycode => $patron_category, + branchcode => $library->{branchcode}, + dateofbirth => $date, + smsalertnumber => undef, + ); + + $dbh->do(q|DELETE FROM message_queue|); + $my_message = { + 'letter' => { + 'content' => 'a message', + 'metadata' => 'metadata', + 'code' => 'TEST_MESSAGE', + 'content_type' => 'text/plain', + 'title' => 'message title' + }, + 'borrowernumber' => $borrowernumber, + 'to_address' => undef, + 'message_transport_type' => 'sms', + 'from_address' => 'from@example.com' + }; + C4::Letters::EnqueueLetter($my_message); + C4::Letters::EnqueueLetter($my_message); + C4::Letters::EnqueueLetter($my_message); + C4::Letters::EnqueueLetter($my_message); + C4::Letters::EnqueueLetter($my_message); + my $messages_processed = C4::Letters::SendQueuedMessages( { limit => 1 } ); + is( $messages_processed, 1, + 'Processed 1 message with limit of 1 and 5 unprocessed messages' ); + $messages_processed = C4::Letters::SendQueuedMessages( { limit => 2 } ); + is( $messages_processed, 2, + 'Processed 2 message with limit of 2 and 4 unprocessed messages' ); + $messages_processed = C4::Letters::SendQueuedMessages( { limit => 3 } ); + is( $messages_processed, 2, + 'Processed 2 message with limit of 3 and 2 unprocessed messages' ); +}; -- 2.10.2