Bug 395451 - "Bugzilla::BugMail needs to use Bug objects internally instead of direct SQL"

[r=mkanat a=mkanat]

14 files changed, 119 insertions, 182 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index def8ad360..c0c2d97ff 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -90,6 +90,7 @@ sub DB_COLUMNS {
         delta_ts
         estimated_time
         everconfirmed
+        lastdiffed
         op_sys
         priority
         product_id
@@ -2612,6 +2613,11 @@ sub blocked {
 # Even bugs in an error state always have a bug_id.
 sub bug_id { $_[0]->{'bug_id'}; }
 
+sub bug_group {
+    my ($self) = @_;
+    return join(', ', (map { $_->name } @{$self->groups_in}));
+}
+
 sub related_bugs {
     my ($self, $relationship) = @_;
     return [] if $self->{'error'};
@@ -3586,7 +3592,8 @@ sub _validate_attribute {
         qw(error groups product_id component_id
            comments milestoneurl attachments isopened
            flag_types num_attachment_flag_types
-           show_attachment_flags any_flags_requesteeble),
+           show_attachment_flags any_flags_requesteeble
+           lastdiffed),
 
         # Bug fields.
         Bugzilla::Bug->fields
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index ff88dfaa3..98c3c40f8 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -27,6 +27,7 @@
 #                 J. Paul Reed <preed@sigkill.com>
 #                 Gervase Markham <gerv@gerv.net>
 #                 Byron Jones <bugzilla@glob.com.au>
+#                 Reed Loden <reed@reedloden.com>
 
 use strict;
 
@@ -110,121 +111,42 @@ sub relationships {
 sub Send {
     my ($id, $forced) = (@_);
 
-    my @headerlist;
-    my %defmailhead;
-    my %fielddescription;
-
-    my $msg = "";
-
     my $dbh = Bugzilla->dbh;
     my $bug = new Bugzilla::Bug($id);
 
-    # XXX - These variables below are useless. We could use field object
-    # methods directly. But we first have to implement a cache in
-    # Bugzilla->get_fields to avoid querying the DB all the time.
-    foreach my $field (Bugzilla->get_fields({obsolete => 0})) {
-        push(@headerlist, $field->name);
-        $defmailhead{$field->name} = $field->in_new_bugmail;
-        $fielddescription{$field->name} = $field->description;
-    }
+    # Only used for headers in bugmail for new bugs
+    my @fields = Bugzilla->get_fields({obsolete => 0, in_new_bugmail => 1});
 
-    my %values = %{$dbh->selectrow_hashref(
-        'SELECT ' . join(',', editable_bug_fields()) . ', reporter,
-                lastdiffed AS start_time, LOCALTIMESTAMP(0) AS end_time
-           FROM bugs WHERE bug_id = ?',
-        undef, $id)};
+    my $start = $bug->lastdiffed;
+    my $end   = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
-    my $product = new Bugzilla::Product($values{product_id});
-    $values{product} = $product->name;
-    if (Bugzilla->params->{'useclassification'}) {
-        $values{classification} = Bugzilla::Classification->new($product->classification_id)->name;
-    }
-    my $component = new Bugzilla::Component($values{component_id});
-    $values{component} = $component->name;
+    # Bugzilla::User objects of people in various roles. More than one person
+    # can 'have' a role, if the person in that role has changed, or people are
+    # watching.
+    my @assignees = ($bug->assigned_to);
+    my @qa_contacts = ($bug->qa_contact);
 
-    my ($start, $end) = ($values{start_time}, $values{end_time});
-
-    # User IDs of people in various roles. More than one person can 'have' a 
-    # role, if the person in that role has changed, or people are watching.
-    my $reporter = $values{'reporter'};
-    my @assignees = ($values{'assigned_to'});
-    my @qa_contacts = ($values{'qa_contact'});
-
-    my $cc_users = $dbh->selectall_arrayref(
-           "SELECT cc.who, profiles.login_name
-              FROM cc
-        INNER JOIN profiles
-                ON cc.who = profiles.userid
-             WHERE bug_id = ?",
-           undef, $id);
-
-    my (@ccs, @cc_login_names);
-    foreach my $cc_user (@$cc_users) {
-        my ($user_id, $user_login) = @$cc_user;
-        push (@ccs, $user_id);
-        push (@cc_login_names, $user_login);
-    }
+    my @ccs = @{ $bug->cc_users };
 
     # Include the people passed in as being in particular roles.
     # This can include people who used to hold those roles.
     # At this point, we don't care if there are duplicates in these arrays.
     my $changer = $forced->{'changer'};
     if ($forced->{'owner'}) {
-        push (@assignees, login_to_id($forced->{'owner'}, THROW_ERROR));
+        push (@assignees, Bugzilla::User->check($forced->{'owner'}));
     }
     
     if ($forced->{'qacontact'}) {
-        push (@qa_contacts, login_to_id($forced->{'qacontact'}, THROW_ERROR));
+        push (@qa_contacts, Bugzilla::User->check($forced->{'qacontact'}));
     }
     
     if ($forced->{'cc'}) {
         foreach my $cc (@{$forced->{'cc'}}) {
-            push(@ccs, login_to_id($cc, THROW_ERROR));
+            push(@ccs, Bugzilla::User->check($cc));
         }
     }
     
-    # Convert to names, for later display
-    $values{'changer'} = $changer;
-    # If no changer is specified, then it has no name.
-    if ($changer) {
-        $values{'changername'} = Bugzilla::User->new({name => $changer})->name;
-    }
-    $values{'assigned_to'} = user_id_to_login($values{'assigned_to'});
-    $values{'reporter'} = user_id_to_login($values{'reporter'});
-    if ($values{'qa_contact'}) {
-        $values{'qa_contact'} = user_id_to_login($values{'qa_contact'});
-    }
-    $values{'cc'} = join(', ', @cc_login_names);
-    $values{'estimated_time'} = format_time_decimal($values{'estimated_time'});
-
-    if ($values{'deadline'}) {
-        $values{'deadline'} = time2str("%Y-%m-%d", str2time($values{'deadline'}));
-    }
-
-    my $dependslist = $dbh->selectcol_arrayref(
-        'SELECT dependson FROM dependencies
-         WHERE blocked = ? ORDER BY dependson',
-        undef, ($id));
-
-    $values{'dependson'} = join(",", @$dependslist);
-
-    my $blockedlist = $dbh->selectcol_arrayref(
-        'SELECT blocked FROM dependencies
-         WHERE dependson = ? ORDER BY blocked',
-        undef, ($id));
-
-    $values{'blocked'} = join(",", @$blockedlist);
-
-    my $grouplist = $dbh->selectcol_arrayref(
-        '    SELECT name FROM groups
-         INNER JOIN bug_group_map
-                 ON groups.id = bug_group_map.group_id
-                    AND bug_group_map.bug_id = ?',
-        undef, ($id));
-
-    $values{'bug_group'} = join(', ', @$grouplist);
- 
-    my @args = ($id);
+    my @args = ($bug->id);
 
     # If lastdiffed is NULL, then we don't limit the search on time.
     my $when_restriction = '';
@@ -291,7 +213,6 @@ sub Send {
         push(@diffparts, $diffpart);
         push(@changedfields, $what);
     }
-    $values{'changed_fields'} = join(' ', @changedfields);
 
     my @depbugs;
     my $deptext = "";
@@ -307,7 +228,8 @@ sub Send {
 
         my $dependency_diffs = $dbh->selectall_arrayref(
            "SELECT bugs_activity.bug_id, bugs.short_desc, fielddefs.name, 
-                   bugs_activity.removed, bugs_activity.added
+                   fielddefs.description, bugs_activity.removed,
+                   bugs_activity.added
               FROM bugs_activity
         INNER JOIN bugs
                 ON bugs.bug_id = bugs_activity.bug_id
@@ -326,7 +248,7 @@ sub Send {
         my $lastbug = "";
         my $interestingchange = 0;
         foreach my $dependency_diff (@$dependency_diffs) {
-            my ($depbug, $summary, $what, $old, $new) = @$dependency_diff;
+            my ($depbug, $summary, $fieldname, $what, $old, $new) = @$dependency_diff;
 
             if ($depbug ne $lastbug) {
                 if ($interestingchange) {
@@ -341,8 +263,8 @@ sub Send {
                 $thisdiff .= ('-' x 76) . "\n";
                 $interestingchange = 0;
             }
-            $thisdiff .= three_columns($fielddescription{$what}, $old, $new);
-            if ($what eq 'bug_status'
+            $thisdiff .= three_columns($what, $old, $new);
+            if ($fieldname eq 'bug_status'
                 && is_open_state($old) ne is_open_state($new))
             {
                 $interestingchange = 1;
@@ -377,21 +299,21 @@ sub Send {
     # array of role constants.
     
     # CCs
-    $recipients{$_}->{+REL_CC} = BIT_DIRECT foreach (@ccs);
+    $recipients{$_->id}->{+REL_CC} = BIT_DIRECT foreach (@ccs);
     
     # Reporter (there's only ever one)
-    $recipients{$reporter}->{+REL_REPORTER} = BIT_DIRECT;
+    $recipients{$bug->reporter->id}->{+REL_REPORTER} = BIT_DIRECT;
     
     # QA Contact
     if (Bugzilla->params->{'useqacontact'}) {
         foreach (@qa_contacts) {
             # QA Contact can be blank; ignore it if so.
-            $recipients{$_}->{+REL_QA} = BIT_DIRECT if $_;
+            $recipients{$_->id}->{+REL_QA} = BIT_DIRECT if $_;
         }
     }
 
     # Assignee
-    $recipients{$_}->{+REL_ASSIGNEE} = BIT_DIRECT foreach (@assignees);
+    $recipients{$_->id}->{+REL_ASSIGNEE} = BIT_DIRECT foreach (@assignees);
 
     # The last relevant set of people are those who are being removed from 
     # their roles in this change. We get their names out of the diffs.
@@ -420,7 +342,7 @@ sub Send {
 
     Bugzilla::Hook::process('bugmail_recipients',
                             { bug => $bug, recipients => \%recipients });
-    
+
     # Find all those user-watching anyone on the current list, who is not
     # on it already themselves.
     my $involved = join(",", keys %recipients);
@@ -497,18 +419,19 @@ sub Send {
             # dep checks passed.
             if ($user->email_enabled && $dep_ok) {
                 # OK, OK, if we must. Email the user.
-                $sent_mail = sendMail($user, 
-                                      \@headerlist,
-                                      \%rels_which_want, 
-                                      \%values,
-                                      \%defmailhead, 
-                                      \%fielddescription, 
-                                      \@diffparts,
-                                      $comments,
-                                      ! $start, 
-                                      $id,
-                                      exists $watching{$user_id} ?
-                                             $watching{$user_id} : undef);
+                $sent_mail = sendMail(
+                    { to       => $user, 
+                      fields   => \@fields,
+                      bug      => $bug,
+                      comments => $comments,
+                      is_new   => !$start,
+                      changer  => $changer,
+                      watchers => exists $watching{$user_id} ?
+                                  $watching{$user_id} : undef,
+                      diff_parts      => \@diffparts,
+                      rels_which_want => \%rels_which_want,
+                      changed_fields  => \@changedfields,
+                    });
             }
         }
        
@@ -522,21 +445,25 @@ sub Send {
     
     $dbh->do('UPDATE bugs SET lastdiffed = ? WHERE bug_id = ?',
              undef, ($end, $id));
+    $bug->{lastdiffed} = $end;
 
     return {'sent' => \@sent, 'excluded' => \@excluded};
 }
 
 sub sendMail {
-    my ($user, $hlRef, $relRef, $valueRef, $dmhRef, $fdRef,
-        $diffRef, $comments_in, $isnew, $id, $watchingRef) = @_;
-
-    my @send_comments = @$comments_in;
-    my %values = %$valueRef;
-    my @headerlist = @$hlRef;
-    my %mailhead = %$dmhRef;
-    my %fielddescription = %$fdRef;
-    my @diffparts = @$diffRef;    
+    my $params = shift;
     
+    my $user   = $params->{to};
+    my @fields = @{ $params->{fields} };
+    my $bug    = $params->{bug};
+    my @send_comments = @{ $params->{comments} };
+    my $isnew   = $params->{is_new};
+    my $changer = $params->{changer};
+    my $watchingRef = $params->{watchers};
+    my @diffparts   = @{ $params->{diff_parts} };
+    my $relRef      = $params->{rels_which_want};
+    my @changed_fields = @{ $params->{changed_fields} };
+
     # Build difftext (the actions) by verifying the user should see them
     my $difftext = "";
     my $diffheader = "";
@@ -584,14 +511,31 @@ sub sendMail {
     $diffs =~ s/^\n+//s; $diffs =~ s/\n+$//s;
     if ($isnew) {
         my $head = "";
-        foreach my $f (@headerlist) {
-            next unless $mailhead{$f};
-            my $value = $values{$f};
+        foreach my $field (@fields) {
+            my $name = $field->name;
+            my $value = $bug->$name;
+
+            if (ref $value eq 'ARRAY') {
+                $value = join(', ', @$value);
+            }
+            elsif (ref $value && $value->isa('Bugzilla::User')) {
+                $value = $value->login;
+            }
+            elsif (ref $value && $value->isa('Bugzilla::Object')) {
+                $value = $value->name;
+            }
+            elsif ($name eq 'estimated_time') {
+                $value = format_time_decimal($value);
+            }
+            elsif ($name eq 'deadline') {
+                $value = time2str("%Y-%m-%d", str2time($value));
+            }
+
             # If there isn't anything to show, don't include this header.
             next unless $value;
             # Only send estimated_time if it is enabled and the user is in the group.
-            if (($f ne 'estimated_time' && $f ne 'deadline') || $user->is_timetracker) {
-                my $desc = $fielddescription{$f};
+            if (($name ne 'estimated_time' && $name ne 'deadline') || $user->is_timetracker) {
+                my $desc = $field->description;
                 $head .= multiline_sprintf(FORMAT_DOUBLE, ["$desc:", $value],
                                            FORMAT_2_SIZE);
             }
@@ -615,31 +559,16 @@ sub sendMail {
     my $vars = {
         isnew => $isnew,
         to_user => $user,
-        bugid => $id,
-        alias => Bugzilla->params->{'usebugaliases'} ? $values{'alias'} : "",
-        classification => $values{'classification'},
-        product => $values{'product'},
-        comp => $values{'component'},
-        keywords => $values{'keywords'},
-        severity => $values{'bug_severity'},
-        status => $values{'bug_status'},
-        priority => $values{'priority'},
-        assignedto => $values{'assigned_to'},
-        assignedtoname => Bugzilla::User->new({name => $values{'assigned_to'}})->name,
-        targetmilestone => $values{'target_milestone'},
-        changedfields => $values{'changed_fields'},
-        summary => $values{'short_desc'},
+        bug => $bug,
+        changedfields => @changed_fields,
         reasons => \@reasons,
         reasons_watch => \@reasons_watch,
         reasonsheader => join(" ", @headerrel),
         reasonswatchheader => join(" ", @watchingrel),
-        changer => $values{'changer'},
-        changername => $values{'changername'},
-        reporter => $values{'reporter'},
-        reportername => Bugzilla::User->new({name => $values{'reporter'}})->name,
+        changer => $changer,
         diffs => $diffs,
         new_comments => \@send_comments,
-        threadingmarker => build_thread_marker($id, $user->id, $isnew),
+        threadingmarker => build_thread_marker($bug->id, $user->id, $isnew),
     };
 
     my $msg;
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 351dddfae..d02dc947b 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1427,7 +1427,7 @@ sub wants_bug_mail {
     # 
     # We do them separately because if _any_ of them are set, we don't want
     # the mail.
-    if ($wants_mail && $changer && ($self->login eq $changer)) {
+    if ($wants_mail && $changer && ($self->id == $changer->id)) {
         $wants_mail &= $self->wants_mail([EVT_CHANGED_BY_ME], $relationship);
     }    
     
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 58a39bf21..ee5591027 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -430,7 +430,7 @@ sub create {
     Bugzilla->login(LOGIN_REQUIRED);
     $params = Bugzilla::Bug::map_fields($params);
     my $bug = Bugzilla::Bug->create($params);
-    Bugzilla::BugMail::Send($bug->bug_id, { changer => $bug->reporter->login });
+    Bugzilla::BugMail::Send($bug->bug_id, { changer => $bug->reporter });
     return { id => $self->type('int', $bug->bug_id) };
 }
 
@@ -520,7 +520,7 @@ sub add_comment {
     $dbh->bz_commit_transaction();
     
     # Send mail.
-    Bugzilla::BugMail::Send($bug->bug_id, { changer => Bugzilla->user->login });
+    Bugzilla::BugMail::Send($bug->bug_id, { changer => Bugzilla->user });
     
     return { id => $self->type('int', $new_comment_id) };
 }
@@ -566,7 +566,7 @@ sub update_see_also {
             $changes{$bug->id}->{see_also} = { added => [], removed => [] };
         }
 
-        Bugzilla::BugMail::Send($bug->id, { changer => $user->login });
+        Bugzilla::BugMail::Send($bug->id, { changer => $user });
     }
 
     return { changes => \%changes };
diff --git a/attachment.cgi b/attachment.cgi
index 17846866f..b79c649f4 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -538,7 +538,7 @@ sub insert {
   $vars->{'header_done'} = 1;
   $vars->{'contenttypemethod'} = $cgi->param('contenttypemethod');
 
-  my $recipients =  { 'changer' => $user->login, 'owner' => $owner };
+  my $recipients =  { 'changer' => $user, 'owner' => $owner };
   $vars->{'sent_bugmail'} = Bugzilla::BugMail::Send($bugid, $recipients);
 
   print $cgi->header();
@@ -666,7 +666,7 @@ sub update {
     $vars->{'bugs'} = [$bug];
     $vars->{'header_done'} = 1;
     $vars->{'sent_bugmail'} = 
-        Bugzilla::BugMail::Send($bug->id, { 'changer' => $user->login });
+        Bugzilla::BugMail::Send($bug->id, { 'changer' => $user });
 
     print $cgi->header();
 
@@ -739,7 +739,7 @@ sub delete_attachment {
         $vars->{'header_done'} = 1;
 
         $vars->{'sent_bugmail'} =
-            Bugzilla::BugMail::Send($bug->id, { 'changer' => $user->login });
+            Bugzilla::BugMail::Send($bug->id, { 'changer' => $user });
 
         $template->process("attachment/updated.html.tmpl", $vars)
           || ThrowTemplateError($template->error());
diff --git a/contrib/sendbugmail.pl b/contrib/sendbugmail.pl
index f4bebabc6..51de9407d 100755
--- a/contrib/sendbugmail.pl
+++ b/contrib/sendbugmail.pl
@@ -56,13 +56,14 @@ if ($changer !~ /$match/) {
     print STDERR "Changer \"$changer\" doesn't match email regular expression.\n";
     usage();
 }
-if(!login_to_id($changer)) {
-    print STDERR "\"$changer\" is not a login ID.\n";
+my $changer_user = new Bugzilla::User({ name => $changer });
+unless ($changer_user) {
+    print STDERR "\"$changer\" is not a valid user.\n";
     usage();
 }
 
 # Send the email.
-my $outputref = Bugzilla::BugMail::Send($bugnum, {'changer' => $changer });
+my $outputref = Bugzilla::BugMail::Send($bugnum, {'changer' => $changer_user });
 
 # Report the results.
 my $sent = scalar(@{$outputref->{sent}});
diff --git a/editusers.cgi b/editusers.cgi
index 7ce6bb9d2..1950446c7 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -647,7 +647,7 @@ if ($action eq 'search') {
     # Send mail about what we've done to bugs.
     # The deleted user is not notified of the changes.
     foreach (keys(%updatedbugs)) {
-        Bugzilla::BugMail::Send($_, {'changer' => $user->login} );
+        Bugzilla::BugMail::Send($_, {'changer' => $user} );
     }
 
 ###########################################################################
diff --git a/email_in.pl b/email_in.pl
index 8e2c00053..4e44222d5 100755
--- a/email_in.pl
+++ b/email_in.pl
@@ -422,7 +422,7 @@ handle_attachments($bug, $attachments, $comment);
 # to wait for $bug->update() to be fully used in email_in.pl first. So
 # currently, process_bug.cgi does the mail sending for bugs, and this does
 # any mail sending for attachments after the first one.
-Bugzilla::BugMail::Send($bug->id, { changer => Bugzilla->user->login });
+Bugzilla::BugMail::Send($bug->id, { changer => Bugzilla->user });
 debug_print("Sent bugmail");
 
 
diff --git a/extensions/Voting/Extension.pm b/extensions/Voting/Extension.pm
index 5d455ad36..97e061313 100644
--- a/extensions/Voting/Extension.pm
+++ b/extensions/Voting/Extension.pm
@@ -619,7 +619,7 @@ sub _update_votes {
     foreach my $bug_id (@updated_bugs) {
         $vars->{'id'} = $bug_id;
         $vars->{'sent_bugmail'} = 
-            Bugzilla::BugMail::Send($bug_id, { 'changer' => $user->login });
+            Bugzilla::BugMail::Send($bug_id, { 'changer' => $user });
         
         $template->process("bug/process/results.html.tmpl", $vars)
           || ThrowTemplateError($template->error());
@@ -729,7 +729,7 @@ sub _modify_bug_votes {
     # And send out emails about changed bugs
     foreach my $bug_id (@updated_bugs) {
         my $sent_bugmail = Bugzilla::BugMail::Send(
-            $bug_id, { changer => Bugzilla->user->login });
+            $bug_id, { changer => Bugzilla->user });
         $changes->{'confirmed_bugs_sent_bugmail'}->{$bug_id} = $sent_bugmail;
     }
 }
diff --git a/importxml.pl b/importxml.pl
index aff475b70..33d385791 100755
--- a/importxml.pl
+++ b/importxml.pl
@@ -1285,7 +1285,7 @@ sub process_bug {
     }
     Debug( $log, OK_LEVEL );
     push(@logs, $log);
-    Bugzilla::BugMail::Send( $id, { 'changer' => $exporter_login } ) if ($mail);
+    Bugzilla::BugMail::Send( $id, { 'changer' => $exporter } ) if ($mail);
 
     # done with the xml data. Lets clear it from memory
     $twig->purge;
diff --git a/post_bug.cgi b/post_bug.cgi
index 0f78cc5cd..c097a96ce 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -245,7 +245,7 @@ if ($token) {
              ("createbug:$id", $token));
 }
 
-my $recipients = { changer => $user->login };
+my $recipients = { changer => $user };
 my $bug_sent = Bugzilla::BugMail::Send($id, $recipients);
 $bug_sent->{type} = 'created';
 $bug_sent->{id}   = $id;
diff --git a/process_bug.cgi b/process_bug.cgi
index 3f1e81bcb..51664e3c5 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -469,7 +469,7 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) {
 
     # Now send emails.
     foreach my $bug (@bug_objects) {
-        $vars->{'mailrecipients'} = { 'changer' => $user->login };
+        $vars->{'mailrecipients'} = { 'changer' => $user };
         $vars->{'id'} = $bug->id;
         $vars->{'type'} = "move";
         send_results($bug->id, $vars);
@@ -589,7 +589,7 @@ foreach my $bug (@bug_objects) {
         cc        => [split(/[\s,]+/, $old_cc)],
         owner     => $old_own,
         qacontact => $old_qa,
-        changer   => Bugzilla->user->login };
+        changer   => Bugzilla->user };
 
     $vars->{'id'} = $bug->id;
     $vars->{'type'} = "bug";
@@ -602,7 +602,7 @@ foreach my $bug (@bug_objects) {
     # other bug of any changes to that bug.
     my $new_dup_id = $changes->{'dup_id'} ? $changes->{'dup_id'}->[1] : undef;
     if ($new_dup_id) {
-        $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login }; 
+        $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user }; 
 
         $vars->{'id'} = $new_dup_id;
         $vars->{'type'} = "dupe";
@@ -614,7 +614,7 @@ foreach my $bug (@bug_objects) {
 
     my %all_dep_changes = (%notify_deps, %changed_deps);
     foreach my $id (sort { $a <=> $b } (keys %all_dep_changes)) {
-        $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login };
+        $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user };
         $vars->{'id'} = $id;
         $vars->{'type'} = "dep";
 
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index c298d1250..63872bd49 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -248,7 +248,7 @@ if ($cgi->param('rescanallBugMail')) {
     # and so choosing this user as being the last one having done a change
     # for the bug may be problematic. So the best we can do at this point
     # is to choose the currently logged in user for email notification.
-    $vars->{'changer'} = Bugzilla->user->login;
+    $vars->{'changer'} = Bugzilla->user;
 
     foreach my $bugid (@$list) {
         Bugzilla::BugMail::Send($bugid, $vars);
diff --git a/template/en/default/email/newchangedmail.txt.tmpl b/template/en/default/email/newchangedmail.txt.tmpl
index 7d30b890d..4163306f4 100644
--- a/template/en/default/email/newchangedmail.txt.tmpl
+++ b/template/en/default/email/newchangedmail.txt.tmpl
@@ -23,26 +23,26 @@
 
 From: [% Param('mailfrom') %]
 To: [% to_user.email %]
-Subject: [[% terms.Bug %] [%+ bugid %]] [% 'New: ' IF isnew %][%+ summary %]
+Subject: [[% terms.Bug %] [%+ bug.id %]] [% 'New: ' IF isnew %][%+ bug.short_desc %]
 X-Bugzilla-Reason: [% reasonsheader %]
 X-Bugzilla-Type: [% isnew ? 'new' : 'changed' %]
 X-Bugzilla-Watch-Reason: [% reasonswatchheader %]
 [% IF Param('useclassification') %]
-X-Bugzilla-Classification: [% classification %]
+X-Bugzilla-Classification: [% bug.classification %]
 [% END %]
-X-Bugzilla-Product: [% product %]
-X-Bugzilla-Component: [% comp %]
-X-Bugzilla-Keywords: [% keywords %]
-X-Bugzilla-Severity: [% severity %]
-X-Bugzilla-Who: [% changer %]
-X-Bugzilla-Status: [% status %]
-X-Bugzilla-Priority: [% priority %]
-X-Bugzilla-Assigned-To: [% assignedto %]
-X-Bugzilla-Target-Milestone: [% targetmilestone %]
+X-Bugzilla-Product: [% bug.product %]
+X-Bugzilla-Component: [% bug.component %]
+X-Bugzilla-Keywords: [% bug.keywords %]
+X-Bugzilla-Severity: [% bug.bug_severity %]
+X-Bugzilla-Who: [% changer.login %]
+X-Bugzilla-Status: [% bug.bug_status %]
+X-Bugzilla-Priority: [% bug.priority %]
+X-Bugzilla-Assigned-To: [% bug.assigned_to.login %]
+X-Bugzilla-Target-Milestone: [% bug.target_milestone %]
 X-Bugzilla-Changed-Fields: [% changedfields %]
 [%+ threadingmarker %]
 
-[%+ urlbase %]show_bug.cgi?id=[% bugid %]
+[%+ urlbase %]show_bug.cgi?id=[% bug.id %]
 [%- IF diffs %]
 
 [%+ diffs %]