]> git.ipfire.org Git - thirdparty/bugzilla.git/commitdiff
Bug 466968: Remove hardcoded strings from BugMail.pm, and refactor it so that bugmail...
authorFrédéric Buclin <LpSolit@gmail.com>
Fri, 6 Aug 2010 10:41:54 +0000 (12:41 +0200)
committerFrédéric Buclin <LpSolit@gmail.com>
Fri, 6 Aug 2010 10:41:54 +0000 (12:41 +0200)
r/a=mkanat

Bugzilla/BugMail.pm
Bugzilla/Template.pm
Bugzilla/User.pm
Bugzilla/Util.pm
template/en/default/email/newchangedmail.txt.tmpl

index 5688c1e7fbe0eb76d6d66dca77f1c28df713c2cd..84bb62e3c91a5e9d395829bb9d29089540aaf604 100644 (file)
@@ -28,6 +28,7 @@
 #                 Gervase Markham <gerv@gerv.net>
 #                 Byron Jones <bugzilla@glob.com.au>
 #                 Reed Loden <reed@reedloden.com>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 use strict;
 
@@ -38,58 +39,18 @@ use Bugzilla::User;
 use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Bug;
-use Bugzilla::Classification;
-use Bugzilla::Product;
-use Bugzilla::Component;
-use Bugzilla::Status;
+use Bugzilla::Comment;
 use Bugzilla::Mailer;
 use Bugzilla::Hook;
 
 use Date::Parse;
 use Date::Format;
-
-use constant FORMAT_TRIPLE => "%19s|%-28s|%-28s";
-use constant FORMAT_3_SIZE => [19,28,28];
-use constant FORMAT_DOUBLE => "%19s %-55s";
-use constant FORMAT_2_SIZE => [19,55];
+use Scalar::Util qw(blessed);
+use List::MoreUtils qw(uniq);
 
 use constant BIT_DIRECT    => 1;
 use constant BIT_WATCHING  => 2;
 
-# We use this instead of format because format doesn't deal well with
-# multi-byte languages.
-sub multiline_sprintf {
-    my ($format, $args, $sizes) = @_;
-    my @parts;
-    my @my_sizes = @$sizes; # Copy this so we don't modify the input array.
-    foreach my $string (@$args) {
-        my $size = shift @my_sizes;
-        my @pieces = split("\n", wrap_hard($string, $size));
-        push(@parts, \@pieces);
-    }
-
-    my $formatted;
-    while (1) {
-        # Get the first item of each part.
-        my @line = map { shift @$_ } @parts;
-        # If they're all undef, we're done.
-        last if !grep { defined $_ } @line;
-        # Make any single undef item into ''
-        @line = map { defined $_ ? $_ : '' } @line;
-        # And append a formatted line
-        $formatted .= sprintf($format, @line);
-        # Remove trailing spaces, or they become lots of =20's in 
-        # quoted-printable emails.
-        $formatted =~ s/\s+$//;
-        $formatted .= "\n";
-    }
-    return $formatted;
-}
-
-sub three_columns {
-    return multiline_sprintf(FORMAT_TRIPLE, \@_, FORMAT_3_SIZE);
-}
-
 sub relationships {
     my $ref = RELATIONSHIPS;
     # Clone it so that we don't modify the constant;
@@ -143,22 +104,26 @@ sub Send {
             push(@ccs, Bugzilla::User->check($cc));
         }
     }
