Attachment 149666 Details for Bug 33360 – Bug 33360: Adjust Letters.t for limit parameter and domain limits

[patch] Bug 33360: Adjust Letters.t for limit parameter and domain limits

Bug-33360-Adjust-Letterst-for-limit-parameter-and-.patch (text/plain), 10.01 KB, created by Kyle M Hall (khall) on 2023-04-14 11:00:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2023-04-14 11:00:04 UTC

Size: 10.01 KB

patch

obsolete

>From 846c9ee772cf4547a58f4a29c8c17cbbf10175d4 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Tue, 4 Apr 2023 16:10:21 +0200
>Subject: [PATCH] Bug 33360: Adjust Letters.t for limit parameter and domain
> limits
>
>Includes:
>[1] Add a counter in the send_or_die mock.
>[2] Correct from processed count to sent count.
>[3] More extensive testing for limit parameter and domain limits.
>
>Test plan:
>Run t/db_dependent/Letters.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> t/db_dependent/Letters.t | 111 +++++++++++++++++++++++++++++----------
> 1 file changed, 84 insertions(+), 27 deletions(-)
>
>diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
>index 33a518fc096..2f52882606d 100755
>--- a/t/db_dependent/Letters.t
>+++ b/t/db_dependent/Letters.t
>@@ -30,6 +30,7 @@ use MARC::Record;
> use utf8;
> 
> my ( $email_object, $sendmail_params );
>+our $send_or_die_count = 0;
> 
> my $email_sender_module = Test::MockModule->new('Email::Stuffer');
> $email_sender_module->mock(
>@@ -38,6 +39,7 @@ $email_sender_module->mock(
>         ( $email_object, $sendmail_params ) = @_;
>         my $str = $email_object->email->as_string;
>         unlike $str, qr/I =C3=A2=C2=99=C2=A5 Koha=/, "Content is not double encoded";
>+        $send_or_die_count++;
>         warn "Fake send_or_die";
>     }
> );
>@@ -55,9 +57,12 @@ use Koha::Acquisition::Booksellers;
> use Koha::Acquisition::Bookseller::Contacts;
> use Koha::Acquisition::Orders;
> use Koha::Libraries;
>+use Koha::Notice::Messages;
> use Koha::Notice::Templates;
>+use Koha::Notice::Util;
> use Koha::Patrons;
> use Koha::Subscriptions;
>+
> my $schema = Koha::Database->schema;
> $schema->storage->txn_begin();
> 
>@@ -156,7 +161,7 @@ Koha::Notice::Messages->find($messages->[0]->{message_id})->time_queued($yesterd
> my $messages_processed = C4::Letters::SendQueuedMessages( { type => 'email' });
> is($messages_processed, 0, 'No queued messages processed if type limit passed with unused type');
> $messages_processed = C4::Letters::SendQueuedMessages( { type => 'sms' });
>-is($messages_processed, 1, 'All queued messages processed, found correct number of messages with type limit');
>+is($messages_processed, 0, 'All queued messages processed, nothing sent');
> $messages = C4::Letters::GetQueuedMessages({ borrowernumber => $borrowernumber });
> is(
>     $messages->[0]->{status},
>@@ -958,7 +963,7 @@ EOF
> };
> 
> subtest 'Test where parameter for SendQueuedMessages' => sub {
>-    plan tests => 1;
>+    plan tests => 3;
> 
>     my $dbh = C4::Context->dbh;
> 
>@@ -968,7 +973,6 @@ subtest 'Test where parameter for SendQueuedMessages' => sub {
>         categorycode => $patron_category,
>         branchcode   => $library->{branchcode},
>         dateofbirth  => $date,
>-        smsalertnumber => undef,
>     })->store->borrowernumber;
> 
>     $dbh->do(q|DELETE FROM message_queue|);
>@@ -1011,15 +1015,23 @@ subtest 'Test where parameter for SendQueuedMessages' => sub {
>         'message_transport_type' => 'sms',
>         'from_address'           => 'from@example.com'
>     };
>-    C4::Letters::EnqueueLetter($my_message);
>-    C4::Letters::EnqueueLetter($my_message2);
>-    C4::Letters::EnqueueLetter($my_message3);
>-    my $messages_processed = C4::Letters::SendQueuedMessages( { where => q{content NOT LIKE '%skipped%'} } );
>-    is( $messages_processed, 2, "Correctly skipped processing one message containing the work 'skipped' in contents" );
>+    my @id = ( C4::Letters::EnqueueLetter($my_message),
>+        C4::Letters::EnqueueLetter($my_message2),
>+        C4::Letters::EnqueueLetter($my_message3),
>+    );
>+    C4::Letters::SendQueuedMessages({
>+        # Test scalar/arrayref in parameter too
>+        letter_code => [ 'TEST_MESSAGE', 'SOMETHING_NOT_THERE' ],
>+        type => 'sms',
>+        where => q{content NOT LIKE '%skipped%'},
>+    });
>+    is( Koha::Notice::Messages->find( $id[0] )->status, 'failed', 'Processed but failed' );
>+    is( Koha::Notice::Messages->find( $id[1] )->status, 'failed', 'Processed but failed' );
>+    is( Koha::Notice::Messages->find( $id[2] )->status, 'pending', 'Skipped, still pending' );
> };
> 
> subtest 'Test limit parameter for SendQueuedMessages' => sub {
>-    plan tests => 3;
>+    plan tests => 18;
> 
>     my $dbh = C4::Context->dbh;
> 
>@@ -1029,7 +1041,7 @@ subtest 'Test limit parameter for SendQueuedMessages' => sub {
>         categorycode => $patron_category,
>         branchcode   => $library->{branchcode},
>         dateofbirth  => $date,
>-        smsalertnumber => undef,
>+        email          => 'shouldnotwork@wrong.net',
>     })->store->borrowernumber;
> 
>     $dbh->do(q|DELETE FROM message_queue|);
>@@ -1043,28 +1055,71 @@ subtest 'Test limit parameter for SendQueuedMessages' => sub {
>         },
>         'borrowernumber'         => $borrowernumber,
>         'to_address'             => undef,
>-        'message_transport_type' => 'sms',
>+        'message_transport_type' => 'email',
>         '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' );
>+    C4::Letters::EnqueueLetter($my_message) for 1..5;
>+
>+    $send_or_die_count = 0; # reset
>+    my $messages_sent;
>+    my $regex = qr|Fake send_or_die|;
>+    warning_like {
>+        $messages_sent = C4::Letters::SendQueuedMessages( { limit => 1 } ) }
>+        $regex,
>+        "SendQueuedMessages with limit 1";
>+    is( $messages_sent, 1,
>+        'Sent 1 message with limit of 1 and 5 unprocessed messages' );
>+
>+    warnings_like {
>+        $messages_sent = C4::Letters::SendQueuedMessages( { limit => 2 } ) }
>+        [ map { $regex } 1..2 ],
>+        "SendQueuedMessages with limit 2";
>+    is( $messages_sent, 2,
>+        'Sent 2 messages with limit of 2 and 4 unprocessed messages' );
>+
>+    warnings_like {
>+        $messages_sent = C4::Letters::SendQueuedMessages( { limit => 3 } ) }
>+        [ map { $regex } 1..2 ],
>+        "SendQueuedMessages with limit 3";
>+    is( $messages_sent, 2,
>+        'Sent 2 messages with limit of 3 and 2 unprocessed messages' );
>+
>+    is( $send_or_die_count, 5, '5 messages sent' );
>+    # Mimic correct status in queue for next tests
>+    Koha::Notice::Messages->search({ to_address => { 'LIKE', '%wrong.net' }})->update({ status => 'sent' });
>+
>+    # Now add a few domain limits too, sending 2 more mails to wrongnet, 2 to fake1, 2 to fake2
>+    # Since we already sent 5 to wrong.net, we expect one deferral when limiting to 6
>+    # Similarly we arrange for 1 deferral for fake1, and 2 for fake2
>+    # We set therefore limit to 3 sent messages: we expect 2 sent, 4 deferred (so 6 processed)
>+    t::lib::Mocks::mock_config( 'message_domain_limits', { domain => [
>+        { name => 'wrong.net',    limit => 6, unit => '1m' },
>+        { name => 'fake1.domain', limit => 1, unit => '1m' },
>+        { name => 'fake2.domain', limit => 0, unit => '1m' },
>+    ]});
>+    C4::Letters::EnqueueLetter($my_message) for 1..2;
>+    $my_message->{to_address} = 'someone@fake1.domain';
>+    C4::Letters::EnqueueLetter($my_message) for 1..2;
>+    $my_message->{to_address} = 'another@fake2.domain';
>+    C4::Letters::EnqueueLetter($my_message) for 1..2;
>+    my $mocked_util = Test::MockModule->new('Koha::Notice::Util');
>+    my $count_exceeds_calls = 0;
>+    $mocked_util->mock( 'exceeds_limit', sub {
>+        $count_exceeds_calls++;
>+        $mocked_util->original('exceeds_limit')->(@_);
>+    });
>+    warnings_like {
>+        $messages_sent = C4::Letters::SendQueuedMessages({ limit => 3 }) }
>+        [ qr/wrong.net reached limit/, $regex, qr/fake1.domain reached limit/, $regex ],
>+        "SendQueuedMessages with limit 2 and domain limits";
>+    is( $messages_sent, 2, 'Only expecting 2 sent messages' );
>+    is(  Koha::Notice::Messages->search({ status => 'pending' })->count, 4, 'Still 4 pending' );
>+    is( $count_exceeds_calls, 6, 'We saw 6 messages while checking domain limits: so we deferred 4' );
> };
> 
> subtest 'Test message_id parameter for SendQueuedMessages' => sub {
> 
>-    plan tests => 7;
>+    plan tests => 8;
> 
>     my $dbh = C4::Context->dbh;
> 
>@@ -1092,8 +1147,9 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub {
>         'from_address'           => '@example.com' # invalid from_address
>     };
>     my $message_id = C4::Letters::EnqueueLetter($my_message);
>+    $send_or_die_count = 0; # reset
>     my $processed = C4::Letters::SendQueuedMessages( { message_id => $message_id } );
>-    is( $processed, 1, 'Processed 1 message when one message_id passed' );
>+    is( $send_or_die_count, 0, 'Nothing sent when one message_id passed' );
>     my $message_1 = C4::Letters::GetMessage($message_id);
>     is( $message_1->{status}, 'failed', 'Invalid from_address => status failed' );
>     is( $message_1->{failure_code}, 'INVALID_EMAIL:from', 'Failure code set correctly for invalid email parameter');
>@@ -1103,6 +1159,7 @@ subtest 'Test message_id parameter for SendQueuedMessages' => sub {
>     warning_like { C4::Letters::SendQueuedMessages( { message_id => $message_id } ); }
>         qr|Fake send_or_die|,
>         "SendQueuedMessages is using the mocked send_or_die routine";
>+    is( $send_or_die_count, 1, 'One message passed through' );
>     $message_1 = C4::Letters::GetMessage($message_1->{message_id});
>     my $message_2 = C4::Letters::GetMessage($message_id);
>     is( $message_1->{status}, 'failed', 'Message 1 status is unchanged' );
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 33360: 149256 | 149257 | 149258 | 149546 | 149547 | 149548 | 149549 | 149550 | 149551 | 149552 | 149645 | 149646 | 149647 | 149648 | 149649 | 149650 | 149651 | 149662 | 149663 | 149664 | 149665 | 149666 | 149667 | 149668