Bug 24197 - Custom destination for failed overdue notices
Summary: Custom destination for failed overdue notices
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Buckley
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 26380
  Show dependency treegraph
 
Reported: 2019-12-09 22:31 UTC by Alex Buckley
Modified: 2021-01-28 16:24 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
This adds a new system preference AddressForFailedEmailNotices that allows to control where the summarized overdue email for patrons without email addresses is sent to.
Version(s) released in:
20.11.00


Attachments
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (3.85 KB, patch)
2019-12-10 01:28 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (5.21 KB, patch)
2019-12-10 05:32 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (5.26 KB, patch)
2019-12-10 05:37 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (6.06 KB, patch)
2020-04-21 23:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 24197: Changed use of 'branch' terminology to 'library' (4.49 KB, patch)
2020-04-22 00:04 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (6.09 KB, patch)
2020-04-22 12:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24197: Changed use of 'branch' terminology to 'library' (4.53 KB, patch)
2020-04-22 12:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (6.12 KB, patch)
2020-09-02 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24197: Changed use of 'branch' terminology to 'library' (4.56 KB, patch)
2020-09-02 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24197: (QA follow-up) Embelished syspref description (4.75 KB, patch)
2020-09-02 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to. (6.21 KB, patch)
2020-09-02 17:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24197: Changed use of 'branch' terminology to 'library' (4.64 KB, patch)
2020-09-02 17:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24197: (QA follow-up) Embelished syspref description (4.83 KB, patch)
2020-09-02 17:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24197: (QA follow-up) Shorten syspref name to AddressForFailedEmailNotices (6.35 KB, patch)
2020-09-02 17:34 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2019-12-09 22:31:25 UTC
Currently failed overdue notices are sent to the branch email , or when that is not set to the KohaAdminEmailAddress syspref. 

Several of our clients have requested a seperate email address for failed overdue notices to be sent to. This email address will be defined in the: RedirectAddressForFailedOverdueNotices syspref.
Comment 1 Alex Buckley 2019-12-10 01:28:44 UTC
Created attachment 96123 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set an email in KohaAdminEmailAddress and notice there is no syspref
named RedirectAddressForFailedOverdueNotices

2. Set some invalid email addresses in patron records then checkout
items to those patrons making the due date in the past

3. Manually run overdue_notices.pl

4. Check the message_queue database table and notice the patrons with
invalid email addresses are in an email starting with the content:
'These messages were not sent directly to the patrons.'

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the KohaAdminEmailAddress
syspref.

7. Repeat steps 2-4 and notice that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Comment 2 Alex Buckley 2019-12-10 05:32:25 UTC
Created attachment 96124 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set an email in KohaAdminEmailAddress and notice there is no syspref
named RedirectAddressForFailedOverdueNotices

2. Set some invalid email addresses in patron records then checkout
items to those patrons making the due date in the past

3. Manually run overdue_notices.pl

4. Check the message_queue database table and notice the patrons with
invalid email addresses are in an email starting with the content:
'These messages were not sent directly to the patrons.'

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the KohaAdminEmailAddress
syspref.

7. Repeat steps 2-4 and notice that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Comment 3 Alex Buckley 2019-12-10 05:37:58 UTC
Created attachment 96125 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set an email in KohaAdminEmailAddress and notice there is no syspref
named RedirectAddressForFailedOverdueNotices

2. Set some invalid email addresses in patron records then checkout
items to those patrons making the due date in the past

3. Manually run overdue_notices.pl

4. Check the message_queue database table and notice the patrons with
invalid email addresses are in an email starting with the content:
'These messages were not sent directly to the patrons.'

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the KohaAdminEmailAddress
syspref.

7. Repeat steps 2-4 and notice that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Comment 4 Alex Buckley 2020-04-21 23:57:25 UTC
Thanks for tagging this 20.05 candidate Martin, I've just applying the patch and it doesn't apply on master. 

Attaching a corrected patch now.
Comment 5 Alex Buckley 2020-04-21 23:58:16 UTC
Created attachment 103428 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set no email addresses in patron record then checkout
items to the patron making the due date in the past

2. Visit patron's home library page: Administration > Libraries
Set a branch email and in global system preferences observe there is no syspref
named RedirectAddressForFailedOverdueNotices

3. Manually run overdue_notices.pl

4. Check the message_queue database table and observe there is a
print overdue notice and a email notice with to_address of the
branch email address

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the branch email.

7. Repeat steps 1,3,4 and observe that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Comment 6 Alex Buckley 2020-04-22 00:04:40 UTC
Created attachment 103429 [details] [review]
Bug 24197: Changed use of 'branch' terminology to 'library'

Sponsored-by: Catalyst IT
Comment 7 Alex Buckley 2020-04-22 00:05:26 UTC
All ready to test :)
Comment 8 Martin Renvoize 2020-04-22 12:26:27 UTC
Created attachment 103465 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set no email addresses in patron record then checkout
items to the patron making the due date in the past

