Sending emails for issues that are auto-closed

We also shouldn't be sending out emails when issues are automatically closed--at least we never have in the past.

Gerhard says this is important to look into before the upgrade, and he will look into it.

I run cron.php on a testsite and apparently the mails are totally messed up:

From www-data@dosprint.org Fri Feb 13 09:47:59 2009
Return-Path:
X-Original-To: 13
Delivered-To: 13@dosprint.org
Received: by theming.dosprint.org (Postfix, from userid 33)
id C0A924A58C8D; Fri, 13 Feb 2009 09:22:14 +0000 (UTC)
To: Release@dosprint.org, critical@dosprint.org, bugs@dosprint.org,
for@dosprint.org, February@dosprint.org, 13@dosprint.org,
2009@dosprint.org
Subject:
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes
Content-Transfer-Encoding: 8Bit
X-Mailer: Drupal
Errors-To: info@drupal.org
Sender: info@drupal.org
Reply-To: Array@dosprint.org
From: Array@dosprint.org
Message-Id: <20090213092214.C0A924A58C8D@theming.dosprint.org>
Date: Fri, 13 Feb 2009 09:22:14 +0000 (UTC)

Also, there is no body to the mail.

Since this happens on a cron run, I assume this is limited to digest & reminder mails only (or does this affect the "regular" mail notifications too?)

I rewrote part of the mail handling in #361651: Port project_issue to use the D6 drupal_mail() but only addressed project_issue/mail.inc. In hindsight the aforementioned issue should also have addressed the mail code in project_issue/cron.php.

here's a first stab at converting the digest and reminder stuff to the 6.x mail API -- totally untested. would be great if i could get a code review of this so i know i'm handling things appropriately. the reminder code is kind of convoluted, so instead of rewriting it i simply embraced it and stuffed the generated body code into a param ;)

also in the process of upgrading, i caught a performance enhancement: we were doing a ton of user_load()'s just to pull assigned names, so i integrated that into the queries instead.

The mail sending code looks good. The original issue was however that they got sent at all. How did the patch affects that?

OMG, don't know how this recategorization happened, sorry.

attached patch has been tested and produces the following output:

DIGEST:

REMINDER:

this all looks ok to me. can i get an amen?

Looks all good!

@aclight, hunmonk, et al: Generating emails during auto-close has been happening in D5 for a while. I'm not sure if we ever had code specifically to prevent it. AFAIK, the only way you don't get them in D5 (if you subscribe at all), is via the settings at node/N/edit/issues -- if you use "Issues e-mail address:" and select some checkboxes for which states you want email for. So long as you select something in there, but don't select "closed", you won't see auto-close emails.

I just tested all this again on my local D5 test install, and that's definitely how it works. Yes, there are all sorts of problems and limitations with the current subscription functionality, but this is a red-herring in terms of the D6 upgrade and deployment on d.o tomorrow. The main thing is fixing the bugs with the D6 mail API which hunmonk's patch is supposed to fix. I'll review that closely next, stay tuned.

Ok, then we might have perceived having some functionality which was not there. No worries with that then.

@dww: As far as I can recall I have never received an email when an issue was automatically closed. I subscribe to all issues on all the project* queues and my issues for pretty much all other projects, so certainly many issues I've been subscribed to were automatically closed yet I got no email message.

It sounds to me like you're talking about subscribing to issues via node/N/edit/issues, not via the standard subscriptions page, which is how I have subscribed to issues.

On the same day Gerhard posted this issue I also got an email for one issue and the email was informing me that the issue had just been automatically closed. I haven't gotten one since, so if sending of issues on d6.drupal.org is enabled and cron is running it seems that there must not be a problem. However if cron has not been running there could be a problem as it would be an (annoying, IMHO) change in behavior to start getting emails for issues as they are automatically closed.

http://drupal.org/node/367162 just got closed automatically half an hour ago. I am subscribed via adding a (useful :) comment, but I did not get an email, so this just proved as a test case of people not getting email on closed issues.

@Gabor: You're "subscribed" via the infrastructure email list, right?

Behold:
http://drupal.org/node/107028/edit/issues

The fact you got no email is exactly the case I described in #10.

Oh, hrm, I guess not:

This issue is completely baffling to me. I've tested locally, on both D5 and D6, and I get the emails for autoclose if I use "normal" subscription via project/issues/subscribe-mail. There are no locally applied patches on d.o to project_issue that have anything to do with this, yet it seems like auto-close doesn't generate emails for regular subscriptions there. With d6.d.o currently down, I'm having trouble testing this to see what's actually happening there...

with the upgrade lingering tomorrow, how do we proceed here?

i'd like to commit the patch in #8 to resolve the digest/reminder issue -- i'm feeling pretty confident that it fixes those issues from my local testing.

for the auto-close emails, we can:

1. discuss and implement a full solution now.
2. hack d.o locally to prevent the problem until we have a solution.
3. just let the problem be a problem until we have a solution.

#1 seems unrealistic for tomorrow's deployment. i can live with #3, but i'm guessing a lot of others would have issues with it, so i'm recommending #2 unless somebody has a better idea. i think a fairly simple hack would be:

1. stuff something into the comment array like $comment['no_mail'] = TRUE.
2. look for this in the comment insert code and skip the issue emailing if it's found.

that hack would probably take about five minutes to write.

We sure need your patch to be committed first. I'm fine with #2 for the mail sending for now.

This patch always generates the weekly critical digest emails, even if there are no open critical issues, due to the way the logic for what to send is now totally separate from the logic for if it should be sent. I found a few other minor problems (wrong comment, stuff that wasn't clear that needed a comment, fixed a query by using db_placeholders() instead of directly stuffing variables into SQL (no, it's not SQL injection, but it's still clumsy and more fragile/risky as the code changes). I'm re-rolling now...

http://drupal.org/cvs?commit=172528 to HEAD fixes everything from #8 that I pointed out in #19.

Setting this back to active for the remaining topic of either fixing or hacking something about auto-close emails for the upgrade. We should test it on d6.d.o to see if it's actually trying to send them or not as the first step before anyone tries to write any code for this. I hope someone else can pick up that torch, I've got other critical issues to deal with and I really need sleep...

http://drupal.org/cvs?commit=172599 handles this issue temporarily. we'll need a better long-term fix.

Adding 6.x-1.0 blocker term based on the commit message for http://drupal.org/cvs?commit=172599

Subscribing. I'm trying to figure out what the real issue is here. I haven't been receiving email for issues that are auto-closed on drupal.org -- does that mean this is fixed, not-fixed, or hacked and needs a better fix?

I'm honestly not sure. ;) I haven't tried to reproduce this off *.d.o. Do you have a project* test site that sends email where you could see how it's currently behaving? That'd help narrow the scope of this issue. Thanks!