413215 - Move the sending of email notifications from process_bug.cgi to Bugzilla::Bug

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Frédéric Buclin]

Currently, process_bug.cgi is looking at $changes->{bug_status} to know if dependent bugs should be notified and is sending emails itself. The same is true about changes in the bug itself and about the bug this one is marked as a dupe of. Even if you do changes from outside process_bug.cgi, these email notifications should be sent automatically without an explicit action from the caller. Maybe $bug->update() should do it.

Else you may be able to commit changes without ever notifying other users.

Comment 1

[Max Kanat-Alexander]

Yes, this is definitely true. This won't be a 3.1.3 blocker, but something like this should probably be done before 3.2. It might not actually make 3.2, though.

The problem here is that we don't really want to send emails inside of a database transaction, so there has to be some solution for that, since I want to make the whole update() loop in process_bug.cgi a single transaction. (Otherwise we might have made validations above the loop that aren't valid inside of the loop. The best would be to include the validations inside the transaction as well, but that might be even more work.)

Comment 2

[Frédéric Buclin]

Too late for 3.2.

Comment 4

[Max Kanat-Alexander]

Attachment: v1

Comment 5

[David Lawrence [:dkl]]

Just a thought, should other places use $bug->send_changes as well instead of Bugzilla::BugMail::Send. Some other places that come to mind:

post_bug.cgi
attachment.cgi
extensions/Voting/Extension.pm
Bugzilla/WebService/Bug.pm (create(), add_comment(), update_see_also())
editusers.cgi

Or should these be separate bugs later? If if a $changes var is not passed in, it can still other useful things such as dependency notifications, etc.

Dave

Comment 6

[Max Kanat-Alexander]

Yeah, they should use it! But yeah, separate bug.

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 447373 [details] [diff] [review]
v1

Everything looks good and works well in my testing. I guess this is good to go if there is no changes needed from my comment about other places in the code. r=dkl

Comment 8

[Max Kanat-Alexander]

Thanks, dkl! :-)

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified process_bug.cgi
modified Bugzilla/Bug.pm
Committed revision 7214.

Comment 9

[David Lawrence [:dkl]]

(In reply to comment #6)
> Yeah, they should use it! But yeah, separate bug.

Should I create a separate bug for each script that has Bugzilla::BugMail::Send or will one suffice?

Dave

Comment 10

[Max Kanat-Alexander]

(In reply to comment #9)
> Should I create a separate bug for each script that has Bugzilla::BugMail::Send
> or will one suffice?

  I think one bug will suffice.

Comment 11

[Max Kanat-Alexander]

Added to the release notes in bug 604256.