Attachment 172538 Details for Bug 23295 – Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail

[patch] Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail

Bug-23295-Automatically-restrict-debar-patrons-whe.patch (text/plain), 21.00 KB, created by Kyle M Hall (khall) on 2024-10-08 16:43:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2024-10-08 16:43:54 UTC

Size: 21.00 KB

patch

obsolete

>From fea125188afd179f5183a1e7b24d001cccb61b32 Mon Sep 17 00:00:00 2001
>From: Alex Buckley <alexbuckley@catalyst.net.nz>
>Date: Wed, 10 Jul 2019 17:28:23 +0000
>Subject: [PATCH] Bug 23295: Automatically restrict (debar) patrons when
> email/sms notices fail
>
>When the 'RestrictPatronsWithFailedNotices' syspref is enabled then
>patrons with email and sms notices which failed sending (have a
>message_queue.status field of 'failed') have a restriction (debarment)
>applied to them.
>
>Test plan:
>1. In the Koha staff client > Tools > Overdue notice/status triggers and
>create the 'First' rule for all patron categories as:
>
>Delay: 1
>Letter: Overdue Notice
>SMS: ticked
>
>Ensure you have an SMS notice for the ODUE letter.
>
>2. In the system preferences make sure you enter dummy data into the
>SMSSendUsername, SMSSendPassword and EmailSMSSendDriverFromAddress
>sysprefs
>
>2. Find two non-debarred patrons and make sure they have invalid SMS numbers set. The SMS numbers must be INCORRECT, for example "123" as an SMS number. Leaving this field empty will result in the message_transport_type defaulting to 'print' instead of 'sms'.
>
>3. Check one item out to each patron in step 2
>
>4. Jump into the database and run the query:
>
>UPDATE issues SET date_due=<2 days before current date> WHERE
>borrowernumber=<borrower1>;
>
>UPDATE issues SET date_due=<2 days before current date> WHERE
>borrowernumber=<borrower2>;
>
>5. Go to  misc/cronjobs directory and enter the Koha shell:
>
>sudo koha-shell <instancename>
>
>6. Run: ./overdue_notices.pl
>
>7. Exit the shell and jump back into the database and run the query:
>
>SELECT message_transport_type, status FROM message_queue WHERE
>borrowernumber=<borrower1> OR borrowernumber=<borrower2>;
>
>8. Confirm both new notice records have the message_transport_type is 'sms' and the status of 'pending'
>
>9. Exit the database and re-enter the Koha shell and run the command:
>./process_message_queue.pl
>
>10. Jump back into the database re-run the query from step 7 and confirm
>the status is 'failed' for both
>
>11. Also run the query:
>SELECT * FROM borrower_debarments WHERE borrowernumber=<borrower1> OR
>borrowernumber=<borrower2>;
>
>Notice there is no added debarment to these two patrons
>
>12. Apply patch, restart memcached and plack. In the
>installer/data/mysql directory enter the Koha shell and run the command:
>./update_database.pl
>
>13. In the Administration > Global System Preferences interface of the
>staff client notice there is a new system (set to "Don't" by default)
>named 'RestrictPatronsWithFailedNotices'.
>
>Enable it (i.e. select 'Do')
>
>14. Create a new file in the /etc/cron.daily directory named koha-custom and
>add the following line to it:
>
>koha-foreach --chdir --enabled
>/usr/share/koha/bin/cronjobs/restrict_patrons_with_failed_notices.pl
>
>15. In the  misc/cronjobs directory enter the Koha shell and run the command:
>./restrict_patrons_with_failed_notices.pl
>
>16. The script should output text saying: There are borrowers with
>failed SMS or email notices
>
>However because you haven't given the script the argument -c it won't
>apply debarments (restrictions) to any of the patrons with the failed
>SMS or email notices.
>
>16. Query the borrower_debarments table:
>
>SELECT * FROM borrower_debarments WHERE borrowernumber=<borrower1> OR
>borrowernumber=<borrower2>;
>
>Notice they still have no restriction
>
>17. Now in the Koha shell run the command:
>./restrict_patrons_with_failed_notices.pl -c
>
>18. Notice the script outputs the text:
>There are borrowers with failed SMS or email notices
>Applying restriction to patron <borrowernumber>: <borrower firstname>
><borrower surname>;
>
>19. Repeat step 16 and notice both patrons now have 1 restriction each
>with the borrower_debarments.type=SUSPENSION and comment=SMSnumber
>invalid and expiration=NULL
>
>20. Query the borrowers table:
>SELECT debarred, debarredcomment FROM borrowers WHERE
>borrowernumber=<borrower1> OR borrowernumber=<borrower2>;
>
>21. Notice the values are:
>debarred= 9999-12-31
>debarredcomment= SMS number invalid
>
>22. Repeat step 17 and notice the script outputs:
>There are borrowers with failed SMS or email notices
>
>Patron <borrowernumber>: <borrower firstname> <borrower surname> is currently restricted due to having an invalid SMS number. No new restriction applied"
>
>23. Repeat step 16 and notice no new debarment has been added to those
>borrowers as they have already been restricted from having a failed SMS
>notice.
>
>24. In the Koha home directory run the command:
>prove t/db_dependent/Koha/Notices.t
>
>This unit test contains the tests for the new subroutines added to
>Koha/Notice/Message.pm which are restrict_patron_when_notice_fails() and
>get_failed_notices()
>
>25. All tests should pass
>
>26. Sign off
>
>Sponsored-by: Catalyst IT
>Signed-off-by: David Nind <david@davidnind.com>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> Koha/Notice/Message.pm                        |  36 ++++
> Koha/Notice/Messages.pm                       |  22 ++-
> ...estrictPatronsWithFailedNotices_syspref.pl |  16 ++
> installer/data/mysql/mandatory/sysprefs.sql   |   1 +
> .../en/modules/admin/preferences/patrons.pref |   7 +
> .../restrict_patrons_with_failed_notices.pl   | 168 ++++++++++++++++++
> t/db_dependent/Koha/Notices.t                 | 101 ++++++++++-
> 7 files changed, 348 insertions(+), 3 deletions(-)
> create mode 100755 installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.pl
> create mode 100755 misc/cronjobs/restrict_patrons_with_failed_notices.pl
>
>diff --git a/Koha/Notice/Message.pm b/Koha/Notice/Message.pm
>index 694ff0a0b0c..b94207a62e6 100644
>--- a/Koha/Notice/Message.pm
>+++ b/Koha/Notice/Message.pm
>@@ -18,6 +18,7 @@ package Koha::Notice::Message;
> use Modern::Perl;
> 
> use Koha::Database;
>+use Koha::Patron::Debarments qw( AddDebarment );
> 
> use base qw(Koha::Object);
> 
>@@ -88,6 +89,41 @@ EOS
>     return $wrapped;
> }
> 
>+=head3 restrict_patron_when_notice_fails
>+
>+    $failed_notice->restrict_patron_when_notice_fails;
>+
>+Places a restriction (debarment) on patrons with failed SMS and email notices.
>+
>+=cut
>+
>+sub restrict_patron_when_notice_fails {
>+    my ($self) = @_;
>+
>+    # Set the appropriate restriction (debarment) comment depending if the failed
>+    # message is a SMS or email notice. If the failed notice is neither then
>+    # return without placing a restriction
>+    my $comment;
>+    if ( $self->message_transport_type eq 'email' ) {
>+        $comment = 'Email address invalid';
>+    } elsif ( $self->message_transport_type eq 'sms' ) {
>+        $comment = 'SMS number invalid';
>+    } else {
>+        return;
>+    }
>+
>+    AddDebarment(
>+        {
>+            borrowernumber => $self->borrowernumber,
>+            type           => 'SUSPENSION',
>+            comment        => $comment,
>+            expiration     => undef,
>+        }
>+    );
>+
>+    return $self;
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/Koha/Notice/Messages.pm b/Koha/Notice/Messages.pm
>index 3fedd5d1424..3c8aef76584 100644
>--- a/Koha/Notice/Messages.pm
>+++ b/Koha/Notice/Messages.pm
>@@ -19,7 +19,6 @@ use Modern::Perl;
> 
> 
> use Koha::Database;
>-
> use Koha::Notice::Message;
> 
> use base qw(Koha::Objects);
>@@ -34,6 +33,27 @@ Koha::Notice::Message - Koha notice message Object class, related to the message
> 
> =cut
> 
>+=head3 get_failed_notices
>+
>+    my $failed_notices = Koha::Notice::Messages->get_failed_notices({ days => 7 });
>+
>+Returns a hashref of all notices that have failed to send in the last X days, as specified in the 'days' parameter.
>+If not specified, will default to the last 7 days.
>+
>+=cut
>+
>+sub get_failed_notices {
>+    my ( $self, $params ) = @_;
>+    my $days = $params->{days} ? $params->{days} : 7;
>+
>+    return $self->search(
>+        {
>+            time_queued => { -between => \"DATE_SUB(NOW(), INTERVAL $days DAY) AND NOW()" },
>+            status      => "failed",
>+        }
>+    );
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.pl b/installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.pl
>new file mode 100755
>index 00000000000..02dd9d24e05
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.pl
>@@ -0,0 +1,16 @@
>+use Modern::Perl;
>+
>+return {
>+    bug_number  => "23295",
>+    description => "Automatically debar patrons if SMS or email notice fail",
>+    up          => sub {
>+        my ($args) = @_;
>+        my ( $dbh, $out ) = @$args{qw(dbh out)};
>+
>+        $dbh->do(
>+            q{ INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES ('RestrictPatronsWithFailedNotices', '0', NULL, 'If enabled then when SMS and email notices fail sending at the Koha level then a debarment will be applied to a patrons account', 'YesNo') }
>+        );
>+
>+        say $out "Added system preference 'RestrictPatronsWithFailedNotices'";
>+    },
>+};
>diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql
>index 66c7b008f21..d4be46b9c1d 100644
>--- a/installer/data/mysql/mandatory/sysprefs.sql
>+++ b/installer/data/mysql/mandatory/sysprefs.sql
>@@ -677,6 +677,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('RestrictedPageLocalIPs','',NULL,'Beginning of IP addresses considered as local (comma separated ex: "127.0.0,127.0.2")','Free'),
> ('RestrictedPageTitle','',NULL,'Title of the restricted page (breadcrumb and header)','Free'),
> ('RestrictionBlockRenewing','0',NULL,'If patron is restricted, should renewal be allowed or blocked','YesNo'),
>+('RestrictPatronsWithFailedNotices', '0', NULL, 'If enabled then when SMS and email notices fail sending at the Koha level then a debarment will be applied to a patrons account', 'YesNo'),
> ('RetainCatalogSearchTerms', '1', NULL, 'If enabled, searches entered into the catalog search bar will be retained', 'YesNo'),
> ('RetainPatronsSearchTerms', '1', NULL, 'If enabled, searches entered into the checkout and patrons search bar will be retained', 'YesNo'),
> ('ReturnBeforeExpiry','0',NULL,'If ON, checkout will be prevented if returndate is after patron card expiry','YesNo'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
>index 4376132e57e..baaf682390d 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
>@@ -407,6 +407,13 @@ Patrons:
>                1: "must have"
>                0: "doesn't need"
>          - a guarantor when adding the patron.
>+     -
>+         - pref: RestrictPatronsWithFailedNotices
>+           choices:
>+               1: Apply
>+               0: "Don't apply"
>+         - "a restriction to a patron when their email and SMS messages fail to send at the Koha level."
>+         - "<br><strong>NOTE:</strong> This system preference requires the <code>misc/cronjobs/restrict_patrons_with_failed_notices.pl</code> cronjob. Ask your system administrator to schedule it."
> 
>     Privacy:
>      -
>diff --git a/misc/cronjobs/restrict_patrons_with_failed_notices.pl b/misc/cronjobs/restrict_patrons_with_failed_notices.pl
>new file mode 100755
>index 00000000000..40ae00d09c4
>--- /dev/null
>+++ b/misc/cronjobs/restrict_patrons_with_failed_notices.pl
>@@ -0,0 +1,168 @@
>+#!/usr/bin/perl
>+
>+# Copyright 2020 Catalyst IT
>+#
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use Getopt::Long;
>+use Pod::Usage;
>+use C4::Log;
>+
>+use C4::Context;
>+use Koha::Patrons;
>+use C4::Letters;
>+use Koha::Notice::Message;
>+
>+# Getting options
>+my ( $help, $verbose, $confirm );
>+GetOptions(
>+    'h|help'    => \$help,
>+    'v|verbose' => \$verbose,
>+    'c|confirm' => \$confirm,
>+);
>+
>+pod2usage(1) if $help;
>+
>+if ( !C4::Context->preference('RestrictPatronsWithFailedNotices') and $verbose ) {
>+    warn <<'END_WARN';
>+
>+The 'RestrictPatronsWithFailedNotices' system preference is disabled.
>+This script requires the 'RestrictPatronsWithFailedNotices' system preference to be enabled.
>+Exiting cronjob
>+
>+END_WARN
>+}
>+cronlogaction();
>+
>+my @failed_notices = Koha::Notice::Messages->get_failed_notices( { days => 7 } )->as_list;
>+
>+if ( C4::Context->preference('RestrictPatronsWithFailedNotices') ) {
>+    if (@failed_notices) {
>+        say "There are borrowers with failed SMS or email notices" if $verbose;
>+
>+        foreach my $failed_notice (@failed_notices) {
>+
>+            # If failed notice is not a sms or email notice then skip to next failed notice
>+            next
>+                unless ( $failed_notice->message_transport_type eq 'sms'
>+                || $failed_notice->message_transport_type eq 'email' );
>+
>+            # If failed sms or email notice has no recipient patron then skip to next failed
>+            # notice
>+            next unless $failed_notice->borrowernumber;
>+
>+            # Find the patron recipient of the failed SMS or email notice.
>+            my $patron = Koha::Patrons->find( $failed_notice->borrowernumber );
>+
>+            # Check if patron of failed SMS or email notice is already restricted due to having
>+            # this happen before. If they are already restricted due to having invalid SMS or
>+            # email address don't apply a new restriction (debarment) to their account.
>+            if ( $patron->restrictions->search( { comment => 'SMS number invalid' } )->count > 0 ) {
>+                say "Patron "
>+                    . $patron->borrowernumber . ":" . " "
>+                    . $patron->firstname . " "
>+                    . $patron->surname . " "
>+                    . "is currently restricted due to having an invalid SMS number. No new restriction applied"
>+                    if $verbose;
>+                next;
>+            } elsif ( $patron->restrictions->search( { comment => 'Email address invalid' } )->count > 0 ) {
>+                say "Patron "
>+                    . $patron->borrowernumber . ":" . " "
>+                    . $patron->firstname . " "
>+                    . $patron->surname . " "
>+                    . "is currently restricted due to having an invalid email address. No new restriction applied"
>+                    if $verbose;
>+                next;
>+            }
>+            if ($confirm) {
>+
>+                # Patron has not been previously restricted for having failed SMS
>+                # or email addresses apply a restriction now.
>+                say "Applying restriction to patron "
>+                    . $patron->borrowernumber . ":" . " "
>+                    . $patron->firstname . " "
>+                    . $patron->surname
>+                    if $verbose;
>+                $failed_notice->restrict_patron_when_notice_fails;
>+            }
>+        }
>+    } else {
>+        say "There are no failed SMS or email notices" if $verbose;
>+    }
>+}
>+
>+exit(0);
>+
>+__END__
>+
>+=head1 NAME
>+
>+restrict_patrons_with_failed_notices.pl
>+
>+=head1 SYNOPSIS
>+
>+./restrict_patrons_with_failed_notices.pl -h
>+
>+Use this script to creates a debarment for all patrons with failed SMS and email notices.
>+
>+The 'RestrictPatronsWithFailedNotices' syspref must be enabled for this script to place restrictions to patrons accounts.
>+
>+=head1 OPTIONS
>+
>+=over 8
>+
>+=item B<-h|--help>
>+
>+Prints this help message
>+
>+=item B<-v|--verbose>
>+
>+Set the verbose flag
>+
>+=item B<-c|--confirm>
>+
>+The script will alter the database placing a restriction on patrons with failed SMS and email notices.
>+
>+=back
>+
>+=head1 AUTHOR
>+
>+Alex Buckley <alexbuckley@catalyst.net.nz>
>+
>+=head1 COPYRIGHT
>+
>+Copyright 2019 Catalyst IT
>+
>+=head1 LICENSE
>+
>+This file is part of Koha.
>+
>+Koha is free software; you can redistribute it and/or modify it
>+under the terms of the GNU General Public License as published by
>+the Free Software Foundation; either version 3 of the License, or
>+(at your option) any later version.
>+
>+Koha is distributed in the hope that it will be useful, but
>+WITHOUT ANY WARRANTY; without even the implied warranty of
>+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+GNU General Public License for more details.
>+
>+You should have received a copy of the GNU General Public License
>+along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+=cut
>diff --git a/t/db_dependent/Koha/Notices.t b/t/db_dependent/Koha/Notices.t
>index c6c47054318..9a58e9e758f 100755
>--- a/t/db_dependent/Koha/Notices.t
>+++ b/t/db_dependent/Koha/Notices.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 4;
>+use Test::More tests => 9;
> 
> use Koha::Notice::Templates;
> use Koha::Database;
>@@ -67,6 +67,104 @@ $retrieved_template->delete;
> is( Koha::Notice::Templates->search->count,
>     $nb_of_templates, 'Delete should have deleted the template' );
> 
>+## Tests for Koha::Notice::Messages->get_failed_notices
>+
>+# Remove all existing messages in the message_queue
>+Koha::Notice::Messages->delete;
>+
>+# Make a patron
>+my $patron_category = $builder->build( { source => 'Category' } )->{categorycode};
>+my $patron          = Koha::Patron->new(
>+    {
>+        firstname      => 'Jane',
>+        surname        => 'Smith',
>+        categorycode   => $patron_category,
>+        branchcode     => $library->{branchcode},
>+        smsalertnumber => '123',
>+    }
>+)->store;
>+my $borrowernumber = $patron->borrowernumber;
>+
>+# With all notices removed from the message_queue table confirm get_failed_notices() returns 0
>+my @failed_notices = Koha::Notice::Messages->get_failed_notices->as_list;
>+is( @failed_notices, 0, 'No failed notices currently exist' );
>+
>+# Add a failed notice to the message_queue table
>+my $message = Koha::Notice::Message->new(
>+    {
>+        borrowernumber         => $borrowernumber,
>+        subject                => 'subject',
>+        content                => 'content',
>+        message_transport_type => 'sms',
>+        status                 => 'failed',
>+        letter_code            => 'just_a_code',
>+        time_queued            => \"NOW()",
>+    }
>+)->store;
>+
>+# With one failed notice in the message_queue table confirm get_failed_notices() returns 1
>+my @failed_notices2 = Koha::Notice::Messages->get_failed_notices->as_list;
>+is( @failed_notices2, 1, 'One failed notice currently exists' );
>+
>+# Change failed notice status to 'pending'
>+$message->update( { status => 'pending' } );
>+
>+# With the 1 failed notice in the message_queue table marked 'pending' confirm get_failed_notices() returns 0
>+my @failed_notices3 = Koha::Notice::Messages->get_failed_notices->as_list;
>+is( @failed_notices3, 0, 'No failed notices currently existing, now the notice has been marked pending' );
>+
>+## Tests for Koha::Notice::Message::restrict_patron_when_notice_fails
>+
>+# Empty the borrower_debarments table
>+my $dbh = C4::Context->dbh;
>+$dbh->do(q|DELETE FROM borrower_debarments|);
>+
>+# Change the status of the notice back to 'failed'
>+$message->update( { status => 'failed' } );
>+
>+my @failed_notices4 = Koha::Notice::Messages->get_failed_notices->as_list;
>+
>+# There should be one failed notice
>+if (@failed_notices4) {
>+
>+    # Restrict the borrower who has the failed notice
>+    foreach my $failed_notice (@failed_notices4) {
>+        if ( $failed_notice->message_transport_type eq 'sms' || $failed_notice->message_transport_type eq 'email' ) {
>+            $failed_notice->restrict_patron_when_notice_fails;
>+        }
>+    }
>+}
>+
>+# Confirm that the restrict_patron_when_notice_fails() has added a restriction to the patron
>+is(
>+    $patron->restrictions->search( { comment => 'SMS number invalid' } )->count, 1,
>+    "Patron has a restriction placed on them"
>+);
>+
>+# Restrict the borrower who has the failed notice
>+foreach my $failed_notice (@failed_notices4) {
>+    if ( $failed_notice->message_transport_type eq 'sms' || $failed_notice->message_transport_type eq 'email' ) {
>+
>+        # If the borrower already has a debarment for failed SMS or email notice then don't apply
>+        # a new debarment to their account
>+        if ( $patron->restrictions->search( { comment => 'SMS number invalid' } )->count > 0 ) {
>+            next;
>+        } elsif ( $patron->restrictions->search( { comment => 'Email address invalid' } )->count > 0 ) {
>+            next;
>+        }
>+
>+        # Place the debarment if the borrower doesn't already have one for failed SMS or email
>+        # notice
>+        $failed_notice->restrict_patron_when_notice_fails;
>+    }
>+}
>+
>+# Confirm that no new debarment is added to the borrower
>+is(
>+    $patron->restrictions->search( { comment => 'SMS number invalid' } )->count, 1,
>+    "No new restriction has been placed on the patron"
>+);
>+
> subtest 'find_effective_template' => sub {
>     plan tests => 7;
> 
>@@ -135,4 +233,3 @@ subtest 'find_effective_template' => sub {
> };
> 
> $schema->storage->txn_rollback;
>-
>-- 
>2.39.5 (Apple Git-154)

Actions: View | Diff | Splinter Review

Attachments on bug 23295: 91471 | 102068 | 102098 | 105003 | 116181 | 158687 | 160785 | 160890 | 167599 | 167778 | 172507 | 172538 | 172539 | 172540