diff options
Diffstat (limited to 'Bugzilla/BugMail.pm')
-rw-r--r-- | Bugzilla/BugMail.pm | 190 |
1 files changed, 162 insertions, 28 deletions
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 55eeeab25..55a37c230 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -43,11 +43,13 @@ use Bugzilla::Bug; use Bugzilla::Comment; use Bugzilla::Mailer; use Bugzilla::Hook; +use Bugzilla::Instrument; use Date::Parse; use Date::Format; use Scalar::Util qw(blessed); -use List::MoreUtils qw(uniq); +use List::MoreUtils qw(uniq firstidx); +use Sys::Hostname; use constant BIT_DIRECT => 1; use constant BIT_WATCHING => 2; @@ -74,6 +76,16 @@ sub Send { my ($id, $forced, $params) = @_; $params ||= {}; + my $instrument; + my $current_login = Bugzilla->user->login; + if ($current_login eq 'ehsan@mozilla.com' + || $current_login eq 'glob@mozilla.com' + || $current_login eq 'dkl@mozilla.com') + { + $instrument = Bugzilla::Instrument->new('bugmail'); + $instrument->begin("bug $id user $current_login"); + } + my $dbh = Bugzilla->dbh; my $bug = new Bugzilla::Bug($id); @@ -107,33 +119,54 @@ sub Send { my %user_cache = map { $_->id => $_ } (@assignees, @qa_contacts, @ccs); my @diffs; + my @referenced_bugs; if (!$start) { @diffs = _get_new_bugmail_fields($bug); } if ($params->{dep_only}) { + my $fields = Bugzilla->fields({ by_name => 1 }); push(@diffs, { field_name => 'bug_status', + field_desc => $fields->{bug_status}->description, old => $params->{changes}->{bug_status}->[0], new => $params->{changes}->{bug_status}->[1], login_name => $changer->login, blocker => $params->{blocker} }, { field_name => 'resolution', + field_desc => $fields->{resolution}->description, old => $params->{changes}->{resolution}->[0], new => $params->{changes}->{resolution}->[1], login_name => $changer->login, blocker => $params->{blocker} }); + push(@referenced_bugs, $params->{blocker}->id); } else { - push(@diffs, _get_diffs($bug, $end, \%user_cache)); + my ($diffs, $referenced) = _get_diffs($bug, $end, \%user_cache); + push(@diffs, @$diffs); + push(@referenced_bugs, @$referenced); } my $comments = $bug->comments({ after => $start, to => $end }); # Skip empty comments. @$comments = grep { $_->type || $_->body =~ /\S/ } @$comments; + # Add duplicate bug to referenced bug list + foreach my $comment (@$comments) { + if ($comment->type == CMT_DUPE_OF || $comment->type == CMT_HAS_DUPE) { + push(@referenced_bugs, $comment->extra_data); + } + } + + # Add dependencies to referenced bug list on new bugs + if (!$start) { + push @referenced_bugs, @{ $bug->dependson }; + push @referenced_bugs, @{ $bug->blocked }; + } + ########################################################################### # Start of email filtering code ########################################################################### + $instrument && $instrument->begin('email filtering'); # A user_id => roles hash to keep track of people. my %recipients; @@ -192,22 +225,26 @@ sub Send { Bugzilla::Hook::process('bugmail_recipients', { bug => $bug, recipients => \%recipients, users => \%user_cache, diffs => \@diffs }); - - # Find all those user-watching anyone on the current list, who is not - # on it already themselves. - my $involved = join(",", keys %recipients); - - my $userwatchers = - $dbh->selectall_arrayref("SELECT watcher, watched FROM watch - WHERE watched IN ($involved)"); - - # Mark these people as having the role of the person they are watching - foreach my $watch (@$userwatchers) { - while (my ($role, $bits) = each %{$recipients{$watch->[1]}}) { - $recipients{$watch->[0]}->{$role} |= BIT_WATCHING - if $bits & BIT_DIRECT; + $instrument && $instrument->end(); + + $instrument && $instrument->begin('watchers'); + if (scalar keys %recipients) { + # Find all those user-watching anyone on the current list, who is not + # on it already themselves. + my $involved = join(",", keys %recipients); + + my $userwatchers = + $dbh->selectall_arrayref("SELECT watcher, watched FROM watch + WHERE watched IN ($involved)"); + + # Mark these people as having the role of the person they are watching + foreach my $watch (@$userwatchers) { + while (my ($role, $bits) = each %{$recipients{$watch->[1]}}) { + $recipients{$watch->[0]}->{$role} |= BIT_WATCHING + if $bits & BIT_DIRECT; + } + push(@{$watching{$watch->[0]}}, $watch->[1]); } - push(@{$watching{$watch->[0]}}, $watch->[1]); } # Global watcher @@ -217,6 +254,7 @@ sub Send { next unless $watcher_id; $recipients{$watcher_id}->{+REL_GLOBAL_WATCHER} = BIT_DIRECT; } + $instrument && $instrument->end(); # We now have a complete set of all the users, and their relationships to # the bug in question. However, we are not necessarily going to mail them @@ -229,6 +267,9 @@ sub Send { my $date = $params->{dep_only} ? $end : $bug->delta_ts; $date = format_time($date, '%a, %d %b %Y %T %z', 'UTC'); + # Remove duplicate references, and convert to bug objects + @referenced_bugs = @{ Bugzilla::Bug->new_from_list([uniq @referenced_bugs]) }; + foreach my $user_id (keys %recipients) { my %rels_which_want; my $sent_mail = 0; @@ -237,6 +278,13 @@ sub Send { # Deleted users must be excluded. next unless $user; + # If email notifications are disabled for this account, or the bug + # is ignored, there is no need to do additional checks. + if ($user->email_disabled || $user->is_bug_ignored($id)) { + push(@excluded, $user->login); + next; + } + if ($user->can_see_bug($id)) { # Go through each role the user has and see if they want mail in # that role. @@ -253,7 +301,7 @@ sub Send { } } } - + if (scalar(%rels_which_want)) { # So the user exists, can see the bug, and wants mail in at least # one role. But do we want to send it to them? @@ -267,8 +315,34 @@ sub Send { } # Make sure the user isn't in the nomail list, and the dep check passed. - if ($user->email_enabled && $dep_ok) { - # OK, OK, if we must. Email the user. + # BMO: never send emails to bugs or .tld addresses. this check needs to + # happen after the bugmail_recipients hook. + if ($user->email_enabled && $dep_ok && + ($user->login !~ /bugs$/) && + # sync-1@bugzilla.tld here is a temporary hack, see bug 844724 + ($user->login eq 'sync-1@bugzilla.tld' || $user->login !~ /\.tld$/)) + + { + # Don't show summaries for bugs the user can't access, and + # provide a hook for extensions such as SecureMail to filter + # this list. + # + # We build an array with the short_desc as a separate item to + # allow extensions to modify the summary without touching the + # bug object. + my $referenced_bugs = []; + foreach my $ref (@{ $user->visible_bugs(\@referenced_bugs) }) { + push @$referenced_bugs, { + bug => $ref, + id => $ref->id, + short_desc => $ref->short_desc, + }; + } + Bugzilla::Hook::process('bugmail_referenced_bugs', + { updated_bug => $bug, + referenced_bugs => $referenced_bugs }); + + $instrument && $instrument->begin("sending to " . $user->login); $sent_mail = sendMail( { to => $user, bug => $bug, @@ -279,7 +353,10 @@ sub Send { $watching{$user_id} : undef, diffs => \@diffs, rels_which_want => \%rels_which_want, + referenced_bugs => $referenced_bugs, + instrument => $instrument, }); + $instrument && $instrument->end(); } } @@ -300,6 +377,7 @@ sub Send { $bug->{lastdiffed} = $end; } + $instrument && $instrument->end(); return {'sent' => \@sent, 'excluded' => \@excluded}; } @@ -314,6 +392,8 @@ sub sendMail { my $watchingRef = $params->{watchers}; my @diffs = @{ $params->{diffs} }; my $relRef = $params->{rels_which_want}; + my $referenced_bugs = $params->{referenced_bugs}; + my $instrument = $params->{instrument}; # Only display changes the user is allowed see. my @display_diffs; @@ -352,6 +432,17 @@ sub sendMail { push(@watchingrel, 'None') unless @watchingrel; push @watchingrel, map { user_id_to_login($_) } @$watchingRef; + # BMO: Use field descriptions instead of field names in header + my @changedfields = uniq map { $_->{field_desc} } @display_diffs; + my @changedfieldnames = uniq map { $_->{field_name} } @display_diffs; + + # Add attachments.created to changedfields if one or more + # comments contain information about a new attachment + if (grep($_->type == CMT_ATTACHMENT_CREATED, @send_comments)) { + push(@changedfields, 'Attachment Created'); + push(@changedfieldnames, 'attachment.created'); + } + my $vars = { date => $date, to_user => $user, @@ -362,12 +453,19 @@ sub sendMail { reasonswatchheader => join(" ", @watchingrel), changer => $changer, diffs => \@display_diffs, - changedfields => [uniq map { $_->{field_name} } @display_diffs], + changedfields => \@changedfields, + changedfieldnames => \@changedfieldnames, new_comments => \@send_comments, threadingmarker => build_thread_marker($bug->id, $user->id, !$bug->lastdiffed), + referenced_bugs => $referenced_bugs, + instrument => $instrument, }; + $instrument && $instrument->begin('generating bugmail'); my $msg = _generate_bugmail($user, $vars); + $instrument && $instrument->end(); + $instrument && $instrument->begin('sending to mta'); MessageToMTA($msg); + $instrument && $instrument->end(); return 1; } @@ -376,11 +474,17 @@ sub _generate_bugmail { my ($user, $vars) = @_; my $template = Bugzilla->template_inner($user->setting('lang')); my ($msg_text, $msg_html, $msg_header); - + my $instrument = $vars->{instrument}; + + $instrument && $instrument->begin('header'); $template->process("email/bugmail-header.txt.tmpl", $vars, \$msg_header) || ThrowTemplateError($template->error()); + $instrument && $instrument->end(); + + $instrument && $instrument->begin('text body'); $template->process("email/bugmail.txt.tmpl", $vars, \$msg_text) || ThrowTemplateError($template->error()); + $instrument && $instrument->end(); my @parts = ( Email::MIME->create( @@ -391,24 +495,32 @@ sub _generate_bugmail { ) ); if ($user->setting('email_format') eq 'html') { + $instrument && $instrument->begin('html body'); $template->process("email/bugmail.html.tmpl", $vars, \$msg_html) || ThrowTemplateError($template->error()); push @parts, Email::MIME->create( attributes => { - content_type => "text/html", + content_type => "text/html", }, body => $msg_html, ); + $instrument && $instrument->end(); } + $instrument && $instrument->begin('email object'); # TT trims the trailing newline, and threadingmarker may be ignored. my $email = new Email::MIME("$msg_header\n"); + + # For tracking/diagnostic purposes, add our hostname + $email->header_set('X-Generated-By' => hostname()); + if (scalar(@parts) == 1) { $email->content_type_set($parts[0]->content_type); } else { $email->content_type_set('multipart/alternative'); } $email->parts_set(\@parts); + $instrument && $instrument->end(); return $email; } @@ -426,6 +538,7 @@ sub _get_diffs { my $diffs = $dbh->selectall_arrayref( "SELECT fielddefs.name AS field_name, + fielddefs.description AS field_desc, bugs_activity.bug_when, bugs_activity.removed AS old, bugs_activity.added AS new, bugs_activity.attach_id, bugs_activity.comment_id, bugs_activity.who @@ -434,11 +547,12 @@ sub _get_diffs { ON fielddefs.id = bugs_activity.fieldid WHERE bugs_activity.bug_id = ? $when_restriction - ORDER BY bugs_activity.bug_when", {Slice=>{}}, @args); + ORDER BY bugs_activity.bug_when, fielddefs.description", {Slice=>{}}, @args); + my $referenced_bugs = []; foreach my $diff (@$diffs) { - $user_cache->{$diff->{who}} ||= new Bugzilla::User($diff->{who}); - $diff->{who} = $user_cache->{$diff->{who}}; + $user_cache->{$diff->{who}} ||= new Bugzilla::User($diff->{who}); + $diff->{who} = $user_cache->{$diff->{who}}; if ($diff->{attach_id}) { $diff->{isprivate} = $dbh->selectrow_array( 'SELECT isprivate FROM attachments WHERE attach_id = ?', @@ -449,9 +563,13 @@ sub _get_diffs { $diff->{num} = $comment->count; $diff->{isprivate} = $diff->{new}; } + elsif ($diff->{field_name} eq 'dependson' || $diff->{field_name} eq 'blocked') { + push @$referenced_bugs, grep { /^\d+$/ } split(/[\s,]+/, $diff->{old}); + push @$referenced_bugs, grep { /^\d+$/ } split(/[\s,]+/, $diff->{new}); + } } - return @$diffs; + return ($diffs, $referenced_bugs); } sub _get_new_bugmail_fields { @@ -459,6 +577,20 @@ sub _get_new_bugmail_fields { my @fields = @{ Bugzilla->fields({obsolete => 0, in_new_bugmail => 1}) }; my @diffs; + # Show fields in the same order as the DEFAULT_FIELDS list, which mirrors + # 4.0's behavour and provides sane grouping of similar fields. + # Any additional fields are sorted by descrsiption + my @prepend; + foreach my $name (map { $_->{name} } Bugzilla::Field::DEFAULT_FIELDS) { + my $idx = firstidx { $_->name eq $name } @fields; + if ($idx != -1) { + push(@prepend, $fields[$idx]); + splice(@fields, $idx, 1); + } + } + @fields = sort { $a->description cmp $b->description } @fields; + @fields = (@prepend, @fields); + foreach my $field (@fields) { my $name = $field->name; my $value = $bug->$name; @@ -484,7 +616,9 @@ sub _get_new_bugmail_fields { # If there isn't anything to show, don't include this header. next unless $value; - push(@diffs, {field_name => $name, new => $value}); + push(@diffs, {field_name => $name, + field_desc => $field->description, + new => $value}); } return @diffs; |