Bug 15967 - Print notices are not generated if the patron cannot be notified
Summary: Print notices are not generated if the patron cannot be notified
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords: regression
Depends on: 14133
Blocks: 23411
  Show dependency treegraph
 
Reported: 2016-03-03 08:57 UTC by Jonathan Druart
Modified: 2020-02-24 14:38 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15967: Fix regression from bug 14133 - notify the library if patron is not (3.37 KB, patch)
2016-03-03 09:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15967: Fix regression from bug 14133 - notify the library if patron is not (3.43 KB, patch)
2016-03-04 02:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15967: Use the email template if the print template does not exist (2.53 KB, patch)
2016-04-04 10:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15967: Use the email template if the print template does not exist (2.59 KB, patch)
2016-04-04 15:25 UTC, Nick Clemens
Details | Diff | Splinter Review
[PASSED QA] Bug 15967: Fix regression from bug 14133 - notify the library if patron is not (3.50 KB, patch)
2016-04-04 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15967: Use the email template if the print template does not exist (2.65 KB, patch)
2016-04-04 20:52 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.