Closed Bug 146261 Opened 22 years ago Closed 22 years ago

Email not sent to users on CC list under some Status field transitions

Categories

(Bugzilla :: Email Notifications, defect)

Product:
Bugzilla
Component:
Email Notifications
Platform:
All
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 2.18

People

(Reporter: ttaylor, Assigned: ttaylor)

Details

Attachments

(1 file)

Sends email on Status field changes other than RESOLVED
827 bytes, patch
myk
: review+
bbaetz
: review+
Details | Diff | Splinter Review 
I'm using a CVS snapshot of Bugzilla updated on 22 May 2002, around 12:00 EST.

I have a user with the following email settings:
- Only email reports of changes by others                                      
          
- All relationships for 'Priority, status, severity, and/or milestone changes'
- All relationships for 'The bug is resolved or verified'
All other email settings are turned off.

This user is on the CC list for a bug.  When the bug is RESOLVED, the user gets
email.  If the bug is then CLOSED, they don't get email, but should since the
status field has changed.  If the bug is then REOPENED they also don't get
email, but should for the same reason.

I have a patch for this problem against revision 1.81 (the head at the time of
this report) of processmail which I will try to attach to this bug after
submitting it.
This is a patch against CVS revision 1.81 of the file processmail.  This was
the HEAD revision at the time this bug was posted.
Summary: Email not sent to uses on CC list under some Status field transitions → Email not sent to users on CC list under some Status field transitions
myk's being playing with the filtering code recently. This looks right to me,
though, and could explain some of the odd bugmails I occasio9nally get from
bugzilla when bugs are reopened.
Keywords: patch, review
Target Milestone: --- → Bugzilla 2.18
Comment on attachment 84669 [details] [diff] [review]
Sends email on Status field changes other than RESOLVED

Looks good, r=myk.

It would be useful to note somewhere that 'The bug is resolved 
or verified' overrides 'Priority, status, severity, and/or 
milestone changes', since it isn't obvious why a user with 
the former unchecked and the latter checked doesn't get mail 
when the status changes to resolved or verified.

This patch doesn't conflict at all with the filter changes
I have been doing over in bug 128469.
Attachment #84669 - Flags: review+
Hang on a sec. Why are these elsif's, then? Shouldn't they all be separate, so
that if any of the things are true, we send mail?
No, because the code containing the elsifs runs once per change, so no change
matches more than one condition.  The only exception is status changes to
resolved or verified, which match both the 'The bug is resolved or verified' and
the 'Priority, status, severity, and/or milestone changes' preferences.

Since the former preference is more specific than the latter, however, it should
override it, so the elsifs work in that situation as well (but this should be
documented).

I realise that they do match once per condition, but I'm asking why they do that.
They do it by definition.  Every email preference matches different fields
(modulo previously discussed Status overlap).  Every iteration of the loop
examines a change to one field.  Thus by definition it will never be the case
that more than one "elsif" condition is true for each iteration of the loop. 
Thus it makes no sense to separate these conditions from each other so that all
of them get run; the first matching condition automatically excludes the
possibility that other conditions match.
Comment on attachment 84669 [details] [diff] [review]
Sends email on Status field changes other than RESOLVED

r=bbaetz, then
Attachment #84669 - Flags: review+
Tim, do you have check-in privileges, or do you need someone to check this in
for you?
I don't have check-in privileges, so if someone could check my patch in for me
that would be great.
If this bug is present on the 2.16 branch also, then it should be fixed there
for 2.16, too, since this is a correctness issue. If this is the case and you
agree, please adjust the target milestone.
Only regressions should be going onto the 2.16 branch at this point. Is this a
regression?
Not sure if this is a regression. Anyway, if it's not gonna make the branch then
you need a release note like this "There is a known bug that causes you to not
get bugmail with certain email settings. To fix this, apply the patch from bug
146261." Not sure what is better: to have such a release note, or to check in
the fix. :-)
At this point, it's probably to add to the release notes, which is bug 97496.
Request for release note submitted to bug 97496. Note that I still think this is
(at least temporary) dataloss because you are not getting notified of possibly
important changes. (And way more important than html compliance and such ;-)
Myk please file a bug on making the wording on those prefs more specific as you
suggest.
I'm trying to write a release note for this, but it's not obvious what the bug
is here specifically.

Is it that 'Priority, status, severity, and/or milestone changes' is ignored? 
Or just in certain situations?  Or doesn't really apply to statuses?  Or what?

Apparently this is true for 2.14 too so I'll release note it for 2.14.2 too.
The bug is that there are two email notification categories that overlap: 
"Priority, status, severity, and/or milestone changes", and "The bug is resolved 
or verified".  Prior to my patch, if the status field of a bug changed from 
CLOSED to REOPENED, NO email notification was being sent.  With my patch, email 
will be sent in this situation.

The information that needs to go in the release notes is to explain a 
side-effect of the patch.  If I've registered to receive email when "Priority, 
status, severity, and/or milestone changes", but not for "The bug is resolved or 
verified", I won't get an email when a bug is "resolved or verified" even though 
the status field has changed.

After submitting my patch and watching the discussion that followed, I've begun 
to think that email should really go to both groups of people in the case I 
described above.  Then there wouldn't be a need for an explanation in the 
release notes.

If people concur, I'll make a new version of the patch.
OK, so the basic problem was that status changes were not governed by the option
that claimed to govern it, but rather the resolution/verification one.  And the
patch that is included changes it so all status changes are governed except to
RESOLVED and VERIFIED.
Matty, are you sure? Isn't it that without the patch, any status changes except
resolved and verified didn't trigger at all? And that nobody else noticed it
only because most people have other flags set as well (e.g. comments) and
usually status changes are accompanied with comments?
That's what I meant.
-> patch author.
Assignee: preed → ttaylor
Myk fixed this with:

revision 1.82
date: 2002/05/23 08:08:53;  author: myk%mozilla.org;  state: Exp;  lines: +2 -4
Fix for bug 146261: fixes bug preventing the sending of email to users when the
status of bugs changes in some situations.
Patch by Tim Taylor <ttaylor@mitre.org>.
r=myk,bbaetz

but didn't mark the bug as fixed
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
I concur with comment #18 from Tim Taylor.  When a bug gets RESOLVED or
CLOSED, bug mail should go out to people who want to be notified of status
changes, whether or not they also signed up to get "resolved or closed"
mail.

It's a lot more logical.  You're doing it the way people expect, instead
of doing it the way people don't expect and documenting it as a feature.

It's any easy fix, too: just change
"elsif ( $fieldName eq 'Severity' || ..." to 
"   if ( $fieldName eq 'Severity' || ..." and you're done!  (Well,
you should also change the comments above, as well.)
I STR that the customised resolution stuff will split those sorts of status
changes up.
QA Contact: matty_is_a_geek → default-qa
(In reply to Andreas Franke (gone) from comment #15)
> Request for release note submitted to bug 97496. Note that I still think
> this is
> (at least temporary) dataloss because you are not getting notified of
> possibly
> important changes. (And way more important than html compliance and such ;-)

bug 1328087  also resolved
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: