add project_issue_reply_to setting and use that as the From address of all issue e-mails

hunmonk and i had a long discussion about this earlier today. this issue is certainly a can of worms. here are the results of our discussion and implications for this patch:

A) that project setting is *only* used as the from address. once we're not using it for the from address at all, it should just be removed from the code entirely:
- DB update to drop the column from the {project_projects} table
- remove all traces of it on the project edit/create form

B) It would be nice to have a way to have project-specific email addresses, e.g. to facilitate mailhander installations on sites that want different IMAP folders for each project (e.g. to make it easier to generate new issues via email, etc). If we remove the per-project setting, that'd be harder. However, I came up with a nice solution to this: the global setting should support some tokens. Initially, the main one would be "%project" which would be substituted with the project's short name. In the future, we could consider using token.module for this, and getting crazy with other possible tokens, but for now, a simple %project token will do. That'd let site admins do various things like:

%project@blackhole.com
%project-issues@example.com
issues@%project.example.com
noreply@%project.example.com


C) Some sites might find that even this token isn't flexible enough, and they *really* want per-project settings, after all. For them, we say "let them eat hook_mail_alter()". If they must, they can form_alter the project node form, maintain their own table, and in hook_mail_alter(), parse the subject to find the project shortname, find the project, and replace the from address with the per-project setting from their own table. It's sort of a hassle, but it'd be for a tiny edge case, and it'll make the main project code and UI simpler.

So, this patch needs the following:
- Rip out the per-project mail setting completely from the UI and DB.
- Mention that this column is getting dropped in a DB update inside project/UPGRADE.txt, and if sites actually want to preserve that data, they should do so manually before running update.php.
- Support (at least) a %project token in the global setting for the from-generating logic.

Thanks,
-Derek

i'd vote for one more token: %project_nid. not critical at all, but i think it could be handy.

Some thoughts...

While we're going crazy here, do we want to do anything with the seemingly redundant two other e-mail address fields @ node/#/edit/issues? Another approach to solve all three of these problems is to have project_issue add in the e-mail address field on node/#/edit via hook_form_alter(), with two checkboxes:

[ ] Send weekly critical issues report to this address.
[ ] Send a copy of all issues to this address.

Or should this be another issue? Or should we just never mind about this? It was just something I noticed as I was looking through mail.inc that seemed kind of dumb. Could one of you run a query on your local dump to see how many instances we have where a) one or both of those fields is not null and b) it is different from the e-mail address specified at node/#/edit?

IMO the simplest thing here seems to be to have project_issue module use variable_get('site_mail') as its "from" on all e-mails and be done with it. Why should project module add Yet Another Email Setting when we already store a value specifically for the From address on Drupal system e-mails? It seems a layer of indirection we don't need, and for those sites that want fancier behaviour, hook_mail_alter is their friend, and it means far less code for us to maintain in project/project_issue modules. A contrib module that uses hook_mail_alter to support token replacement and custom from addresses per system mail event sounds like a great thing that could support this kind of advanced configuration for those who need it. But IMO this doesn't belong in project/project_issue.

And again, I maintain this is a critical privacy issue: see #4 in http://drupal.org/node/36619#comment-620225.

Personally, I think %project_nid is overkill. I'd rather stick with the simple %project token at this point, and then look into token API support and extra complexity in a separate issue.

@webchick:
- I totally agree about the privacy thing, I definitely want to see this all resolved ASAP.
- I think discussion of the other email settings should move into a separate issue -- out of scope in here.
- I (strongly) disagree about forcing this to be site_mail by default in project_issue:
-- a) d.o currently wants noreply@d.o for issues, not info@d.o.
-- b) mailhandler is an *obvious* use-case for something different than site_mail in general, and in fact, for per-project from addresses. The ability to generate and reply to issues directly via email is a critical feature that hasn't gotten any love in a long time. I refuse to do anything to the project_issue codebase that's going to make that any more difficult for anyone, including d.o.

Thanks for the input,
-Derek

Cool. Thanks for the input, dww. We'll go with your suggestions in #4. I created a separate issue for the merging of e-mail addresses at http://drupal.org/node/189629.

I'm traveling tomorrow and will be spending the rest of the week in Providence doing training, but will try and set aside time for this one of the evenings. Or if nothing else, aclight e-mailed me about this, and said that he would be around this weekend.

code looks good. haven't tested yet. one thing though:

do we want to use t() like this?

+  $form['project_issue_reply_to'] = array(
+    '#type' => 'textfield',
+    '#title' => t('Reply-to address on e-mail notifications'),
+    '#default_value' => variable_get('project_issue_reply_to', variable_get('site_mail', ini_get('sendmail_from'))),
+    '#description' => t('All issue e-mails sent via subscriptions will appear from this e-mail address. You can use %project as a placeholder which will be replaced with the %short_project_name setting for the issue\'s current project.', array('%project' => '%project', '%short_project_name' => t('Short project name'))),
+  );

it looks like the idea is to put the em tags in there, but i don't think this is actually the recommended way of using t(). The second t() inside of the first t()'s replacements looks especially weird.

The first is a way to get theme('placeholder') called easily and consistently. The second one is to avoid translators having to translate the same thing again. It's just the existing t() for the name of the UI element. There's nothing wrong with either one, IMHO.

code style looks good. tested added/editing projects, setting the new global mail addy, and receiving emails. all works perfectly.

Committed to HEAD and DRUPAL-5 (for project_issue). installed on d.o, and setting set to noreply@drupal.org. Phew! ;)

Yay!!! Thanks, guys!! :D