Attachment 154969 Details for Bug 8838 – Bug 8838: Add digest option for HOLD notice

[patch] Bug 8838: Add digest option for HOLD notice

Bug-8838-Add-digest-option-for-HOLD-notice.patch (text/plain), 11.10 KB, created by ByWater Sandboxes on 2023-08-30 14:51:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: ByWater Sandboxes

Created: 2023-08-30 14:51:41 UTC

Size: 11.10 KB

patch

obsolete

>From 167493a14ba67a309c48fa5d4eff4fa2d3e3e528 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <george@nekls.org>
>---
> 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 fd8f7e58bd..77c739ce03 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -1889,23 +1889,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} } ) {
>@@ -1916,7 +1933,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 28c482bf9a..06a1c47524 100644
>--- a/installer/data/mysql/en/mandatory/sample_notices.yml
>+++ b/installer/data/mysql/en/mandatory/sample_notices.yml
>@@ -1771,6 +1771,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: ""
>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 7e2d83e50e..886bb350c3 100755
>--- a/t/db_dependent/Reserves.t
>+++ b/t/db_dependent/Reserves.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 77;
>+use Test::More tests => 78;
> use Test::MockModule;
> use Test::Warn;
> 
>@@ -1773,3 +1773,106 @@ subtest 'DefaultHoldExpiration tests' => 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 8838: 154947 | 154948 | 154950 | 154959 | 154969 | 155415 | 155487 | 155488 | 155489 | 155490 | 155504 | 155505 | 155506 | 155507 | 157360 | 157361 | 157450 | 157451 | 157452 | 157453 | 157454 | 157455 | 157456 | 157457 | 157459 | 157460 | 157461 | 157462