From fde91e178d7a2fd4748ad1712d5fc59170ee3919 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 30 Aug 2023 07:06:31 -0400 Subject: [PATCH] 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 Signed-off-by: Laura ONeil Signed-off-by: Emily Lamancusa Signed-off-by: Tomas Cohen Arazi --- C4/Reserves.pm | 37 ++++-- installer/data/mysql/atomicupdate/bug_8838.pl | 37 ++++++ .../mysql/en/mandatory/sample_notices.yml | 23 ++++ .../sample_notices_message_transports.sql | 3 + t/db_dependent/Reserves.t | 105 +++++++++++++++++- 5 files changed, 194 insertions(+), 11 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_8838.pl diff --git a/C4/Reserves.pm b/C4/Reserves.pm index e2dc17df69..da7e13a75b 100644 --- a/C4/Reserves.pm +++ b/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( { - letter => $letter, - borrowernumber => $borrowernumber, - from_address => $from_email_address, - message_transport_type => $mtt, - } ); + unless ($wants_digest) { + C4::Letters::EnqueueLetter( + { + letter => $letter, + 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. diff --git a/installer/data/mysql/atomicupdate/bug_8838.pl b/installer/data/mysql/atomicupdate/bug_8838.pl new file mode 100755 index 0000000000..cfcb5e8fb5 --- /dev/null +++ b/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)"; + }, +}; diff --git a/installer/data/mysql/en/mandatory/sample_notices.yml b/installer/data/mysql/en/mandatory/sample_notices.yml index a55ca0da08..34ae02223c 100644 --- a/installer/data/mysql/en/mandatory/sample_notices.yml +++ b/installer/data/mysql/en/mandatory/sample_notices.yml @@ -1794,6 +1794,29 @@ tables: - "<>" - "<> <>" + - 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: "" diff --git a/installer/data/mysql/mandatory/sample_notices_message_transports.sql b/installer/data/mysql/mandatory/sample_notices_message_transports.sql index d1fae4c0ce..55ed683d6b 100644 --- a/installer/data/mysql/mandatory/sample_notices_message_transports.sql +++ b/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'), diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 9dd6a0b04b..023a031632 100755 --- a/t/db_dependent/Reserves.t +++ b/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; +}; -- 2.39.5