2. Visit patron's home library page: Administration > Libraries
Set a branch email and in global system preferences observe there is no syspref
named RedirectAddressForFailedOverdueNotices

3. Manually run overdue_notices.pl

4. Check the message_queue database table and observe there is a
print overdue notice and a email notice with to_address of the
branch email address

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the branch email.

7. Repeat steps 1,3,4 and observe that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-04-22 12:26:30 UTC
Created attachment 103466 [details] [review]
Bug 24197: Changed use of 'branch' terminology to 'library'

Sponsored-by: Catalyst IT
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2020-04-22 12:27:20 UTC
Works as expected, passes QA scripts.. Signing Off
Comment 11 Katrin Fischer 2020-08-16 10:06:44 UTC
The preference description is not correct for libraries using the *replyto functionality to avoid having the emails marked as spam.

See here:
-    my $branch_email_address = $library->inbound_email_address;
+    my $branch_email_address = C4::Context->preference('RedirectAddressForFailedOverdueNotices')
+      || $library->inbound_email_address;

The inbound_email_address is not the KohaAdminEmailAdress or branchemail in htese cases.

Can you please check and adapt the wording?
Comment 12 Martin Renvoize 2020-09-02 15:34:28 UTC
Created attachment 109549 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set no email addresses in patron record then checkout
items to the patron making the due date in the past

2. Visit patron's home library page: Administration > Libraries
Set a branch email and in global system preferences observe there is no syspref
named RedirectAddressForFailedOverdueNotices

3. Manually run overdue_notices.pl

4. Check the message_queue database table and observe there is a
print overdue notice and a email notice with to_address of the
branch email address

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the branch email.

7. Repeat steps 1,3,4 and observe that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2020-09-02 15:34:31 UTC
Created attachment 109550 [details] [review]
Bug 24197: Changed use of 'branch' terminology to 'library'

Sponsored-by: Catalyst IT
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2020-09-02 15:34:35 UTC
Created attachment 109551 [details] [review]
Bug 24197: (QA follow-up) Embelished syspref description

As requested, I have embelished the syspref description to more
accurately depict the fallback sequence for email delivery.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2020-09-02 15:35:09 UTC
Patch rescued, ready for second QA run.
Comment 16 Nick Clemens 2020-09-02 17:34:06 UTC
Created attachment 109552 [details] [review]
Bug 24197: Added new local use syspref which sets the email address that failed overdue notices are set to.

Test plan:
1. Set no email addresses in patron record then checkout
items to the patron making the due date in the past

2. Visit patron's home library page: Administration > Libraries
Set a branch email and in global system preferences observe there is no syspref
named RedirectAddressForFailedOverdueNotices

3. Manually run overdue_notices.pl

4. Check the message_queue database table and observe there is a
print overdue notice and a email notice with to_address of the
branch email address

5. Apply patch

6. Run database update:
cd installer/data/mysql
sudo koha-shell <instance_name>
./updatedb.pl

7. Confirm there is a new system preference named:
RedirectAddressForFailedOverdueNotices

Give it a different email address to that in the branch email.

7. Repeat steps 1,3,4 and observe that the failed overdue notices have been
sent to the email defined in RedirectAddressForFailedOverdueNotices

Sponsored-By: Catalyst IT
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2020-09-02 17:34:11 UTC
Created attachment 109553 [details] [review]
Bug 24197: Changed use of 'branch' terminology to 'library'

Sponsored-by: Catalyst IT
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Nick Clemens 2020-09-02 17:34:14 UTC
Created attachment 109554 [details] [review]
Bug 24197: (QA follow-up) Embelished syspref description

As requested, I have embelished the syspref description to more
accurately depict the fallback sequence for email delivery.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 Nick Clemens 2020-09-02 17:34:18 UTC
Created attachment 109555 [details] [review]
Bug 24197: (QA follow-up) Shorten syspref name to AddressForFailedEmailNotices

If you define the address we use it, if not we fallback,
it's not really a redirect, and that just makes the name longer.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Nick Clemens 2020-09-02 17:41:37 UTC
Last patch can be accepted or ignored, just trying to keep pref names under control :-)
Comment 21 Jonathan Druart 2020-09-03 12:10:15 UTC
Not blocker but: shouldn't have it been defined at library level instead?
Comment 22 Jonathan Druart 2020-09-03 13:04:30 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 23 Martin Renvoize 2020-09-03 17:15:37 UTC
(In reply to Jonathan Druart from comment #21)
> Not blocker but: shouldn't have it been defined at library level instead?

I did wonder that myself when testing, but then decided that we have a pattern of introducing a system level default and then enhancing later with library level refinement if it proves useful.

I'm still keen to overhaul how library emails get associated at some point but didn't think that should hold this piece up either.
Comment 24 Lucas Gass 2020-09-14 21:54:33 UTC
enhancement will not be backported to 20.05.x series