-    
-    my ($diffparts, $changedfields, $diffs) =
-      $params->{dep_only} ? ([], [], []) : _get_diffs($bug, $end);
-
-    if ($params->{dep_only} && $start) {
-        my $blocker_id = $params->{blocker}->id;
-        my $deptext =
-          "\nBug " . $bug->id . " depends on bug $blocker_id, which changed state.\n\n" .
-          "Bug $blocker_id Summary: " . $params->{blocker}->short_desc . "\n" .
-          correct_urlbase() . "show_bug.cgi?id=$blocker_id\n\n";
-        $deptext .= three_columns("What    ", "Old Value", "New Value");
-        $deptext .= ('-' x 76) . "\n";
-        $deptext .= three_columns('Status', @{$params->{changes}->{bug_status}});
-        $deptext .= three_columns('Resolution', @{$params->{changes}->{resolution}});
-
-        push(@$diffparts, {text => $deptext});
+
+    my @diffs;
+    if (!$start) {
+        @diffs = _get_new_bugmail_fields($bug);
+    }
+
+    if ($params->{dep_only}) {
+        push(@diffs, { field_name => 'bug_status',
+                       old => $params->{changes}->{bug_status}->[0],
+                       new => $params->{changes}->{bug_status}->[1],
+                       login_name => $changer->login,
+                       blocker => $params->{blocker} },
+                     { field_name => 'resolution',
+                       old => $params->{changes}->{resolution}->[0],
+                       new => $params->{changes}->{resolution}->[1],
+                       login_name => $changer->login,
+                       blocker => $params->{blocker} });
+    }
+    else {
+        push(@diffs, _get_diffs($bug, $end));
     }
 
     my $comments = $bug->comments({ after => $start, to => $end });
@@ -194,24 +159,23 @@ sub Send {
 
     # 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.
-    foreach my $ref (@$diffs) {
-        my ($who, $whoname, $what, $when, $old, $new) = (@$ref);
-        if ($old) {
+    foreach my $change (@diffs) {
+        if ($change->{old}) {
             # You can't stop being the reporter, so we don't check that
             # relationship here.
             # Ignore people whose user account has been deleted or renamed.
-            if ($what eq "CC") {
-                foreach my $cc_user (split(/[\s,]+/, $old)) {
+            if ($change->{field_name} eq 'cc') {
+                foreach my $cc_user (split(/[\s,]+/, $change->{old})) {
                     my $uid = login_to_id($cc_user);
                     $recipients{$uid}->{+REL_CC} = BIT_DIRECT if $uid;
                 }
             }
-            elsif ($what eq "QAContact") {
-                my $uid = login_to_id($old);
+            elsif ($change->{field_name} eq 'qa_contact') {
+                my $uid = login_to_id($change->{old});
                 $recipients{$uid}->{+REL_QA} = BIT_DIRECT if $uid;
             }
-            elsif ($what eq "AssignedTo") {
-                my $uid = login_to_id($old);
+            elsif ($change->{field_name} eq 'assigned_to') {
+                my $uid = login_to_id($change->{old});
                 $recipients{$uid}->{+REL_ASSIGNEE} = BIT_DIRECT if $uid;
             }
         }
@@ -251,9 +215,6 @@ sub Send {
     my @sent;
     my @excluded;
 
-    # Only used for headers in bugmail for new bugs
-    my @fields = $start ? () : Bugzilla->get_fields({obsolete => 0, mailhead => 1});
-
     foreach my $user_id (keys %recipients) {
         my %rels_which_want;
         my $sent_mail = 0;
@@ -265,13 +226,12 @@ sub Send {
             # Go through each role the user has and see if they want mail in
             # that role.
             foreach my $relationship (keys %{$recipients{$user_id}}) {
-                if ($user->wants_bug_mail($id,
+                if ($user->wants_bug_mail($bug,
                                           $relationship, 
-                                          $diffs, 
+                                          $start ? \@diffs : [],
                                           $comments,
                                           $params->{dep_only},
-                                          $changer,
-                                          !$start))
+                                          $changer))
                 {
                     $rels_which_want{$relationship} = 
                         $recipients{$user_id}->{$relationship};
@@ -291,27 +251,23 @@ sub Send {
                 $dep_ok = $user->can_see_bug($params->{blocker}->id) ? 1 : 0;
             }
 
-            # Make sure the user isn't in the nomail list, and the insider and 
-            # dep checks passed.
+            # 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, 
-                      fields   => \@fields,
                       bug      => $bug,
                       comments => $comments,
-                      is_new   => !$start,
                       delta_ts => $params->{dep_only} ? $end : undef,
                       changer  => $changer,
                       watchers => exists $watching{$user_id} ?
                                   $watching{$user_id} : undef,
-                      diff_parts      => $diffparts,
+                      diffs    => \@diffs,
                       rels_which_want => \%rels_which_want,
-                      changed_fields  => $changedfields,
                     });
             }
         }
-       
+
         if ($sent_mail) {
             push(@sent, $user->login); 
         } 
@@ -336,96 +292,38 @@ sub sendMail {
     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 $delta_ts = $params->{delta_ts};
     my $changer = $params->{changer};
     my $watchingRef = $params->{watchers};
-    my @diffparts   = @{ $params->{diff_parts} };
+    my @diffs = @{ $params->{diffs} };
     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 = "";
-    my $add_diff;
+    # Only display changes the user is allowed see.
+    my @display_diffs;
 
-    foreach my $diff (@diffparts) {
-        $add_diff = 0;
+    foreach my $diff (@diffs) {
+        my $add_diff = 0;
         
-        if (exists($diff->{'fieldname'}) && 
-            ($diff->{'fieldname'} eq 'estimated_time' ||
-             $diff->{'fieldname'} eq 'remaining_time' ||
-             $diff->{'fieldname'} eq 'work_time' ||
-             $diff->{'fieldname'} eq 'deadline'))
-        {
+        if (grep { $_ eq $diff->{field_name} } TIMETRACKING_FIELDS) {
             $add_diff = 1 if $user->is_timetracker;
-        } elsif ($diff->{'isprivate'} 
-                 && !$user->is_insider)
-        {
-            $add_diff = 0;
-        } else {
-            $add_diff = 1;
         }
-
-        if ($add_diff) {
-            if (exists($diff->{'header'}) && 
-             ($diffheader ne $diff->{'header'})) {
-                $diffheader = $diff->{'header'};
-                $difftext .= $diffheader;
-            }
-            $difftext .= $diff->{'text'};
+        elsif (!$diff->{isprivate} || $user->is_insider) {
+            $add_diff = 1;
         }
+        push(@display_diffs, $diff) if $add_diff;
     }
 
     if (!$user->is_insider) {
         @send_comments = grep { !$_->is_private } @send_comments;
     }
 
-    if ($difftext eq "" && !scalar(@send_comments) && !$isnew) {
+    if (!scalar(@display_diffs) && !scalar(@send_comments)) {
       # Whoops, no differences!
       return 0;
     }
 
-    my $diffs = $difftext;
-    # Remove extra newlines.
-    $diffs =~ s/^\n+//s; $diffs =~ s/\n+$//s;
-    if ($isnew) {
-        my $head = "";
-        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 = ($value == 0) ? 0 : format_time_decimal($value);
-            }
-            elsif ($name eq 'deadline') {
-                $value = time2str("%Y-%m-%d", str2time($value)) if $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 (($name ne 'estimated_time' && $name ne 'deadline') || $user->is_timetracker) {
-                my $desc = $field->description;
-                $head .= multiline_sprintf(FORMAT_DOUBLE, ["$desc:", $value],
-                                           FORMAT_2_SIZE);
-            }
-        }
-        $diffs = $head . ($difftext ? "\n\n" : "") . $diffs;
-    }
-
     my (@reasons, @reasons_watch);
     while (my ($relationship, $bits) = each %{$relRef}) {
         push(@reasons, $relationship) if ($bits & BIT_DIRECT);
@@ -440,19 +338,18 @@ sub sendMail {
     push @watchingrel, map { user_id_to_login($_) } @$watchingRef;
 
     my $vars = {
-        isnew => $isnew,
         delta_ts => $delta_ts,
         to_user => $user,
         bug => $bug,
-        changedfields => \@changed_fields,
         reasons => \@reasons,
         reasons_watch => \@reasons_watch,
         reasonsheader => join(" ", @headerrel),
         reasonswatchheader => join(" ", @watchingrel),
         changer => $changer,
-        diffs => $diffs,
+        diffs => \@display_diffs,
+        changedfields => [uniq map { $_->{field_name} } @display_diffs],
         new_comments => \@send_comments,
-        threadingmarker => build_thread_marker($bug->id, $user->id, $isnew),
+        threadingmarker => build_thread_marker($bug->id, $user->id, !$bug->lastdiffed),
     };
 
     my $msg;
@@ -478,9 +375,9 @@ sub _get_diffs {
     }
 
     my $diffs = $dbh->selectall_arrayref(
-           "SELECT profiles.login_name, profiles.realname, fielddefs.description,
-                   bugs_activity.bug_when, bugs_activity.removed,
-                   bugs_activity.added, bugs_activity.attach_id, fielddefs.name,
+           "SELECT profiles.login_name, profiles.realname, fielddefs.name AS field_name,
+                   bugs_activity.bug_when, bugs_activity.removed AS old,
+                   bugs_activity.added AS new, bugs_activity.attach_id,
                    bugs_activity.comment_id
               FROM bugs_activity
         INNER JOIN fielddefs
@@ -489,50 +386,58 @@ sub _get_diffs {
                 ON profiles.userid = bugs_activity.who
              WHERE bugs_activity.bug_id = ?
                    $when_restriction
-          ORDER BY bugs_activity.bug_when", undef, @args);
-
-    my $difftext = "";
-    my $diffheader = "";
-    my @diffparts;
-    my $lastwho = "";
-    my $fullwho;
-    my @changedfields;
-    foreach my $ref (@$diffs) {
-        my ($who, $whoname, $what, $when, $old, $new, $attachid, $fieldname, $comment_id) = @$ref;
-        my $diffpart = {};
-        if ($who ne $lastwho) {
-            $lastwho = $who;
-            $fullwho = $whoname ? "$whoname <$who>" : $who;
-            $diffheader = "\n$fullwho changed:\n\n";
-            $diffheader .= three_columns("What    ", "Removed", "Added");
-            $diffheader .= ('-' x 76) . "\n";
+          ORDER BY bugs_activity.bug_when", {Slice=>{}}, @args);
+
+    foreach my $diff (@$diffs) {
+        if ($diff->{attach_id}) {
+            $diff->{isprivate} = $dbh->selectrow_array(
+                'SELECT isprivate FROM attachments WHERE attach_id = ?',
+                undef, $diff->{attach_id});
+         }
+         if ($diff->{field_name} eq 'longdescs.isprivate') {
+             my $comment = Bugzilla::Comment->new($diff->{comment_id});
+             $diff->{num} = $comment->count;
+             $diff->{isprivate} = $diff->{new};
+         }
+    }
+
+    return @$diffs;
+}
+
+sub _get_new_bugmail_fields {
+    my $bug = shift;
+    my @fields = Bugzilla->get_fields({obsolete => 0, mailhead => 1});
+    my @diffs;
+
+    foreach my $field (@fields) {
+        my $name = $field->name;
+        my $value = $bug->$name;
+
+        if (ref $value eq 'ARRAY') {
+            $value = join(', ', @$value);
         }
-        $what =~ s/^(Attachment )?/Attachment #$attachid / if $attachid;
-        if( $fieldname eq 'estimated_time' ||
-            $fieldname eq 'remaining_time' ) {
-            $old = format_time_decimal($old);
-            $new = format_time_decimal($new);
+        elsif (blessed($value) && $value->isa('Bugzilla::User')) {
+            $value = $value->login;
         }
-        if ($attachid) {
-            ($diffpart->{'isprivate'}) = $dbh->selectrow_array(
-                'SELECT isprivate FROM attachments WHERE attach_id = ?',
-                undef, ($attachid));
+        elsif (blessed($value) && $value->isa('Bugzilla::Object')) {
+            $value = $value->name;
+        }
+        elsif ($name eq 'estimated_time') {
+            # "0.00" (which is what we get from the DB) is true,
+            # so we explicitly do a numerical comparison with 0.
+            $value = 0 if $value == 0;
         }
-        if ($fieldname eq 'longdescs.isprivate') {
-            my $comment = Bugzilla::Comment->new($comment_id);
-            my $comment_num = $comment->count;
-            $what =~ s/^(Comment )?/Comment #$comment_num /;
-            $diffpart->{'isprivate'} = $new;
+        elsif ($name eq 'deadline') {
+            $value = time2str("%Y-%m-%d", str2time($value)) if $value;
         }
-        $difftext = three_columns($what, $old, $new);
-        $diffpart->{'header'} = $diffheader;
-        $diffpart->{'fieldname'} = $fieldname;
-        $diffpart->{'text'} = $difftext;
-        push(@diffparts, $diffpart);
-        push(@changedfields, $what);
+
+        # If there isn't anything to show, don't include this header.
+        next unless $value;
+
+        push(@diffs, {field_name => $name, new => $value});
     }
 
-    return (\@diffparts, \@changedfields, $diffs);
+    return @diffs;
 }
 
 1;
index 923336d4591f06d78109a7c715ba00d946dd08a5..c964dd0222aca06958d95b8fd7cd02cd24156478 100644 (file)
@@ -61,6 +61,11 @@ use Scalar::Util qw(blessed);
 
 use base qw(Template);
 
+use constant FORMAT_TRIPLE => '%19s|%-28s|%-28s';
+use constant FORMAT_3_SIZE => [19,28,28];
+use constant FORMAT_DOUBLE => '%19s %-55s';
+use constant FORMAT_2_SIZE => [19,55];
+
 # Convert the constants in the Bugzilla::Constants module into a hash we can
 # pass to the template object for reflection into its "constants" namespace
 # (which is like its "variables" namespace, but for constants).  To do so, we
@@ -352,6 +357,36 @@ sub get_bug_link {
     return qq{$pre<a href="$linkval" title="$title">$link_text</a>$post};
 }
 
+# We use this instead of format because format doesn't deal well with
+# multi-byte languages.
+sub multiline_sprintf {
+    my ($format, $args, $sizes) = @_;
+    my @parts;
+    my @my_sizes = @$sizes; # Copy this so we don't modify the input array.
+    foreach my $string (@$args) {
+        my $size = shift @my_sizes;
+        my @pieces = split("\n", wrap_hard($string, $size));
+        push(@parts, \@pieces);
+    }
+
+    my $formatted;
+    while (1) {
+        # Get the first item of each part.
+        my @line = map { shift @$_ } @parts;
+        # If they're all undef, we're done.
+        last if !grep { defined $_ } @line;
+        # Make any single undef item into ''
+        @line = map { defined $_ ? $_ : '' } @line;
+        # And append a formatted line
+        $formatted .= sprintf($format, @line);
+        # Remove trailing spaces, or they become lots of =20's in
+        # quoted-printable emails.
+        $formatted =~ s/\s+$//;
+        $formatted .= "\n";
+    }
+    return $formatted;
+}
+
 #####################
 # Header Generation #
 #####################
@@ -833,6 +868,14 @@ sub create {
             # Function to create date strings
             'time2str' => \&Date::Format::time2str,
 
+            # Fixed size column formatting for bugmail.
+            'format_columns' => sub {
+                my $cols = shift;
+                my $format = ($cols == 3) ? FORMAT_TRIPLE : FORMAT_DOUBLE;
+                my $col_size = ($cols == 3) ? FORMAT_3_SIZE : FORMAT_2_SIZE;
+                return multiline_sprintf($format, \@_, $col_size);
+            },
+
             # Generic linear search function
             'lsearch' => sub {
                 my ($array, $item) = @_;
index 2bbe6310187b38d6ea3f8b16882fc70b2d5c0660..595964bf9dba8c1ab4e7c9d67f7c24c0c7bfe693 100644 (file)
@@ -1492,35 +1492,33 @@ sub match_field {
 
 }
 
-# Changes in some fields automatically trigger events. The 'field names' are
-# from the fielddefs table. We really should be using proper field names 
-# throughout.
+# Changes in some fields automatically trigger events. The field names are
+# from the fielddefs table.
 our %names_to_events = (
-    'Resolution'             => EVT_OPENED_CLOSED,
-    'Keywords'               => EVT_KEYWORD,
-    'CC'                     => EVT_CC,
-    'Severity'               => EVT_PROJ_MANAGEMENT,
-    'Priority'               => EVT_PROJ_MANAGEMENT,
-    'Status'                 => EVT_PROJ_MANAGEMENT,
-    'Target Milestone'       => EVT_PROJ_MANAGEMENT,
-    'Attachment description' => EVT_ATTACHMENT_DATA,
-    'Attachment mime type'   => EVT_ATTACHMENT_DATA,
-    'Attachment is patch'    => EVT_ATTACHMENT_DATA,
-    'Depends on'             => EVT_DEPEND_BLOCK,
-    'Blocks'                 => EVT_DEPEND_BLOCK);
+    'resolution'              => EVT_OPENED_CLOSED,
+    'keywords'                => EVT_KEYWORD,
+    'cc'                      => EVT_CC,
+    'bug_severity'            => EVT_PROJ_MANAGEMENT,
+    'priority'                => EVT_PROJ_MANAGEMENT,
+    'bug_status'              => EVT_PROJ_MANAGEMENT,
+    'target_milestone'        => EVT_PROJ_MANAGEMENT,
+    'attachments.description' => EVT_ATTACHMENT_DATA,
+    'attachments.mimetype'    => EVT_ATTACHMENT_DATA,
+    'attachments.ispatch'     => EVT_ATTACHMENT_DATA,
+    'dependson'               => EVT_DEPEND_BLOCK,
+    'blocked'                 => EVT_DEPEND_BLOCK);
 
 # Returns true if the user wants mail for a given bug change.
 # Note: the "+" signs before the constants suppress bareword quoting.
 sub wants_bug_mail {
     my $self = shift;
-    my ($bug_id, $relationship, $fieldDiffs, $comments, $dep_mail,
-        $changer, $bug_is_new) = @_;
+    my ($bug, $relationship, $fieldDiffs, $comments, $dep_mail, $changer) = @_;
 
     # Make a list of the events which have happened during this bug change,
     # from the point of view of this user.    
     my %events;    
-    foreach my $ref (@$fieldDiffs) {
-        my ($who, $whoname, $fieldName, $when, $old, $new) = @$ref;
+    foreach my $change (@$fieldDiffs) {
+        my $fieldName = $change->{field_name};
         # A change to any of the above fields sets the corresponding event
         if (defined($names_to_events{$fieldName})) {
             $events{$names_to_events{$fieldName}} = 1;
@@ -1532,16 +1530,16 @@ sub wants_bug_mail {
 
         # If the user is in a particular role and the value of that role
         # changed, we need the ADDED_REMOVED event.
-        if (($fieldName eq "AssignedTo" && $relationship == REL_ASSIGNEE) ||
-            ($fieldName eq "QAContact" && $relationship == REL_QA)) 
+        if (($fieldName eq "assigned_to" && $relationship == REL_ASSIGNEE) ||
+            ($fieldName eq "qa_contact" && $relationship == REL_QA))
         {
             $events{+EVT_ADDED_REMOVED} = 1;
         }
         
-        if ($fieldName eq "CC") {
+        if ($fieldName eq "cc") {
             my $login = $self->login;
-            my $inold = ($old =~ /^(.*,\s*)?\Q$login\E(,.*)?$/);
-            my $innew = ($new =~ /^(.*,\s*)?\Q$login\E(,.*)?$/);
+            my $inold = ($change->{old} =~ /^(.*,\s*)?\Q$login\E(,.*)?$/);
+            my $innew = ($change->{new} =~ /^(.*,\s*)?\Q$login\E(,.*)?$/);
             if ($inold != $innew)
             {
                 $events{+EVT_ADDED_REMOVED} = 1;
@@ -1549,7 +1547,7 @@ sub wants_bug_mail {
         }
     }
 
-    if ($bug_is_new) {
+    if (!$bug->lastdiffed) {
         # Notify about new bugs.
         $events{+EVT_BUG_CREATED} = 1;
 
@@ -1589,19 +1587,8 @@ sub wants_bug_mail {
         $wants_mail &= $self->wants_mail([EVT_CHANGED_BY_ME], $relationship);
     }    
     
-    if ($wants_mail) {
-        my $dbh = Bugzilla->dbh;
-        # We don't create a Bug object from the bug_id here because we only
-        # need one piece of information, and doing so (as of 2004-11-23) slows
-        # down bugmail sending by a factor of 2. If Bug creation was more
-        # lazy, this might not be so bad.
-        my $bug_status = $dbh->selectrow_array('SELECT bug_status
-                                                FROM bugs WHERE bug_id = ?',
-                                                undef, $bug_id);
-
-        if ($bug_status eq "UNCONFIRMED") {
-            $wants_mail &= $self->wants_mail([EVT_UNCONFIRMED], $relationship);
-        }
+    if ($wants_mail && $bug->bug_status eq 'UNCONFIRMED') {
+        $wants_mail &= $self->wants_mail([EVT_UNCONFIRMED], $relationship);
     }
     
     return $wants_mail;
index f5ab51d2bacfd2970b24b107fb47a9482fda3d3d..6f5686a3bcdea2bb35d948aff48a371abd217bbb 100644 (file)
@@ -39,8 +39,7 @@ use base qw(Exporter);
                              do_ssl_redirect_if_required use_attachbase
                              diff_arrays on_main_db
                              trim wrap_hard wrap_comment find_wrap_point
-                             format_time format_time_decimal validate_date
-                             validate_time datetime_from
+                             format_time validate_date validate_time datetime_from
                              file_mod_time is_7bit_clean
                              bz_crypt generate_random_password
                              validate_email_syntax clean_text
@@ -466,18 +465,6 @@ sub datetime_from {
     return $dt;
 }
 
-sub format_time_decimal {
-    my ($time) = (@_);
-
-    my $newtime = sprintf("%.2f", $time);
-
-    if ($newtime =~ /0\Z/) {
-        $newtime = sprintf("%.1f", $time);
-    }
-
-    return $newtime;
-}
-
 sub file_mod_time {
     my ($filename) = (@_);
     my ($dev,$ino,$mode,$nlink,$uid,$gid,$rdev,$size,
@@ -935,11 +922,6 @@ is used, as defined in his preferences.
 This routine is mainly called from templates to filter dates, see
 "FILTER time" in L<Bugzilla::Template>.
 
-=item C<format_time_decimal($time)>
-
-Returns a number with 2 digit precision, unless the last digit is a 0. Then it 
-returns only 1 digit precision.
-
 =item C<datetime_from($time, $timezone)>
 
 Returns a DateTime object given a date string. If the string is not in some
index 3bd7d789bfa8c8e73999672f4301c63cedd8eedf..de4a248e3ec2b546250023bfc720dc381200e064 100644 (file)
   # Rights Reserved.
   #
   # Contributor(s): André Batosti <batosti@async.com.br>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
-[% PROCESS "global/variables.none.tmpl" %]
+[% PROCESS "global/field-descs.none.tmpl" %]
 [% PROCESS "global/reason-descs.none.tmpl" %]
 
+[% isnew = bug.lastdiffed ? 0 : 1 %]
+
 From: [% Param('mailfrom') %]
 To: [% to_user.email %]
 Subject: [[% terms.Bug %] [%+ bug.id %]] [% 'New: ' IF isnew %][%+ bug.short_desc %]
@@ -43,11 +46,8 @@ X-Bugzilla-Target-Milestone: [% bug.target_milestone %]
 X-Bugzilla-Changed-Fields: [% changedfields.join(" ") %]
 [%+ threadingmarker %]
 
-[%+ urlbase %]show_bug.cgi?id=[% bug.id %]
-[%- IF diffs %]
+[%+ PROCESS generate_diffs -%]
 
-[%+ diffs %]
-[% END -%]
 [% FOREACH comment = new_comments %]
 
 [%- IF comment.count %]
@@ -68,3 +68,52 @@ Configure [% terms.bug %]mail: [% urlbase %]userprefs.cgi?tab=email
        IF watch_reason_descs.$reason %]
 [% END %]
 [%+ reason_lines.join("\n") %]
+
+[% BLOCK generate_diffs %]
+  [% urlbase %]show_bug.cgi?id=[% bug.id %]
+
+[%+ last_changer = "" %]
+  [% FOREACH change = diffs %]
+    [% IF !isnew && change.login_name != last_changer %]
+      [% last_changer = change.login_name %]
+      [% IF change.blocker %]
+        [% terms.Bug %] [%+ bug.id %] depends on [% terms.bug %] [%+ change.blocker.id %], which changed state.
+
+[%+ terms.Bug %] [%+ change.blocker.id %] Summary: [% change.blocker.short_desc %]
+[%+ urlbase %]show_bug.cgi?id=[% change.blocker.id %]
+      [% ELSE %]
+        [%~ IF change.realname %]
+          [% change.realname _ " <" _ change.login_name _ ">" %]
+        [% ELSE %]
+          [% change.login_name %]
+        [% END %] changed:
+      [% END %]
+
+           What    |Removed                     |Added
+----------------------------------------------------------------------------
+[%+ END %][%# End of IF. This indentation is intentional! ~%]
+
+    [% field_label = field_descs.${change.field_name} %]
+    [% old_value = display_value(change.field_name, change.old) %]
+    [% new_value = display_value(change.field_name, change.new) %]
+
+    [%~ IF change.field_name == "estimated_time" || change.field_name == "remaining_time" %]
+      [% old_value = old_value FILTER format('%.2f') %]
+      [% new_value = new_value FILTER format('%.2f') %]
+    [% END %]
+
+    [%~ IF change.attach_id %]
+      [% field_label = field_label.replace('^(Attachment )?', "Attachment #${change.attach_id} ") %]
+    [% END %]
+
+    [%~ IF change.field_name == 'longdescs.isprivate' %]
+      [% field_label = field_label.replace('^(Comment )?', "Comment #${change.num} ") %]
+    [% END %]
+
+    [%~ IF isnew %]
+      [% format_columns(2, field_label _ ":", new_value) -%]
+    [% ELSE %]
+      [% format_columns(3, field_label, old_value, new_value) -%]
+    [% END %]
+  [% END -%]
+[% END %]