Bug 8838: Add digest option for HOLD notice

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Restart all the things!
4) Enable the new digest option for "Hold filled" messages
5) Trap multiple holds for a patron
6) Note a single notices is generated for all the trapped holds!

Signed-off-by: George Williams <george@nekls.org>
Signed-off-by: Laura ONeil <laura@bywatersolutions.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

C4/Reserves.pm

@@ -1848,23 +1848,40 @@ sub _koha_notify_reserve {
     );
 
     my $notification_sent = 0; #Keeping track if a Hold_filled message is sent. If no message can be sent, then default to a print message.
+    my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_TESTING};
     my $send_notification = sub {
-        my ( $mtt, $letter_code ) = (@_);
+        my ( $mtt, $letter_code, $wants_digest ) = (@_);
         return unless defined $letter_code;
-        $letter_params{letter_code} = $letter_code;
+        $letter_params{letter_code}            = $letter_code;
         $letter_params{message_transport_type} = $mtt;
-        my $letter =  C4::Letters::GetPreparedLetter ( %letter_params );
+        my $letter = C4::Letters::GetPreparedLetter(%letter_params);
         unless ($letter) {
             warn "Could not find a letter called '$letter_params{'letter_code'}' for $mtt in the 'reserves' module";
             return;
         }
 
-        C4::Letters::EnqueueLetter( {
+        unless ($wants_digest) {
-            letter => $letter,
+            C4::Letters::EnqueueLetter(
-            borrowernumber => $borrowernumber,
+                {
-            from_address => $from_email_address,
+                    letter                 => $letter,
-            message_transport_type => $mtt,
+                    borrowernumber         => $borrowernumber,
-        } );
+                    from_address           => $from_email_address,
+                    message_transport_type => $mtt,
+                }
+            );
+        } else {
+            C4::Context->dbh->do(q|LOCK TABLE message_queue READ|)  unless $do_not_lock;
+            C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|) unless $do_not_lock;
+            my $message = C4::Message->find_last_message( $patron->unblessed, $letter_code, $mtt );
+            unless ($message) {
+                C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
+                C4::Message->enqueue( $letter, $patron, $mtt );
+            } else {
+                $message->append($letter);
+                $message->update;
+            }
+            C4::Context->dbh->do(q|UNLOCK TABLES|) unless $do_not_lock;
+        }
     };
 
     while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) {
@@ -1875,7 +1892,7 @@ sub _koha_notify_reserve {
             or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call
         );
 
-        &$send_notification($mtt, $letter_code);
+        &$send_notification($mtt, $letter_code, $messagingprefs->{wants_digest});
         $notification_sent++;
     }
     #Making sure that a print notification is sent if no other transport types can be utilized.

installer/data/mysql/atomicupdate/bug_8838.pl

@@ -0,0 +1,37 @@
+use Modern::Perl;
+
+return {
+    bug_number  => "8838",
+    description => "Add digest option for HOLD notice",
+    up          => sub {
+        my ($args) = @_;
+        my ( $dbh, $out ) = @$args{qw(dbh out)};
+
+        $dbh->do(q{
+            INSERT INTO `letter` VALUES (NULL,'reserves','HOLDDGST','','Hold available for pickup (digest)',0,'Hold(s) available for pickup','You have one or more holds available for pickup:\r\n----\r\nTitle: [% hold.biblio.title %]\r\nAuthor: [% hold.biblio.author %]\r\nCopy: [% hold.item.copynumber %]\r\nLocation: [% hold.branch.branchname %]\r\nWaiting since: [% hold.waitingdate %]:\r\n[% hold.branch.branchaddress1 %]\r\n[% hold.branch.branchaddress2 %]\r\n[% hold.branch.branchaddress3 %]\r\n[% hold.branch.branchcity %] [% hold.branch.branchzip %]\r\n----','email','default','2023-08-29 18:42:15');
+        });
+
+        $dbh->do(q{
+            INSERT INTO message_transports VALUES
+            ( 4, "email", 1, "reserves", "HOLDDGST", "" ),
+            ( 4, "sms", 1, "reserves", "HOLDDGST", "" ),
+            ( 4, "phone", 1, "reserves", "HOLDDGST", "");
+        });
+
+        # Print useful stuff here
+        # tables
+        say $out "Added new table 'XXX'";
+        say $out "Added column 'XXX.YYY'";
+
+        # sysprefs
+        say $out "Added new system preference 'XXX'";
+        say $out "Updated system preference 'XXX'";
+        say $out "Removed system preference 'XXX'";
+
+        # permissions
+        say $out "Added new permission 'XXX'";
+
+        # letters
+        say $out "Added new letter 'XXX' (TRANSPORT)";
+    },
+};

installer/data/mysql/en/mandatory/sample_notices.yml

@@ -1794,6 +1794,29 @@ tables:
             - "<<branches.branchaddress3>>"
             - "<<branches.branchcity>> <<branches.branchzip>>"
 
