Pick which mail to use based on user.mail being more reliable than user.init being more reliable than comment.mail

Hi,

I used the "init" field back then as I was sure that this is set all the time,
(AKA initial signup)

I also think that "mail" is more intuitive and should be used when possible,
but I couldnt find any documentation on that so far... did you?

thx christoph

I feel that an ifnull statement in the query will do it...

if the .mail field is not set, we use the .init field ... that should do it,
but I still want to give it a 2nd thought as I remember I spent some time digging in other codes
for finding the right fields to use

This is a security leak. It is quite possible that information which should not be seen will be sent to the email address in the init field, when the owner of that address no longer has access to the site.

huh? can you explain me that "security leak" in detail?

the init email will receive notifications of follow up emails,
nothing else.

So worst case you used the email of a friend,
did never change that email to your own's and then post some comments
and your friends gets notified about follow up comments

where's the problem except that the USER forgot to put in his own mail???

the ifnull solution specified above will be shipped in the 5.x version

Sorry to open this again after so long, but I just ran into this bug!

I have an old site, started out in drupal 4.4 or something many years ago and has been upgraded as the years passed. The original email address I used when installing drupal long since became defunct - I haven't used it for over 5 years! But this is the email address drupal has correctly stored in the users table's 'init' field.

I have of course updated both the site email address and my own email address on my user profile, but comment_notify is not sending notifications to those. Only to the ancient non existent 'init' address.

I do receive the notifications by a roundabout route – rejected by the receiving server 'no such account' they bounce back to my server and are routed 'failed to deliver' to me as sysadmin. But I presume other people may well not be seeing notifications and be blithely unaware of it.
I'd also assume some site admins get returned undeliverable email even when users have kept their address up to date.

Line 441 in comment_notify.module seems to be where the trouble starts.

Something missing in the old discussion above is that there is a UI to set and change the users current email address - there is no UI to change the init email address, the only way to do it is in the database, and very carefully...

The code does open the init - I'm not sure exactly why, it was a legacy thing - but then it says "is the user->mail populated? if so use it, if not use the init."

$umail = empty($alert->umail) ? $alert->uinit : $alert->umail;
$mail = empty($alert->cmail) ? $umail : $alert->cmail;

The next line may actually be your problem. There it uses the mail from the comment (which may be very out of date as well) regardless of whether the person has registered and now has a new e-mail on the $user.

I could see changing
$mail = empty($alert->cmail) ? $umail : $alert->cmail;

to something more like checking if we have an address, and only if not do we use the comment mail.

$mail = empty($umail) ? $alert->cmail : $umail;

We should also check that after all this work we have a real mail address...

Would that help you?

We should also check that after all this work we have a real mail address...

You can probably use valid_email_address() to check validity of email address:
http://api.drupal.org/api/function/valid_email_address/6

Just looking more closely at my issue...

I think the check just needs to be "do we even have a string?"

Ah yes, that's the issue. Some of the earlier comments I made (on a blog post I'd written) were posted before I updated the email address on my profile, so some comments on the post have the right email address, some don't – in comments table's 'mail' column.

I assume if someone hits 'reply' on one of the earlier comments comment_notify will pick up the address from the comment being replied too? It's hard to see how you can get around stale email addresses in that situation.

It looks like your suggestion might fix it, but I'm barely following the code....

yeah, assuming comment module checks for a valid email when the comment is made...

Ok, here's a patch.

Certainly user.module and comment.module both do their own validation as they accept users and comments, so yes I'm relying on them for validation of the strings.

better title.

Thanks. Might take me a day or two to test that properly, but I will. :)

Apologies, it's taken me a couple of months to get around to testing this patch.
But testing was not without issues.
I can say that comment notify seems to work perfectly well with the patch. I can't yet say whether it fixes my issue because I haven't been able to replicate it yet!

I set up a new site and posted a bunch of comments from different browsers with different accounts, including anonymous, and different email addresses. I also changed the account addresses - everything worked perfectly even before applying that patch.

Taking a closer look at the new site database comment table - I see the mail field is empty, except for the instance where anonymous commenters leave their email address. This makes perfect sense - if you have a user account why not just use that mail instead of recording their mail address in the comment table. Looking at the site where I have problems I notice more recent comments by logged in users also have no mail recorded in the comment table. So it seems the behaviour of comment module itself has changed recently.

What that means is that my issue is not just an edge case but also only going to happen on legacy comments.

I'm about tweak a clone of the problem site to see if I can check whether your patch has fixed the issue.

Well deleting comments from a clone of the problem site resulted in comment notifications not working at all. (didn't want to spam legit users with test messages so deleted there comments).

I tried setting a mail for a comment directly in the db, to replicate what happened with the legacy comments. Without the patch, the notification goes to the mail in the comment table mail field, as before. So finally replicated the original problem.

After applying the patch the person whose comment is being replied too doesn't get a notification. So I don't think the patch is working. Damned if I can see why.

I'm starting not to care though. It seems this is only an issue for legacy comments anyway, I'm leaning towards just tweaking my old comments in the db and dropping this issue.

Here's a better status.

Given that nobody's run into this in a long time I feel like it's not worth the added complexity.