Created attachment 91471 [details] [review]
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

warning: 3 lines applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Letters.pm
CONFLICT (content): Merge conflict in C4/Letters.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-23295-Automatically-restrict-debar-patrons-whe-6bIBRK.patch

Created attachment 102068 [details] [review]
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: Brimbank Library, Australia

Rebased, however tests fail:

$ prove -v t/db_dependent/Koha/Notices.t 
t/db_dependent/Koha/Notices.t .. 
1..8
ok 1 - The template should have been added
ok 2 - Find a notice template by pk should return the correct template
ok 3 - Delete should have deleted the template
ok 4 - No failed notices currently exist
not ok 5 - One failed notice currently exists

#   Failed test 'One failed notice currently exists'
#   at t/db_dependent/Koha/Notices.t line 98.
#          got: '0'
#     expected: '1'
ok 6 - No failed notices currently existing, now the notice has been marked pending
not ok 7 - Patron has a restriction placed on them

#   Failed test 'Patron has a restriction placed on them'
#   at t/db_dependent/Koha/Notices.t line 129.
#          got: '0'
#     expected: '1'
not ok 8 - No new restriction has been placed on the patron

#   Failed test 'No new restriction has been placed on the patron'
#   at t/db_dependent/Koha/Notices.t line 150.
#          got: '0'
#     expected: '1'
# Looks like you failed 3 tests of 8.
Dubious, test returned 3 (wstat 768, 0x300)
Failed 3/8 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Notices.t (Wstat: 768 Tests: 8 Failed: 3)
  Failed tests:  5, 7-8
  Non-zero exit status: 3
Files=1, Tests=8,  1 wallclock secs ( 0.02 usr  0.00 sys +  1.08 cusr  0.11 csys =  1.21 CPU)
Result: FAIL

ASSIGNED

Created attachment 102098 [details] [review]
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: Brimbank Library, Australia

Hi,
patch does not apply, an easy conflict in patrons.pref

Solving that, qa tools little complain

 FAIL   misc/cronjobs/restrict_patrons_with_failed_notices.pl
   FAIL   critic
                "$type" is declared but not used at line 16, column 1. Unused variables clutter code and make it harder to read.
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     spelling
   OK     valid

That new file also lacks a copyright notice!

