Manage e-mail notifications: Add option to not email the person who is making the update

I'm tempted to won't fix this. I often find it's useful to get my own emails on other sites where I'm using this module, for example, to forward the update to someone who's not necessarily on the site using the issue queue. In terms of the situation you describe where someone's on a spree updating issues -- if you're spamming everyone subscribed to the queue, you might as well be getting a dose of your own medicine. ;)

That said, this *could* be an option that users could control for themselves as part of the issue subscription overhaul that desperately needs to happen...

@dww perhaps just adding the option to not email the owner would be nice.

If I go, say, to http://drupal.org/project/issues/subscribe-mail/dynamic_persistent_menu, I'm given the option of being notified of all issues, issues I follow, or none. It would make sense to add a checkbox here saying: "Don't notify of own posts/comments", so that next time I create a post or comment, I don't get notified.

Cheers,

Albert.

I would also like to see this- it's kind of annoying to get an email for an update to an issue, post a reply, delete the email, and then a few moments later get ANOTHER email with the reply that I just posted.

Here's a very rough, first cut of this. Completely untested, but just want to get something out there.

Here's a slightly tweaked version that flips the checkbox around, so that you tick the box to not receive your own updates, which I think makes more sense, and copies what other systems (Redmine) do.

I've tested this on a simple project issue setup locally, and it seems to work fine!

Here's a screenshot of what the added checkbox looks like:

Actually the patch in #5 contravenes the User interface standards so the patch in comment #4 should be reviewed and preferred.

Here's an updated screenshot:

+1 for this. I get a lot of email anyway, and if I post on an issue, I tend to know I've posted, since I am not senile (yet), so I'd like to opt not to get an email confirming that I've done something I know I've just done!

Review of patch in #4

+++ b/project_issue.installundefined
@@ -300,6 +300,14 @@ function project_issue_schema() {
+        'description' => 'User preference for whether they want to be receive notifications of their own updates.',

@@ -1181,3 +1189,22 @@ function project_issue_update_6018() {
+    'description' => 'User preference for whether they want to be receive notifications of their own updates.',

... want to receive ...

Manual edit of the patch that corrects the issue described in #9

Is looking good, but I have to admit I didn't try patching

Sorry, but this really does need a proper review I think.

I installed everything and created a new project + issues and the notification are only sent to myself when the checkbox is checked.

RTBC for real this time

Thanks for the review.

This still needs this question from the issue summary answering:

check that this can be ported to D7 easily.

I don't really know enough about the D7 port to say one way or the other.

From d.o office hours IRC:

<@darthsteven> I'm interested in finding out what it would take to get an issue I've fixed deployed onto d.o: http://drupal.org/node/667122
<@Druplicon> http://drupal.org/node/667122 => Manage e-mail notifications: Add option to not email the person who is making the update => Project issue tracking, Mail, normal, reviewed & tested by the community, 14 comments, 1 IRC mention
<@darthsteven> Specifically do I need to port my fix to D7 too
<@drumm> darthsteven: yes, for project we've been working on only the D7 upgrade for the last few months. We do need a 7.x version.
<@drumm> darthsteven: thanks for the patch so far!
<@darthsteven> drumm: thanks, I'll work out how to port it and post a patch

so we'll port and fix in D7.

If this gets ported to D7 and then backported to D6, I will laugh...

Add issue summary

Totally going to work on this at Drupalcon.

Here's a cheeky little re-roll for D7.

This still needs some tests.

Actually, we should use the ->uid properties for the actor, not use globals.

Right, actually, I can remove the 'actor' stuff from the previous patch, and I can add some tests!

Thanks! Very brief review looks good. I just downloaded the patch to my laptop and will do a more thorough review and testing ASAP.

Cheers,
-Derek

Had a lot of time on a plane so I gave this a thorough review and local testing. It works great, thanks! And yay for including tests with your patch. :)

I committed your patch exactly as-is to commit 661e6f318. Then, I made some cleanups. Mostly pedantic nit-picky stuff that I didn't want to have you re-roll for. Committed that as commit 437444744. Notes on what I changed:

A) I renamed the DB column and internal variables to 'notify_own_updates' instead of 'include_own_updates'. Using 'include' sort of implies the setting controls what's included in the notification (e.g. another setting affecting the content of the messages) not if I should get notified at all. The UI text for the setting (which I like) uses 'notified', so we should use the same verb for the internal stuff that references it, too.

B) I didn't like how the patch overloaded $can_access inside _project_issue_email_notify(). Seems cleaner to use continue; once we realize we don't want to send the message. Frankly, I should just kill $can_access and use continue; there, too. Just pushed that as commitd d3f7e7adee, in fact. ;)

C) Fixed a handful of wonky code comments.

D) I wasn't thrilled with ProjectIssueWebTestCase::createIssueComment(). It was basically cut+paste from CommentHelperCase::postComment(), and was needlessly re-testing comment.module's settings and behaviors (generating more assertions than we need, slowing things down, etc). IMHO, project_issue shouldn't be in the business of testing if requiring comment previews is working or not. ;) It now variable_set()'s to set preview to optional, has a lot less assertions, doesn't mess with all the $contact stuff we don't care about, etc.

Tested again (just to be sure I didn't break anything) and pushed both commits.

Now this just needs to be deployed. ;)

Thanks so much, and sorry it took so long to get this reviewed and in!

Cheers,
-Derek

AMAZING!

Thank you so much, and completely agree with the cleanups that you've made.
I'll try to tone down my 'conversational' style comments in tests next time :)

Now deployed to Drupal.org.

Thanks very much!

(I'm looking forward to not getting this email)

Thanks all, this is great