Bug 10621 - Subscription alert emails have wrong from-address
Summary: Subscription alert emails have wrong from-address
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Mirko Tietgen
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-21 15:14 UTC by Mirko Tietgen
Modified: 2014-12-07 20:03 UTC (History)
6 users (show)

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


Attachments
Bug 10621 Subscription alert emails have wrong from-address (1.38 KB, patch)
2013-07-23 21:26 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 10621 Subscription alert emails have wrong from-address (1.94 KB, patch)
2013-07-23 21:37 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 10621 Subscription alert emails have wrong from-address (1.99 KB, patch)
2013-07-24 07:22 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10621 Subscription alert emails have wrong from-address (2.09 KB, patch)
2013-07-25 11:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mirko Tietgen 2013-07-21 15:14:57 UTC
The email addresses for 'to' and 'from' are the same for subscription alerts. They both contain the patron's mail address (which is the actual to-adress). Depending on you MTA/SMTP settings, this will cause errors instead of sending emails.
Comment 1 Mirko Tietgen 2013-07-23 21:26:39 UTC Comment hidden (obsolete)
Comment 2 Mirko Tietgen 2013-07-23 21:37:37 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2013-07-24 07:22:09 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2013-07-25 11:27:46 UTC
I was following your test plan, but it looks like there is some bug in sending alerts that makes your test plan fail (although it is (far) outside the scope of your report).

Just following the code, I note the following:
Serials/ModSerialStatus calls Letters/SendAlerts
Letters/SendAlerts calls getalert to get a list of email addresses for that subscription.
Sub getalert does however look in table alert (it is empty in my case), and does not send anything.
Note that the routing list is found in table subscriptionroutinglist (that was not empty for me).

So I cannot fail this patch, but the test plan does not work out for me. So I would like to ask patch author and signer: How did you manage to test this? Because we stumble over another bug.. How is it possible that existing code here looks for email addresses in the wrong table??

Thanks.
Comment 5 Marcel de Rooy 2013-07-25 11:30:35 UTC
I realize now that I was not subscribing via opac but via staff. Will have another try :)
Comment 6 Marcel de Rooy 2013-07-25 11:34:59 UTC
(In reply to M. de Rooy from comment #5)
> I realize now that I was not subscribing via opac but via staff. Will have
> another try :)

And of course, that works fine ;)
Well, at least learned something more about the code here.
Comment 7 Marcel de Rooy 2013-07-25 11:39:03 UTC
QA Comment:
Simple patch. Have been testing this very well :) with lots of arriving issues, and debugging statements..
Looks good to me now.
No complaints from koha-qa.

Passed QA
Comment 8 Marcel de Rooy 2013-07-25 11:39:25 UTC
Created attachment 19945 [details] [review]
Bug 10621 Subscription alert emails have wrong from-address

From-address and to-address were the same (patron's email) for subscription alerts. This patch changes 'from' the branch or kohaadminemailaddress

To test
- add a subscription in staff/serials in case you don't have any
- enable patron notifications or the subscription
- in the OPAC, subscribe to the serial
- in staff/serial, receive an issue of the serial

Before applying the patch, the email that is supposed to be sent has the patron's email as 'from' and 'to' (and is likely to fail).

If you follow the steps after applying the patch, the email alert should have the 'from' address of the patron's branch or kohaadminemiladdress -- which should also work fine with the MTA/SMTP you have set up for messaging.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Galen Charlton 2013-07-25 13:56:08 UTC
Pushed to master.  Thanks, Mirko!
Comment 10 Tomás Cohen Arazi 2013-08-16 15:57:06 UTC
This patch has been pushed to 3.12.x, will be in 3.12.4.

Thanks Mirko!
Comment 11 Bernardo Gonzalez Kriegel 2013-09-13 22:51:59 UTC
Pushed to 3.10.x, will be in 3.10.11
Comment 12 Chris Hall 2013-09-17 07:52:16 UTC
Pushed to 3.8.x, will be in 3.8.18