Allow to configure content of issue notification e-mails

I agree, I also think it's a pain to get the entire history of a bug report. It also screws up digest subscriptions to the Drupal-Devel list, as well as clutters the mailing list archives.

As only "patch" status issues get mailed, having the history of a bug in the e-mail is a useful feature.Emailing all issues is not a good alternative. We tried that a long time, and even back then it killed off the normal discussion on the list.

It would be nice if the mails could be formatted as proper digests (with threading info if it were available).

I'll look at it shortly. Bear with me as I'm slowly taking over maintenance of the project module.

Committed to HEAD.

Re-added the link to update a bug report.

This is an old issue from the project queue that now falls under the realm of the project_issue queue.

I have my Drupal project issue subscriptions sent to my gmail account, and the long history of issues does get really annoying since gmail displays all messages in a thread together. Scrolling down past pages of history to get to the next follow up of an issue takes a fair amount of time for long issues.

Once I get some time I'll look into how we can do this better.

this is not a bug. recategorizing

yes, this is just brutal in gmail. i think we should prepend all the older followups with a '>' charachter so that mail clients treat this as quoted text and can fold it out of sight if so configured.

@moshe: I agree that this is brutal in Gmail. However, I don't think that just prepending quoted text with a '>' character will fix the problem. I tried some tests on my own doing this and I could not get Gmail to collapse much or any of the quoted text. I think Gmail looks for text that is identical to previous text and not so much text that has '>' characters. If adding '>' characters doesn't work in Gmail I'm not sure what will.

The primary argument for keeping the mail history in each mail is gone now. Steven said in 2005

As only "patch" status issues get mailed, having the history of a bug in the e-mail is a useful feature.Emailing all issues is not a good alternative. We tried that a long time, and even back then it killed off the normal discussion on the list.

That is no longer how we work. The mailing of issues does not depend on status (correct me if i am wrong). I really think we can do away with the history. If there is a good argument for it, it should be enabled on a per user basis. We are filling up inboxes and network pipes for no good reason.

I was thinking the same thing myself today. I personally never read anything but the most recent comment in an issue. If I find myself needing more, I just look at the issue on the web. However, I think some people like to read the issues on their cell phones, etc. where web access may be more spotty than email access. So having a user option to receive the entire history of an issue vs. just the most recent post in an issue is probably the way to go here.

I'd be willing to work on a patch for this, but as my last issue subscriptions patch got "won't fix"ed, I'd like to get hunmonk and/or dww's opinion of this idea before I spend any time writing a patch. If they are willing to accept a patch now, I'll work on it.

I've discussed this with hunmonk, and we have decided that there are three options of how to deal with this:

1. Change nothing.
2. Add a per-user option to allow users to choose whether to receive all comments on an issue in each email of that issue.
3. Send only the most recent update to an issue.

#2 would probably require a new database table and user interface to allow the user to change this setting. This might also cause problems if/when we convert project_issue to use the subscriptions module.

My vote is with #3, because I'd like this problem to be addressed but I don't want to have to add a bunch of new code to address it.

I'm going to post a request for feedback about this on the Drupal development list. If you have strong feelings one way or the other, please make them known here.

In an ideal world, this would be a per-user setting.

However, in the real world, I'd be happy to see a per-site setting and turning it off on d.o. Basically, I'd prefer to spend as little time as possible on this issue while the bigger question of sane subscription functionality goes unsolved. We need major reworking of that problem, and this is but a small piece of the puzzle, so I'd rather not have a lengthy debate, lots of code, etc. Let's just write a 20 line patch to add a setting for this and wrap the "spit out the whole history" part of project_mail_notify() inside an if() clause, and be done with it.

I think that's a sensible solution. Here's a patch that adds a site wide setting at admin/project/project-issue-settings which is set to include the entire history of an issue by default. This way this patch does not change the current behavior by default. Patch is 78 lines, but a lot of that is just because I had to indent a block of code. Tested on a local site and seems to work well.