Running that file always generates some output. It has a verbose option, but not a quiet/silent one. As a cron job it will always send an email with the output, unless you redirects out to /dev/null. 
Why not print something only with '-v' and be silent otherwise (unless there's some error)?

Not fully tested yet.

Created attachment 105003 [details] [review]
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: Brimbank Library, Australia

(In reply to Bernardo Gonzalez Kriegel from comment #6)
> Hi,
> patch does not apply, an easy conflict in patrons.pref
> 
> Solving that, qa tools little complain
> 
>  FAIL   misc/cronjobs/restrict_patrons_with_failed_notices.pl
>    FAIL   critic
>                 "$type" is declared but not used at line 16, column 1.
> Unused variables clutter code and make it harder to read.
>    OK     forbidden patterns
>    OK     git manipulation
>    OK     pod
>    OK     spelling
>    OK     valid
> 
> That new file also lacks a copyright notice!
> 
> Running that file always generates some output. It has a verbose option, but
> not a quiet/silent one. As a cron job it will always send an email with the
> output, unless you redirects out to /dev/null. 
> Why not print something only with '-v' and be silent otherwise (unless
> there's some error)?
> 
> Not fully tested yet.

Have fixed conflict, removed unused variable, added copyright notice, and amended the cron to default to no output unless the verbose option is specified. Ready for testing.

Nice work.

I just wonder about get_failed_notices() method in Koha/Notice/Message.pm.
It does not use current object.
And why using SQL and not the ORM ?

Also :
Koha::Notice::Message::restrict_patron_when_notice_fails($failed_notice);
This is strange.
Method restrict_patron_when_notice_fails() should be called on a Koha::Notice::Message object instance.

I'll work on that ^

Created attachment 116181 [details] [review]
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: Brimbank Library, Australia

Created attachment 158687 [details] [review]
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

When I applied the patch it doesnt't work. I have this error :

Applying: Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail
Using index info to reconstruct a base tree...
M	Koha/Notice/Message.pm
M	installer/data/mysql/mandatory/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
Auto-merging installer/data/mysql/mandatory/sysprefs.sql
Auto-merging Koha/Notice/Message.pm
CONFLICT (content): Merge conflict in Koha/Notice/Message.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail

Created attachment 160785 [details] [review]
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

Aleisha,

There is a YAML problem with patrons.pref, I believe. With this patchset applied when I do a seach for system preferences I get:

YAML::XS::Load Error: The problem:

    did not find expected key

was found at document: 1, line: 391, column: 5
while parsing a block mapping at line: 2, column: 5
 at /kohadevbox/koha/admin/preferences.pl line 51

    in CGI::Compile::ROOT::kohadevbox_koha_admin_preferences_2epl::GetTab at /kohadevbox/koha/admin/preferences.pl line 51 


Also, ,maybe perltidy "restrict_patrons_with_failed_notices.pl" ?

Created attachment 160890 [details] [review]
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

The patch no longer applies 8-(....

git bz apply 23295

Bug 23295 - Automatically debar patrons if SMS or email notice fail

160890 - Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail
Using index info to reconstruct a base tree...
M	Koha/Notice/Message.pm
M	installer/data/mysql/mandatory/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
.git/rebase-apply/patch:130: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
Auto-merging installer/data/mysql/mandatory/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/mandatory/sysprefs.sql
Auto-merging Koha/Notice/Message.pm
CONFLICT (content): Merge conflict in Koha/Notice/Message.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23295: Automatically restrict (debar) patrons when email/sms notices fail

Created attachment 167599 [details] [review]
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

Created attachment 167778 [details] [review]
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>

Testing notes (using KTD):
1. Before step 1 (so can tick the SMS option for triggers and add an SMS notice of ODUE): Do test plan step 2.
2. I checked out items to Mary and Henry (I changed Henry's patron category to Patron).
3. Step 4 - format for date:
   . UPDATE issues SET date_due='2024-06-14 23:59:00' WHERE borrowernumber=49;
4. Steps 5 and 6 - in KTD from ktd--shell run: misc/cronjobs/overdue_notices.pl
5. Steps 15 + 16 - run with -v to see the message:
   ./restrict_patrons_with_failed_notices.pl -v
   There are borrowers with failed SMS or email notices
6. Output from step 18:
   ./restrict_patrons_with_failed_notices.pl -v -c
   There are borrowers with failed SMS or email notices
   Applying restriction to patron 49: Mary Burton
   Applying restriction to patron 19: Henry Acevedo

I think it makes sense to add a new row to restriction_types for this type of restriction. This will also make the messages translatable!

(In reply to Kyle M Hall (khall) from comment #21)
> I think it makes sense to add a new row to restriction_types for this type
> of restriction. This will also make the messages translatable!

Thanks for your review Kyle and your testing David!

I am adding to our atomicupdate to add a new SUSPENSION row to the restriction_types table!

Created attachment 172507 [details] [review]
Bug 23295: (follow-up) Add 'NOTICE_FAILURE_SUSPENSION' as a new system restriction type

Sponsored-by: Catalyst IT

(In reply to Alex Buckley from comment #22)
> (In reply to Kyle M Hall (khall) from comment #21)
> > I think it makes sense to add a new row to restriction_types for this type
> > of restriction. This will also make the messages translatable!
> 
> Thanks for your review Kyle and your testing David!
> 
> I am adding to our atomicupdate to add a new SUSPENSION row to the
> restriction_types table!

Hi Kyle, 

I've attached a follow-up adding a new 'NOTICE_FAILURE_SUSPENSION' system restriction type and assigning that to users whose email or SMS notices fail.

Could you please confirm what I have added is what you had in mind? If not I will follow-up again.

Thanks,
Alex

This looks good! Thanks!

Created attachment 172538 [details] [review]
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>

Created attachment 172539 [details] [review]
Bug 23295: (follow-up) Add 'NOTICE_FAILURE_SUSPENSION' as a new system restriction type

Sponsored-by: Catalyst IT

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 172540 [details] [review]
Bug 23295: (QA follow-up) Fix up qa script issues

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Thanks so much Kyle!

I've reviewed the code and the test plan and found some things:

1) Configuration (question)

get_failed_notices takes days as a parameter, but it's always used with the default (7) or explicitly with 7. 

So it will always check all notices sent in the last 7 days.

Should we include a note in the system preference about this timeframe?


2) Terminology (solved)

say "There are borrowers with failed SMS or email notices" if $verbose;

borrowers => patrons

Fixed in follow-up.


3) Database update (for future reference)

Please submit database updates in a single atomicupdate file (less work for RM who has to combine them manually).

Pushed for 24.11!

Well done everyone, thank you!

We should not try catch without rethrow. Please adjust the db rev: Simply remove the try-catch block.

See bug 37895 for more details.

(In reply to Jonathan Druart from comment #32)
> We should not try catch without rethrow. Please adjust the db rev: Simply
> remove the try-catch block.
> 
> See bug 37895 for more details.

See bug 38394.

(In reply to Jonathan Druart from comment #32)
> We should not try catch without rethrow. Please adjust the db rev: Simply
> remove the try-catch block.
> 
> See bug 37895 for more details.

We can make that change. Should that be attached here or to a new follow-up bug report?

(In reply to Aleisha Amohia from comment #34)
> (In reply to Jonathan Druart from comment #32)
> > We should not try catch without rethrow. Please adjust the db rev: Simply
> > remove the try-catch block.
> > 
> > See bug 37895 for more details.
> 
> We can make that change. Should that be attached here or to a new follow-up
> bug report?

It's done on bug 38394.

However there is still a question from Katrin waiting for an answer.

(In reply to Katrin Fischer from comment #30)
> I've reviewed the code and the test plan and found some things:
> 
> 1) Configuration (question)
> 
> get_failed_notices takes days as a parameter, but it's always used with the
> default (7) or explicitly with 7. 
> 
> So it will always check all notices sent in the last 7 days.
> 
> Should we include a note in the system preference about this timeframe?
> 

Yes that's fair - have reported Bug 38404

(In reply to Aleisha Amohia from comment #36)
> (In reply to Katrin Fischer from comment #30)
> > I've reviewed the code and the test plan and found some things:
> > 
> > 1) Configuration (question)
> > 
> > get_failed_notices takes days as a parameter, but it's always used with the
> > default (7) or explicitly with 7. 
> > 
> > So it will always check all notices sent in the last 7 days.
> > 
> > Should we include a note in the system preference about this timeframe?
> > 
> 
> Yes that's fair - have reported Bug 38404

Have also reported Bug 38405 to make the script take a --days param

added release notes

Enhancement, no 24.05.x backport

Changed status to needs documenting - enhancement to notices, new system preference, new cronjob.