Bug 15967

Summary: Print notices are not generated if the patron cannot be notified
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: NoticesAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: brendan, f.demians, gmcharlt, gwilliams, julian.maurice, katrin.fischer, nick
Version: masterKeywords: regression
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14133    
Bug Blocks: 23411    
Attachments: Bug 15967: Fix regression from bug 14133 - notify the library if patron is not
Bug 15967: Fix regression from bug 14133 - notify the library if patron is not
Bug 15967: Use the email template if the print template does not exist
Bug 15967: Use the email template if the print template does not exist
[PASSED QA] Bug 15967: Fix regression from bug 14133 - notify the library if patron is not
[PASSED QA] Bug 15967: Use the email template if the print template does not exist

Description Jonathan Druart 2016-03-03 08:57:27 UTC
Regression introduced by bug 14133, see but 14133 comment 13.
Comment 1 Jonathan Druart 2016-03-03 09:01:08 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2016-03-04 02:00:33 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2016-04-03 20:50:39 UTC
I am sorry, but this doesn't appear to work for me.

My test:
- A patron from Midway, has 1 checkout, 1 day overdue
- No holidays in the calendar
- Patron has neither SMS nor email
- Overdue notice trigger for Midway is set to delay 1
- Koha admin email and branch email are set
- Running overdue_notices.pl - no email to the library is generated
- Add email address to patron
- Running overdue_notices.pl - generates email notice.
Comment 4 Katrin Fischer 2016-04-03 20:52:18 UTC
I should note: without an email address no notice is generated at all. Neither the print notice as a fallback, nor the email to the library.
Comment 5 Jonathan Druart 2016-04-04 07:45:31 UTC
(In reply to Katrin Fischer from comment #3)
> I am sorry, but this doesn't appear to work for me.
> 
> My test:
> - A patron from Midway, has 1 checkout, 1 day overdue
> - No holidays in the calendar
> - Patron has neither SMS nor email
> - Overdue notice trigger for Midway is set to delay 1

What MTT is/are checked?
Comment 6 Katrin Fischer 2016-04-04 07:46:45 UTC
Only e-mail was checked.
Comment 7 Jonathan Druart 2016-04-04 08:05:12 UTC
It seems that the test plan is wrong.
The print notice is generated even without this patch.

The issue is that the email generated for the library is not generated.
This email starts with "These messages were not sent directly to the patrons" and should be generated if an email/sms has not been sent to a patron (but a print should have been built instead).
Comment 8 Katrin Fischer 2016-04-04 08:07:53 UTC
With and without the patch and without e-mail my message_queue remained empty - I can try to retest, but a bit out of ideas.
Comment 9 Jonathan Druart 2016-04-04 08:15:01 UTC
(In reply to Katrin Fischer from comment #8)
> With and without the patch and without e-mail my message_queue remained
> empty - I can try to retest, but a bit out of ideas.

Define a print template for the letter you use :)
Comment 10 Katrin Fischer 2016-04-04 08:47:29 UTC
I can do that, to get this bug moving, but it should not be mandatory.
I just checked some of our 3.18.x installations and the notice to the library is generated without an existing print template. It seems to me like the fallback is broken/the behaviour was changed.
Comment 11 Jonathan Druart 2016-04-04 10:39:34 UTC Comment hidden (obsolete)
Comment 12 Nick Clemens 2016-04-04 14:51:03 UTC
Just adding this from the other bug , I think fallback to email is a fine option but wanted to mention this in case it seems worthwhile:
Maybe we could fallback to just sending the library a notice saying "No email for patron XXX - define print notice if you would like to receive printable notices instead of this message"?  Or something along those lines?
Comment 13 Nick Clemens 2016-04-04 15:25:23 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2016-04-04 20:51:10 UTC
Created attachment 49875 [details] [review]
[PASSED QA] Bug 15967: Fix regression from bug 14133 - notify the library if patron is not

Regression introduced by bug 14133, see but 14133 comment 13.

Test plan:
Without this patch applied, if a patron cannot be notified (no email
address or sms number), the print notice generated for the library was
not.
With this patch applied, the print notice should be generated using the
print template

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2016-04-04 20:51:38 UTC
Thx a lot!
Comment 16 Katrin Fischer 2016-04-04 20:52:13 UTC
Created attachment 49876 [details] [review]
[PASSED QA] Bug 15967: Use the email template if the print template does not exist

From commit 581759e985c170db0edb4a895cda641930e5ac11
 Bug 14133: Print notices should be generated with the print
 template

"""
IMPORTANT NOTE: This test plan does not take into account the notices
generated for the staff ("These messages were not sent directly to the
patrons."). However the behavior will also change, the print template
will be used in all cases. Is it what we want?
"""

Yes, it is what we want. But if the print template does not exist, the
notice is not generated, we'd like to get the email template instead.

Test plan:
- Remove the print template for the letter you use for overdues
- Define an overdue rule to send an email
- Remove the email address for the patron which has overdues
- Execute the overdue_notices script
The staff should get an email notice and a print notice (using the
email template) should be generated for the patron

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Brendan Gallagher 2016-04-05 15:15:57 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!
Comment 18 Julian Maurice 2016-04-06 13:02:14 UTC
Patches pushed to 3.22.x, will be in 3.22.6
Comment 19 Frédéric Demians 2016-04-27 15:41:50 UTC
Pushed to 3.20.x, will be in 3.20.11.