Bug 6302 - Failing to send email notices - to_address not set
Summary: Failing to send email notices - to_address not set
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Target Milestone: ---
Assignee: Katrin Fischer
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-05 05:06 UTC by Nicholas van Oudtshoorn
Modified: 2013-12-05 19:57 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Simple patch to fix the issue (562 bytes, application/octet-stream)
2011-05-05 05:06 UTC, Nicholas van Oudtshoorn
Details
Bug 6302: Separate AutoEmailPrimaryAdress and AutoEmailOpacUser (1.65 KB, patch)
2011-12-02 06:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6302: Separate AutoEmailPrimaryAdress and AutoEmailOpacUser (1.72 KB, patch)
2011-12-06 16:14 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Follow up for updating queue (1.38 KB, patch)
2011-12-14 16:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6302: Separate AutoEmailPrimaryAdress and AutoEmailOpacUser (1.81 KB, patch)
2011-12-15 09:42 UTC, Koustubha Kale
Details | Diff | Splinter Review
[SIGNED-OFF] 6302 follow up (1.44 KB, patch)
2011-12-15 09:45 UTC, Koustubha Kale
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicholas van Oudtshoorn 2011-05-05 05:06:24 UTC Comment hidden (obsolete)
Comment 1 Katrin Fischer 2011-05-05 06:00:30 UTC
We are using 3.2.2 with overdue notices in production. One of our libraries started with fines and overdue notices at the beginning of this week. They told me overdue notices were successfully sent out, one to the librarian for testing even, and I saw nothing wrong in the database. I am wondering if this problem might be related to configuration?
Comment 2 Magnus Enger 2011-10-26 07:56:34 UTC
Changing the status to "PATCH-Sent" and "Needs Signoff".
Comment 3 Katrin Fischer 2011-10-26 08:07:27 UTC
I am still wondering about this. If the to_address is no set - afaik - the address will be taken from the patron account before the notice is sent out. We have a lot of libraries using notices with 3.2.2
Comment 4 Katrin Fischer 2011-11-04 21:25:04 UTC
There was some discussion on the mailing list about this. 
One hint is to check the setting for AutoEmailPrimaryAddress. 
The to_address will be populated once the process_message_queue process was run.

http://lists.katipo.co.nz/pipermail/koha/2011-November/031064.html

I tend to mark this INVALID - ok?
Comment 5 Nicholas van Oudtshoorn 2011-11-08 01:45:41 UTC
Have tested, and it does work as noted in Comment 4 when the  AutoEmailPrimaryAddress is set to "first valid".

A few suggestions though:

1) That the system preference "AutoEmailPrimaryAddress" be made distinct from the "AutoEmailOpacUser" prefernce. I don't want to send an email to newly created patrons with their account details, but I *do* want to send them overdue notices. (Under 3.4.2 the preferences seem to be joined together?)

2) That process_message_queue should update the "to_address" field in the "message_queue" table upon sending the email. That way, there's a historical record of which email address the notice was actually sent to (even if the email address for a user is changed in the future)
Comment 6 Chris Cormack 2011-12-01 18:58:55 UTC
I agree with Comment 5, I will switch this from patch sent, and hopefully we will get a patch to do both 1 and 2 in comment 5
Comment 7 Katrin Fischer 2011-12-02 06:56:04 UTC Comment hidden (obsolete)
Comment 8 Nicole C. Engard 2011-12-06 16:14:33 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2011-12-07 13:08:12 UTC
If I am not mistaken, this patch only resolves the first point of Comment 5 and is actually just a cosmetic change.
The second point refers to updating the to_address in sub _send_message_by_email in Letters.pm.

I would propose to put this report into hold until we can also consider the patch solving the second point. The first patch is OK, but setting the status to Failed QA in order to reflect the on hold status.
Comment 10 Katrin Fischer 2011-12-07 13:10:05 UTC
Hi Marcel, 
is there a reason the 'cosmetic' change can not go in now? It would make it easier for people to get their configuration right and send out mails.
Comment 11 Marcel de Rooy 2011-12-07 13:12:42 UTC
(In reply to comment #10)
> Hi Marcel, 
> is there a reason the 'cosmetic' change can not go in now? It would make it
> easier for people to get their configuration right and send out mails.

Hi Katrin,
It may speed up receiving the second patch ;)
But if it would really help, I do not oppose pushing it.
Comment 12 Katrin Fischer 2011-12-07 14:41:31 UTC
I don't think I will have time to work on the second part of the patch. :(
Comment 13 Marcel de Rooy 2011-12-07 16:08:05 UTC
(In reply to comment #12)
> I don't think I will have time to work on the second part of the patch. :(

Will take a look.
Comment 14 Marcel de Rooy 2011-12-14 16:08:33 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2011-12-14 16:09:37 UTC
Second patch needs signoff ! Please test..
Comment 16 Koustubha Kale 2011-12-15 09:42:31 UTC
Created attachment 6806 [details] [review]
[SIGNED-OFF] Bug 6302: Separate AutoEmailPrimaryAdress and AutoEmailOpacUser

AutoEmailPrimaryAddress is not only used for the patron account mails, but
also for overdues and other email notices to patrons.

This patch separates the system preferences in the system preference editor.

To test:
- Check Patrons tab shows both system preferences separated
- Check you can still change the settings

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>
Comment 17 Koustubha Kale 2011-12-15 09:45:38 UTC
Created attachment 6807 [details] [review]
[SIGNED-OFF] 6302 follow up

Updates to_address in message queue when sending an email (when needed).

Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>
Comment 18 Koustubha Kale 2011-12-15 09:49:12 UTC
(In reply to comment #16)
> Created attachment 6806 [details] [review]
> [SIGNED-OFF] Bug 6302: Separate AutoEmailPrimaryAdress and AutoEmailOpacUser
> 
> AutoEmailPrimaryAddress is not only used for the patron account mails, but
> also for overdues and other email notices to patrons.
> 
> This patch separates the system preferences in the system preference editor.
> 
> To test:
> - Check Patrons tab shows both system preferences separated
> - Check you can still change the settings
> 
> Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
> Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>

1) Verified that prior to applying patch one the System preferences AutoEmailOpacUser and AutoEmailPrimaryAddress were combined.
2) after applying first patch, verified that there were two separate system preferences for AutoEmailOpacUser and AutoEmailPrimaryAddress
Comment 19 Koustubha Kale 2011-12-15 09:50:30 UTC
(In reply to comment #17)
> Created attachment 6807 [details] [review]
> [SIGNED-OFF] 6302 follow up
> 
> Updates to_address in message queue when sending an email (when needed).
> 
> Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>

3) Verified that before applying patch two, after running process_message_queue.pl, the to_address field in table message_queue had null for Item Overdue notice. 
4) Verified that after applying patch two , after running process_message_queue.pl, the to_address field in table message_queue had patrons email address in it.
Comment 20 Paul Poulain 2011-12-16 17:11:23 UTC
QA comment: both patches are small, good code, no specific comment
no change in perlcritic (19 warnings before, 19 after applying the patch)

test plan: see comment 5 to understand what those patches are doing

passed QA

patches pushed, please test