Code looks great, thanks. The description of the setting and the comment in the code seem a little unwieldy, but it's 1:30am and I'm too tired to propose clearer alternatives. ;) I'll look at this again in the morning with fresh eyes and hopefully come up with something better, then commit/deploy this.

I gave this a second shot as far as the setting description and code comment go. I'm not sure that these are an improvement, however. I'll let you decide :)

In reply to aclight's mail from the development list:

If you have feelings one way or another about whether we should stop sending the complete history and instead send only the most recent comment in issue emails, please respond in the issue queue, not via email.

Seems like I'm a loner here :P

The problem that I can see with skipping them altogether is for issues where I subscribe only later - in order to know what's going on in that issue, I need to open it in the browser then. Could we maybe have the history sent for the first mail (= comment) in a newly created or newly subscribed issue? That's like:

• If a reporter opens an issue, there's no history yet anyways, so no additional data sent.
• If someone subscribes further down (first comment in this node), the first notification mail will include the whole history. Or better yet, the original node and each previous comment is sent as separate mail so mail threading works just as well as for people who've been there from the beginning.
• Subsequent comments only get sent as such, without history.

I've got no problem with letting my mail client handle the threading, but I'd certainly like all the important data (minus attachments, of course) to be in there.

Postponed for now until we have time to rework subscriptions in project issue, since they're lacking a lot of functionality as is.

huh? you worked so hard and finally got some consensus here and then all of a sudden you say "maybe someday".

If it were my call and my module, it would have been committed by now. But it's not, so I can't do that. The current plan, as I understand it, is to revamp subscriptions within project issue, and in the process we'll probably end up adding a per-user setting for whether or not to include all comments in a subscription email message. But doing this is probably lower on the list than moving to views and possibly some other things, and it wouldn't make a lot of sense to stop sending all comments right now and then in the not too distant future have the possibility to send them again.

Trust me, I hate having all of the comments sent as much as I do, but for now have trained myself to use the n and p shortcuts in gmail which allow me to at least skip over all of the stuff in the message I don't need to read.

Right, this isn't just "someday", it's just that there's not agreement yet, so it seems we're going to need a per-user setting for this, and that should most likely just be handled while we deal with #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment.

Where are we on that? For those of us following issues with gmail, the upside-down nature of issue updates makes them a real pain.

As per: http://3281d.com/projects/improving-subscriptions

Moshe re-proposed this change over at #886310: Mobile phone friendly emails. It's clear we can't just do this change as a global setting, it needs to be a per-user setting (there's a patch in progress, which I'm working on re-tooling). I just had a nice conversation in person with yoroy here at DCC about the interface issues here. This ties into my longer-term plan around fixing issue subscriptions.

