110980 - no email to CC list when opening new bug

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect, P2)

Description

[Julian Ladisch]

In the user preferences in section
"When I'm on the CC list, email me" 
I check "When I'm added to or removed from this capacity"
and I uncheck "New Comments".

If someone else puts me in the CC list when opening a 
new bug, I won't get email.

Expected result, however, is that I get email because
I have been added to the capacity "being on CC list".

If I get added to CC list lateron, or if I check "New Comments",
then emailing works fine.

Comment 1

[Dave Miller [:justdave]]

no patch, not a blocker, -> 2.18

Comment 2

[Matthew Tuck [:CodeMachine]]

*** Bug 125169 has been marked as a duplicate of this bug. ***

Comment 3

[Matthew Tuck [:CodeMachine]]

We need to also check the assignee and QA, plus reporter when the "my changes"
setting is on.

Comment 4

[Jacob Steenhagen]

Attachment: Patch

This makes post_bug.cgi use the same method of "tricking" processmail that
process_bug.cgi does.

Comment 5

[Jacob Steenhagen]

Moving this back down to 2.16, but still not a blocker.

Comment 6

[Bradley Baetz (:bbaetz)]

Comment on attachment 69250 [details] [diff] [review]
Patch


>+my $CcString = "";
> foreach my $person (keys %ccids) {
>     SendSQL("insert into cc (bug_id, who) values ($id, $person)");
>+    $CcString .= $ccids{$person};
> }

Don't you need to separate those by a comma or a space? I won't mark needs work
because I haven't tested this yet, though.

Comment 7

[Jacob Steenhagen]

Attachment: Patch v2

Yes, if there's more than one value it does have to comma seperate them...
silly me, I only tested w/one user :)

Comment 8

[Dave Miller [:justdave]]

just to double-check here, I seem to recall a patch that Myk did that totally
redid the mail preferences checking...  I don't recall if it's been checked in
or not though.  Would that negate the need for this? (since it was almost a
total rewrite, seems like it may have fixed it on accident)

Comment 9

[Bradley Baetz (:bbaetz)]

Comment on attachment 74580 [details] [diff] [review]
Patch v2

OK, this WFM.

r=bbaetz if you add the -forcereporter option to the help message just below

Comment 10

[Dave Miller [:justdave]]

Changing default owner of Email Notifications component to JayPee, a.k.a.
J. Paul Reed ([e-mail]).  Jake will be offline for a few months.

Comment 11

[Dave Miller [:justdave]]

changing owner back to Jake for the record books because he wrote the patch for
this one.

Comment 12

[Dave Miller [:justdave]]

Just out of curiosity, what circumstances would allow me to reproduce this?  I'm
unable to duplicate the symptoms of this bug on the tip.  I can create a new bug
and everybody I put on the CC list gets mailed.

Comment 13

[Bradley Baetz (:bbaetz)]

See comment 0 - if you have your email prefs set so that you only get notified
on add/remove, not comments, then you won't get mail if you're ccd when a new
bug is created.

Comment 14

[Gervase Markham [:gerv]]

Comment on attachment 74580 [details] [diff] [review]
Patch v2

>-            $ccids{DBNameToIdAndCheck($person)} = 1;
>+            $ccids{DBNameToIdAndCheck($person)} = $person;
...
>+my @Cc;
> foreach my $person (keys %ccids) {
>     SendSQL("insert into cc (bug_id, who) values ($id, $person)");
>+    push(@Cc, $ccids{$person});

Huh? Why not nuke the first change, then just do:

>+    push(@Cc, $person);

? And can we call it @cc, please - @Cc looks weird.

>+if ($#ARGV >= 0 && $ARGV[0] eq "-forcereporter") {
>+    shift(@ARGV);
>+    @{$force{'Reporter'}} = trim(shift(@ARGV));
>+}
>+
> if (($#ARGV < 0) || ($#ARGV > 1)) {
>     print "Usage:\n processmail {bugid} {nametoexclude} " . 
>       "[-forcecc list,of,users]\n             [-forceowner name] " .
></PRE></BODY></HTML>

This method of command-line processing sucks a bit; if we are 
going to force the options to be in a certain order, why do we
bother labelling them with "-force*"?

Gerv

Comment 15

[Bradley Baetz (:bbaetz)]

gerv: one is an id, the other is an email address. The two are only equivalent
iff  |DBNameToIdAndCheck($person) = $person|, which is obviously not true.

Yes, commandline processing sucks. Thats a separate bug, though - see bug 92276,
plus bug 124174.

Comment 16

[Gervase Markham [:gerv]]

Comment on attachment 74580 [details] [diff] [review]
Patch v2

Doh. OK. :-) r=gerv.

Gerv

Comment 17

[Bradley Baetz (:bbaetz)]

Attachment: patch

Fixed cvs conflicts because of the templatisation. gerv, can you review again,
please?

Comment 18

[Gervase Markham [:gerv]]

Comment on attachment 78137 [details] [diff] [review]
patch

You seem to have made the change I suggested which you then said was bogus. 

What is going on there?

Gerv

Comment 19

[Bradley Baetz (:bbaetz)]

the templatisation merged the two loops - we can use $person directly, rather
than storing it in the hash and pulling it out later.

In the orignal patch, $person was referring to different things each time.
Firstly as a name, then as the keys of the hasah, which is an id.

Here the combined loop just uses the email addr as $person, and $ccid as the id.

Comment 20

[Gervase Markham [:gerv]]

Comment on attachment 78137 [details] [diff] [review]
patch

Right. No wonder the old code had me confused. :-)

r=gerv.

Gerv

Comment 21

[Dave Miller [:justdave]]

Comment on attachment 78137 [details] [diff] [review]
patch

ok, makes much better sense to me now.	r= justdave

Comment 22

[Bradley Baetz (:bbaetz)]

Checked in:

Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.45; previous revision: 1.44
done
Checking in processmail;
/cvsroot/mozilla/webtools/bugzilla/processmail,v  <--  processmail
new revision: 1.81; previous revision: 1.80
done