+        - module: reserves
+          code: HOLDDGST
+          branchcode: ""
+          name: "Hold(s) available for pickup"
+          is_html: 0
+          title: "Hold(s) available for pickup"
+          message_transport_type: email
+          lang: default
+          content:
+            - "You have one or more holds available for pickup:"
+            - "----"
+            - "Title: [% hold.biblio.title %]"
+            - "Author: [% hold.biblio.author %]"
+            - "Copy: [% hold.item.copynumber %]"
+            - "Waiting since: [% hold.waitingdate %]"
+            - "Waitng at: [% hold.branch.branchname %]"
+            - "[% hold.branch.branchaddress1 %]"
+            - "[% hold.branch.branchaddress2 %]"
+            - "[% hold.branch.branchaddress3 %]"
+            - "[% hold.branch.branchcity %] [% hold.branch.branchzip %]"
+            - "----"
+            - "Thank you!"
+
         - module: reserves
           code: HOLD
           branchcode: ""

installer/data/mysql/mandatory/sample_notices_message_transports.sql

@@ -19,6 +19,9 @@ values
 (4, 'sms',   0, 'reserves',    'HOLD'),
 (4, 'phone', 0, 'reserves',    'HOLD'),
 (4, 'itiva', 0, 'reserves',    'HOLD'),
+(4, 'email', 1, 'reserves',    'HOLDDGST'),
+(4, 'sms',   1, 'reserves',    'HOLDDGST'),
+(4, 'phone', 1, 'reserves',    'HOLDDGST'),
 (5, 'email', 0, 'circulation', 'CHECKIN'),
 (5, 'sms',   0, 'circulation', 'CHECKIN'),
 (5, 'phone', 0, 'circulation', 'CHECKIN'),

t/db_dependent/Reserves.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 76;
+use Test::More tests => 79;
 use Test::MockModule;
 use Test::Warn;
 
@@ -1818,3 +1818,106 @@ subtest '_Findgroupreserves' => sub {
 
     $schema->txn_rollback;
 };
+
+subtest 'HOLDDGST tests' => sub {
+
+    plan tests => 2;
+    $schema->storage->txn_begin;
+
+    my $branch = $builder->build_object({
+        class => 'Koha::Libraries',
+        value => {
+            branchemail => 'branch@e.mail',
+            branchreplyto => 'branch@reply.to',
+            pickup_location => 1
+        }
+    });
+    my $item = $builder->build_sample_item({
+        homebranch => $branch->branchcode,
+        holdingbranch => $branch->branchcode
+    });
+    my $item2 = $builder->build_sample_item({
+        homebranch => $branch->branchcode,
+        holdingbranch => $branch->branchcode
+    });
+
+    my $wants_hold_and_email = {
+        wants_digest => '1',
+        transports => {
+            sms => 'HOLDDGST',
+            email => 'HOLDDGST',
+            },
+        letter_code => 'HOLDDGST'
+    };
+
+    my $mp = Test::MockModule->new( 'C4::Members::Messaging' );
+
+    $mp->mock("GetMessagingPreferences",$wants_hold_and_email);
+
+    $dbh->do('DELETE FROM letter');
+
+    my $email_hold_notice = $builder->build({
+            source => 'Letter',
+            value => {
+                message_transport_type => 'email',
+                branchcode => '',
+                code => 'HOLDDGST',
+                module => 'reserves',
+                lang => 'default',
+            }
+        });
+
+    my $sms_hold_notice = $builder->build({
+            source => 'Letter',
+            value => {
+                message_transport_type => 'sms',
+                branchcode => '',
+                code => 'HOLDDGST',
+                module => 'reserves',
+                lang=>'default',
+            }
+        });
+
+    my $hold_borrower = $builder->build({
+            source => 'Borrower',
+            value => {
+                smsalertnumber=>'5555555551',
+                email=>'a@c.com',
+            }
+        })->{borrowernumber};
+
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $hold_borrower,
+            biblionumber   => $item->biblionumber,
+        }
+    );
+
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $item2->homebranch,
+            borrowernumber => $hold_borrower,
+            biblionumber   => $item2->biblionumber,
+        }
+    );
+
+    ModReserveAffect($item->itemnumber, $hold_borrower, 0);
+    ModReserveAffect($item2->itemnumber, $hold_borrower, 0);
+
+    my $sms_count = $schema->resultset('MessageQueue')->search({
+            letter_code     => 'HOLDDGST',
+            message_transport_type => 'sms',
+            borrowernumber => $hold_borrower,
+        })->count;
+    is($sms_count, 1 ,"Only one sms hold digest message created for two holds");
+
+    my $email_count = $schema->resultset('MessageQueue')->search({
+            letter_code     => 'HOLDDGST',
+            message_transport_type => 'email',
+            borrowernumber => $hold_borrower,
+        })->count;
+    is($email_count, 1 ,"Only one email hold digest message created for two holds");
+
+    $schema->txn_rollback;
+};