summaryrefslogtreecommitdiffstats
path: root/Bugzilla/BugMail.pm
diff options
context:
space:
mode:
Diffstat (limited to 'Bugzilla/BugMail.pm')
-rw-r--r--Bugzilla/BugMail.pm177
1 files changed, 136 insertions, 41 deletions
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index 55eeeab25..3d3bd2dbf 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -47,7 +47,8 @@ use Bugzilla::Hook;
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;
@@ -107,30 +108,53 @@ 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 };
+ }
+
+ # If no changes have been made, there is no need to process further.
+ return {'sent' => []} unless scalar(@diffs) || scalar(@$comments);
+
###########################################################################
# Start of email filtering code
###########################################################################
@@ -186,28 +210,30 @@ sub Send {
# Make sure %user_cache has every user in it so far referenced
foreach my $user_id (keys %recipients) {
- $user_cache{$user_id} ||= new Bugzilla::User($user_id);
+ $user_cache{$user_id} ||= new Bugzilla::User({ id => $user_id, cache => 1 });
}
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);
+ 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)");
+ 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;
+ # 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
@@ -222,21 +248,25 @@ sub Send {
# the bug in question. However, we are not necessarily going to mail them
# all - there are preferences, permissions checks and all sorts to do yet.
my @sent;
- my @excluded;
# The email client will display the Date: header in the desired timezone,
# so we can always use UTC here.
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;
- $user_cache{$user_id} ||= new Bugzilla::User($user_id);
- my $user = $user_cache{$user_id};
+ my $user = $user_cache{$user_id} ||= new Bugzilla::User({ id => $user_id, cache => 1 });
# 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.
+ next if ($user->email_disabled || $user->is_bug_ignored($id));
+
if ($user->can_see_bug($id)) {
# Go through each role the user has and see if they want mail in
# that role.
@@ -253,7 +283,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,10 +297,32 @@ 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.
- $sent_mail = sendMail(
- { to => $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$/) && ($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 });
+
+ my $sent_mail = sendMail(
+ { to => $user,
bug => $bug,
comments => $comments,
date => $date,
@@ -279,16 +331,12 @@ sub Send {
$watching{$user_id} : undef,
diffs => \@diffs,
rels_which_want => \%rels_which_want,
+ referenced_bugs => $referenced_bugs,
+ dep_only => $params->{dep_only}
});
+ push(@sent, $user->login) if $sent_mail;
}
}
-
- if ($sent_mail) {
- push(@sent, $user->login);
- }
- else {
- push(@excluded, $user->login);
- }
}
# When sending bugmail about a blocker being reopened or resolved,
@@ -300,12 +348,12 @@ sub Send {
$bug->{lastdiffed} = $end;
}
- return {'sent' => \@sent, 'excluded' => \@excluded};
+ return {'sent' => \@sent};
}
sub sendMail {
my $params = shift;
-
+
my $user = $params->{to};
my $bug = $params->{bug};
my @send_comments = @{ $params->{comments} };
@@ -314,6 +362,8 @@ sub sendMail {
my $watchingRef = $params->{watchers};
my @diffs = @{ $params->{diffs} };
my $relRef = $params->{rels_which_want};
+ my $referenced_bugs = $params->{referenced_bugs};
+ my $dep_only = $params->{dep_only};
# Only display changes the user is allowed see.
my @display_diffs;
@@ -352,6 +402,21 @@ 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 $bugmailtype = "changed";
+ $bugmailtype = "new" if !$bug->lastdiffed;
+ $bugmailtype = "dep_changed" if $dep_only;
+
my $vars = {
date => $date,
to_user => $user,
@@ -362,9 +427,12 @@ 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,
+ bugmailtype => $bugmailtype
};
my $msg = _generate_bugmail($user, $vars);
MessageToMTA($msg);
@@ -376,9 +444,10 @@ sub _generate_bugmail {
my ($user, $vars) = @_;
my $template = Bugzilla->template_inner($user->setting('lang'));
my ($msg_text, $msg_html, $msg_header);
-
+
$template->process("email/bugmail-header.txt.tmpl", $vars, \$msg_header)
|| ThrowTemplateError($template->error());
+
$template->process("email/bugmail.txt.tmpl", $vars, \$msg_text)
|| ThrowTemplateError($template->error());
@@ -395,7 +464,7 @@ sub _generate_bugmail {
|| ThrowTemplateError($template->error());
push @parts, Email::MIME->create(
attributes => {
- content_type => "text/html",
+ content_type => "text/html",
},
body => $msg_html,
);
@@ -403,6 +472,10 @@ sub _generate_bugmail {
# 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 {
@@ -426,6 +499,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 +508,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({ id => $diff->{who}, cache => 1 });
+ $diff->{who} = $user_cache->{$diff->{who}};
if ($diff->{attach_id}) {
$diff->{isprivate} = $dbh->selectrow_array(
'SELECT isprivate FROM attachments WHERE attach_id = ?',
@@ -449,9 +524,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 +538,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 +577,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;