Attachment 91471 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), 19.61 KB, created by Alex Buckley on 2019-07-12 00:01:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Buckley

Created: 2019-07-12 00:01:49 UTC

Size: 19.61 KB

patch

obsolete

>From 943f8491327ee205fef29a93778fe788257caae1 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
>
>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
>
>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: Brimbank Library, Australia
>---
> C4/Letters.pm | 1 +
> Koha/Notice/Message.pm | 61 +++++++++
> ...dd_RestrictPatronsWithFailedNotices_syspref.sql | 1 +
> installer/data/mysql/sysprefs.sql | 1 +
> .../prog/en/modules/admin/preferences/patrons.pref | 7 ++
> .../restrict_patrons_with_failed_notices.pl | 137 +++++++++++++++++++++
> t/db_dependent/Koha/Notices.t | 85 ++++++++++++-
> 7 files changed, 292 insertions(+), 1 deletion(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.sql
> create mode 100755 misc/cronjobs/restrict_patrons_with_failed_notices.pl
>
>diff --git a/C4/Letters.pm b/C4/Letters.pm
>index bdb213a..35c6ccf 100644
>--- a/C4/Letters.pm
>+++ b/C4/Letters.pm
>@@ -37,6 +37,7 @@ use Koha::SMS::Providers;
> 
> use Koha::Email;
> use Koha::Notice::Messages;
>+use Koha::Notice::Message;
> use Koha::DateUtils qw( format_sqldatetime dt_from_string );
> use Koha::Patrons;
> use Koha::Subscriptions;
>diff --git a/Koha/Notice/Message.pm b/Koha/Notice/Message.pm
>index 054f7dd..b255635 100644
>--- a/Koha/Notice/Message.pm
>+++ b/Koha/Notice/Message.pm
>@@ -20,6 +20,8 @@ use Modern::Perl;
> use Carp;
> 
> use Koha::Database;
>+use Koha::Patron::Debarments;
>+use C4::Letters;
> 
> use base qw(Koha::Object);
> 
>@@ -33,6 +35,65 @@ Koha::Notice::Message - Koha notice message Object class, related to the message
> 
> =cut
> 
>+=head3 restrict_patron_when_notice_fails
>+
>+Places a restriction (debarment) on patrons with failed SMS and email notices
>+
>+restrict_patron_when_notice_fails($params);
>+
>+=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 get_failed_notices
>+
>+Retrieves all failed notices in the last 7 days and returns (as C4::Letters::_get_unsent_messages does) an array of matching hash 
>+referenced rows from message_queue with some borrower information added
>+
>+my $failed_notices = get_failed_notices();
>+
>+=cut
>+
>+sub get_failed_notices {
>+ my $dbh = C4::Context->dbh();
>+
>+ my $statement = qq{
>+ SELECT mq.message_id, mq.borrowernumber, mq.subject, mq.content, mq.message_transport_type, mq.status, mq.time_queued, mq.from_address, mq.to_address, mq.content_type, b.branchcode, mq.letter_code
>+ FROM message_queue mq
>+ LEFT JOIN borrowers b ON b.borrowernumber = mq.borrowernumber
>+ WHERE status = ?
>+ AND mq.time_queued BETWEEN date_sub(now(),INTERVAL 1 WEEK) and now();
>+ };
>+ my $sth = $dbh->prepare($statement);
>+ my $result = $sth->execute('failed');
>+ return $sth->fetchall_arrayref({});
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.sql b/installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.sql
>new file mode 100644
>index 0000000..a904513
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_23295-add_RestrictPatronsWithFailedNotices_syspref.sql
>@@ -0,0 +1 @@
>+INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) VALUES ('RestrictPatronsWithFailedNotices', 0, "Do|Don't", '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');
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index a0d1132..2cb8ec0 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -511,6 +511,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('RESTdefaultPageSize','20','','Default page size for endpoints listing objects','Integer'),
> ('RESTOAuth2ClientCredentials','0',NULL,'If enabled, the OAuth2 client credentials flow is enabled for the REST API.','YesNo'),
> ('RESTPublicAPI','1',NULL,'If enabled, the REST API will expose the /public endpoints.','YesNo'),
>+('RestrictPatronsWithFailedNotices', 0, "Do|Don't", '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'),
> ('RestrictedPageLocalIPs','',NULL,'Beginning of IP addresses considered as local (comma separated ex: "127.0.0,127.0.2")','Free'),
> ('RestrictedPageContent','',NULL,'HTML content of the restricted page','TextArea'),
> ('RestrictedPageTitle','',NULL,'Title of the restricted page (breadcrumb and header)','Free'),
>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 9007da9..12398aa 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
>@@ -227,6 +227,13 @@ Patrons:
> no: Allow all permitted users
> - "to access/change superlibrarian privileges."
> - " NOTE: A permitted user needs to have the 'permissions' flag (if no superlibrarian)."
>+ -
>+ - pref: RestrictPatronsWithFailedNotices
>+ choices:
>+ yes: Do
>+ no: "Don't"
>+ - "apply a restriction to a patron when their email and SMS messages fail sending at the Koha level."
>+ - " NOTE: 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 0000000..e4f4de3
>--- /dev/null
>+++ b/misc/cronjobs/restrict_patrons_with_failed_notices.pl
>@@ -0,0 +1,137 @@
>+#!/usr/bin/perl
>+
>+use Modern::Perl;
>+
>+use Getopt::Long;
>+use Pod::Usage;
>+use C4::Log;
>+
>+use C4::Context;
>+use Koha::Patron::Debarments;
>+use Koha::Patrons;
>+use C4::Letters;
>+use Koha::Notice::Message;
>+
>+# Getting options
>+my ($help, $verbose, $confirm, $type);
>+GetOptions(
>+ 'h|help' => \$help,
>+ 'v|verbose' => \$verbose,
>+ 'c|confirm' => \$confirm,
>+);
>+
>+pod2usage(1) if $help;
>+$verbose = 1 unless $confirm;
>+
>+if ( ! C4::Context->preference('RestrictPatronsWithFailedNotices') ) {
>+ warn <<'END_WARN';
>+
>+The 'RestrictPatronsWithFailedNotices' syspref is off.
>+Therefore, it is unlikely that this script will actually place any restrictions
>+on patrons with failed SMS or email notices.
>+To change this, enable the 'RestrictPatronsWithFailedNotices' syspref.
>+
>+END_WARN
>+}
>+cronlogaction();
>+
>+my $failed_notices = Koha::Notice::Message::get_failed_notices();
>+
>+if ( C4::Context->preference('RestrictPatronsWithFailedNotices') ) {
>+ if ( @$failed_notices ) {
>+ say "There are borrowers with failed SMS or email notices";
>+
>+ 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 ( @{ Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber, comment => 'SMS number invalid' }) } ) {
>+ say "Patron " . $patron->borrowernumber . ":" . " " . $patron->firstname . " " . $patron->surname . " " . "is currently restricted due to having an invalid SMS number. No new restriction applied";
>+ next;
>+ } elsif ( @{ Koha::Patron::Debarments::GetDebarments({ borrowernumber => $patron->borrowernumber, comment => 'Email address invalid' }) }) {
>+ say "Patron " . $patron->borrowernumber . ":" . " " . $patron->firstname . " " . $patron->surname . " " . "is currently restricted due to having an invalid email address. No new restriction applied";
>+ 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;
>+ Koha::Notice::Message::restrict_patron_when_notice_fails($failed_notice);
>+ }
>+ }
>+ } else {
>+ say "There are no failed SMS or email notices";
>+ }
>+}
>+
>+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 dc9646f..7f82186 100644
>--- 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 => 3;
>+use Test::More tests => 8;
> 
> use Koha::Notice::Templates;
> use Koha::Database;
>@@ -66,5 +66,88 @@ $retrieved_template->delete;
> is( Koha::Notice::Templates->search->count,
> $nb_of_templates, 'Delete should have deleted the template' );
> 
>+# Test bug 23295 - Test the new subroutine Koha::Notice::Message::get_failed_notices()
>+my $dbh = C4::Context->dbh;
>+
>+# Remove all existing messages in the message_queue
>+$dbh->do(q|DELETE FROM message_queue|);
>+
>+# Make a patron
>+my $patron_category = $builder->build({ source => 'Category' })->{categorycode};
>+my $borrowernumber = Koha::Patron->new({
>+ firstname => 'Jane',
>+ surname => 'Smith',
>+ categorycode => $patron_category,
>+ branchcode => $library->{branchcode},
>+ smsalertnumber => undef,
>+})->store->borrowernumber;
>+
>+# With all notices removed from the message_queue table confirm get_failed_notices() returns 0
>+my $failed_notices = Koha::Notice::Message::get_failed_notices();
>+is( scalar @$failed_notices, 0, 'No failed notices currently exist');
>+
>+# Add a failed notice to the message_queue table
>+$dbh->do(q|
>+ INSERT INTO message_queue(borrowernumber, subject, content, message_transport_type, status, letter_code)
>+ VALUES (?, 'subject', 'content', 'sms', 'failed', 'just_a_code')
>+ |, undef, $borrowernumber
>+);
>+
>+# With one failed notice in the message_queue table confirm get_failed_notices() returns 1
>+my $failed_notices2 = Koha::Notice::Message::get_failed_notices();
>+is( scalar(@$failed_notices2), 1, 'One failed notice currently exists');
>+
>+# Change failed notice status to 'pending'
>+$dbh->do(q|UPDATE message_queue SET 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::Message::get_failed_notices();
>+is( scalar(@$failed_notices3), 0, 'No failed notices currently existing, now the notice has been marked pending');
>+
>+
>+# Test bug 23295 - Test the new subroutine Koha::Notice::Message::restrict_patron_when_notice_fails()
>+
>+# Empty the borrower_debarments table
>+$dbh->do(q|DELETE FROM borrower_debarments|);
>+
>+# Change the status of the notice back to 'failed'
>+$dbh->do(q|UPDATE message_queue SET status='failed'|);
>+
>+my $failed_notices4 = Koha::Notice::Message::get_failed_notices();
>+
>+# 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') {
>+ Koha::Notice::Message::restrict_patron_when_notice_fails($failed_notice);
>+ }
>+ }
>+}
>+
>+# Confirm that the restrict_patron_when_notice_fails() has added a restriction to the patron
>+is(@{ Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber, comment => 'SMS number invalid' })}, 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 ( @{ Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber, comment => 'SMS number invalid' }) } ) {
>+ next;
>+ } elsif ( @{ Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber, comment => 'Email address invalid' }) }) {
>+ next;
>+ }
>+
>+ # Place the debarment if the borrower doesn't already have one for failed SMS or email
>+ # notice
>+ Koha::Notice::Message::restrict_patron_when_notice_fails($failed_notice);
>+ }
>+}
>+
>+# Confirm that no new debarment is added to the borrower
>+is(@{ Koha::Patron::Debarments::GetDebarments({ borrowernumber => $borrowernumber, comment => 'SMS number invalid' })}, 1, "No new restriction has been placed on the patron");
>+
> $schema->storage->txn_rollback;
> 
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

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