Eventually, we're going to have a lot of per-user settings related to notifications. Read the "plan" link above for why. For now, we're going to need at least 2 settings to satisfy this issue (and the other #886310):

E-mail subject:
[x] Include project name
[x] Include issue category

E-mail body:
(*) Include full issue history
( ) Only include new content

So, the idea is to make a new /user/N/notifications tab. Doing it as the 8th subtab of user/N/edit would be insane. If we had such a tab, we could also fix #1088774: Move newsletter settings from 'My newsletters' subtab into user/N/notifications.

Lots of this code already exists in a patch from Moshe in #886310. I've been working on it in a local Git clone, but I'll continue hacking on it "tomorrow" (i.e. later today) during the code sprint... stay tuned. Yes, I should probably push it up into a feature branch here in project_issue, but I'm too tired for that right now, and just wanted to at least lay out the plan.

Thanks,
-Derek

p.s. This doesn't actually have anything to do with the flag integration itself, but it certainly relates to the wider problem of notifications on d.o, so I'm re-tagging, too.

See the 15380-user-notifications branch in project_issue:
http://drupalcode.org/project/project_issue.git/shortlog/refs/heads/1538...
I committed Moshe's patch from #886310: Mobile phone friendly emails then fixed it based on what I described in #33. The UI is tested, but I haven't verified that it actually changes the emails themselves yet. Pretty sure it works, but it definitely needs testing.

I haven't messed with the ordering of the message yet, since that should happen over at #199178: Usability: Re-order contents of project issue emails -- ideally without needing to add another preference setting. I hope we can just find a good solution that will make everyone happy.

I'm attaching two screenshots here, one of just the end of the 15380-user-notifications branch, and another with #199178-1: Usability: Re-order contents of project issue emails applied.

All buttons on the forms under user/*/edit have 'Save' as the button text, lets do that here as well.

Tagging…

As it's likely that we're going to merge these changes into #397458: Revamp mailing logic to leverage flag module, or at least have to bring both patches in line in terms of schema changes, router paths, and user interface, I've extracted the changes of the (outdated) git branch into a patch.

We should

1. change constants and function names to be in the project_issue_subscriptions namespace.
2. change the new table name into {project_issue_subscriptions_user}
3. move the user interface code from user_preferences.inc into subscribe.inc.

I'm going to change/prepare the other patch first, and will re-roll this patch against latest branch HEAD and with those changes afterwards, unless someone else beats me to it.

Latest patch in #397458-23: Revamp mailing logic to leverage flag module has been adjusted for the changes of this patch now. Schema, API functions, and user settings form are ready to be extended.

Setting this to needs work until those schema and namespace changes are in.

Crosslinking #606016: Figure out how best to present overridden/default/custom data

Although 'git merge' was some help, this mostly had to be re-written for the end of the 34496-flag-integration branch. Light local testing appears good. I'd like to get this up on the http://subscribe-drupal.redesign.devdrupal.org site soon for more eyes on it, and definitely have this on staging.d.o before we do the final testing for #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment. Meanwhile, anyone want to look at this code?

Here's the new screenie for the UI at /user/N/project-issue. The "Advanced settings" fieldset is collapsed by default, but expanded here so folks can see what's going on in there.

(BTW, the current defaults are that the subject includes both the project name and issue category, and the body includes "Full issue history". I.e. the defaults in the new UI match the current d.o/project_issue behavior).

p.s. Now deployed on http://subscribe-drupal.redesign.devdrupal.org -- DB updates ran without error. UI seems to be working fine. Initial mailing testing is looking good.

Note: now that I see this with the "Advanced settings" fieldset, I think the main fieldset around the table is stupid and should go. See #397458-86: Revamp mailing logic to leverage flag module.

Here's a re-roll for after #397458-86: Revamp mailing logic to leverage flag module is applied.

And here's a new screenie of this patch's Advanced settings fieldset (again, expanded, but it'd be collapsed by default) with the latest from #397458: Revamp mailing logic to leverage flag module:

Minor tweaks based on IRC feedback from yoroy. Renamed "Advanced settings" to be "Configure e-mail contents" to be more self-documenting, and then dropped "E-mail" from the labels of both form elements inside (since it's now redundant with the fieldset label).

Looks good to me. Elegant work! :)

Attached patch fixes a couple of things...

+++ b/includes/mail.inc
@@ -422,20 +426,42 @@ function _project_issue_mail($key, &$message, $params) {
+        $subject = t('[!short_name] [!category] !title', $subject_tokens);
...
+        $subject = t('!title', $subjet_tokens);

I don't think there's a point in t() here, so let's simplify this logic.

+++ b/includes/mail.inc
@@ -476,23 +502,20 @@ function _project_issue_mail($key, &$message, $params) {
+  $mail_body = isset($recipient->project_issue_notification['mail_body']) ? $recipient->project_issue_notification['mail_body'] : PROJECT_ISSUE_MAIL_BODY_FULL_HISTORY;

The loader already ensures default values.

+++ b/includes/notification.inc
@@ -106,6 +106,38 @@ function project_issue_notification_user_form(&$form_state, $account) {
+  $form['advanced'] = array(

'content'

+++ b/includes/notification.inc
@@ -106,6 +106,38 @@ function project_issue_notification_user_form(&$form_state, $account) {
+    '#title' => t('Configure e-mail contents'),

"content" is singular

+++ b/includes/notification.inc
@@ -142,8 +174,13 @@ function project_issue_notification_user_form_submit($form, &$form_state) {
+    'level' => $form_state['values']['projects']['default'],
+    'mail_body' => $form_state['values']['advanced']['project_issue_mail_body'],
+    'mail_subject_project' => !empty($form_state['values']['advanced']['project_issue_mail_subject']['mail_subject_project']),
+    'mail_subject_category' => !empty($form_state['values']['advanced']['project_issue_mail_subject']['mail_subject_category']),

Let's make sure that these submitted form values are integers and not auto-casted in strange ways by PHP.

+++ b/project_issue.install
@@ -298,7 +298,31 @@ function project_issue_schema() {
+        'default' => PROJECT_ISSUE_NOTIFICATION_NONE,
...
+        'default' => PROJECT_ISSUE_MAIL_BODY_FULL_HISTORY,

To my knowledge, constants defined in .module files are not available at install time/hook_install().

+++ b/project_issue.install
@@ -298,7 +298,31 @@ function project_issue_schema() {
+        'unsigned' => 1,
...
+        'unsigned' => 1,
...
+        'unsigned' => 1,

TRUE

+++ b/project_issue.module
@@ -9,6 +9,11 @@ define('PROJECT_ISSUE_AUTO_CLOSE_DAYS', 14);
+// Email notification e-mail contains the full issue history.
+define('PROJECT_ISSUE_MAIL_BODY_FULL_HISTORY', 0);
+// Email notification e-mail only contains new content.
+define('PROJECT_ISSUE_MAIL_BODY_NEW_CONTENT', 1);

Coding style.

+++ b/project_issue.module
@@ -959,19 +967,36 @@ function project_issue_notification_user_settings_load($account) {
   $level = $account->project_issue_notification['level'];
...
+  $mail_body = $account->project_issue_notification['mail_body'];
+  $mail_subject_category = $account->project_issue_notification['mail_subject_category'];
+  $mail_subject_project = $account->project_issue_notification['mail_subject_project'];

These separate local variables don't seem to have a purpose.

Thanks sun! Agreed on all points except: "Configure e-mail contents" -- "contents" is totally legitimate English in that case (e.g. "Table of contents" or "the contents of my desk drawer", etc), and yoroy wanted a verb in there to help clue people in. Sure, it's a bit redundant on a settings form, but it doesn't hurt.

So, I undid that, reviewed, tested again locally, committed and pushed to the '34496-flag-integration branch':

http://drupal.org/commitlog/commit/1894/7f11480f772fafac3bd64df76aaf3f44...

HURRAY! ;)

-Derek

1. Could we move project name and component name to the end of the Subject line as proposed at #886310: Mobile phone friendly emails. That would keep the most relevant content first. Important when using a small screen. This could be done for everyone or add another checkbox.

2. Could we (optionally?) move the table of issue metadata below the words that the commenter added? Again, this moves the highest value contribution up. Also requested at #886310: Mobile phone friendly emails

@moshe: As I mentioned at #886310-12: Mobile phone friendly emails (and the e-mail I sent you a few days ago) those requests are for #199178: Usability: Re-order contents of project issue emails not here nor #886310. They're both possible but I want to leave this issue fixed since the main win is already won. Let's meet at #199178 to discuss further.

Thanks,
-Derek

Yay -- alpha testing of the DB upgrade uncovered the following bug:

http://drupal.org/commitlog/commit/1894/1e17ea40064befcf4455f5bc4ff1d529...

We'll try this again tomorrow morning PDT.

testing++ ! ;)

Cheers,
-Derek