Bug 389313: summarize_time.cgi needs some cleanup - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat

4 files changed, 149 insertions, 341 deletions

diff --git a/process_bug.cgi b/process_bug.cgi
index ae7eb23be..0d76a5c70 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -152,7 +152,7 @@ if (defined $cgi->param('id')) {
 }
 
 # Make sure there are bugs to process.
-scalar(@idlist) || ThrowUserError("no_bugs_chosen");
+scalar(@idlist) || ThrowUserError("no_bugs_chosen", {action => 'modify'});
 
 # Build a bug object using the first bug id, for validations.
 my $bug = $bug_objects[0];
diff --git a/summarize_time.cgi b/summarize_time.cgi
index 26cc04725..df1297e5e 100755
--- a/summarize_time.cgi
+++ b/summarize_time.cgi
@@ -15,23 +15,19 @@
 #
 # Contributor(s): Christian Reis <kiko@async.com.br>
 #                 Shane H. W. Travis <travis@sedsystems.ca>
-#
+#                 Frédéric Buclin <LpSolit@gmail.com>
+
 use strict;
 
 use lib qw(.);
 
 use Date::Parse;         # strptime
-use Date::Format;        # strftime
 
 use Bugzilla;
 use Bugzilla::Constants; # LOGIN_*
 use Bugzilla::Bug;       # EmitDependList
 use Bugzilla::Util;      # trim
 use Bugzilla::Error;
-use Bugzilla::User;      # Bugzilla->user->in_group
-
-my $template = Bugzilla->template;
-my $vars = {};
 
 #
 # Date handling
@@ -98,30 +94,6 @@ sub date_adjust_up {
     return ($year, $month, $day);
 }
 
-sub check_dates {
-    my ($start_date, $end_date) = @_;
-    if ($start_date) {
-        if (!str2time($start_date)) {
-            ThrowUserError("illegal_date", {'date' => $start_date});
-        }
-        # This code may strike you as funny. It's actually a workaround
-        # for an "issue" in str2time. If you enter the date 2004-06-31,
-        # even though it's a bogus date (there *are* only 30 days in
-        # June), it will parse and return 2004-07-01. To make this
-        # less painful to the end-user, I do the "normalization" here,
-        # but it might be "surprising" and warrant a warning in the end.
-        $start_date = time2str("%Y-%m-%d", str2time($start_date));
-    } 
-    if ($end_date) {
-        if (!str2time($end_date)) {
-            ThrowUserError("illegal_date", {'date' => $end_date});
-        }
-        # see related comment above.
-        $end_date = time2str("%Y-%m-%d", str2time($end_date));
-    }
-    return ($start_date, $end_date);
-}
-
 sub split_by_month {
     # Takes start and end dates and splits them into a list of
     # monthly-spaced 2-lists of dates.
@@ -175,34 +147,6 @@ sub split_by_month {
     return @months;
 }
 
-sub include_tt_details {
-    my ($res, $bugids, $start_date, $end_date) = @_;
-
-
-    my $dbh = Bugzilla->dbh;
-    my ($date_bits, $date_values) = sqlize_dates($start_date, $end_date);
-    my $buglist = join ", ", @{$bugids};
-
-    my $q = qq{SELECT bugs.bug_id, profiles.login_name, bugs.deadline,
-                      bugs.estimated_time, bugs.remaining_time
-               FROM   longdescs
-               INNER JOIN bugs
-                  ON longdescs.bug_id = bugs.bug_id
-               INNER JOIN profiles
-                  ON longdescs.who = profiles.userid
-               WHERE  longdescs.bug_id in ($buglist) $date_bits};
-
-    my %res = %{$res};
-    my $sth = $dbh->prepare($q);
-    $sth->execute(@{$date_values});
-    while (my $row = $sth->fetch) {
-        $res{$row->[0]}{"deadline"} = $row->[2];
-        $res{$row->[0]}{"estimated_time"} = $row->[3];
-        $res{$row->[0]}{"remaining_time"} = $row->[4];
-    }
-    return \%res;
-}
-
 sub sqlize_dates {
     my ($start_date, $end_date) = @_;
     my $date_bits = "";
@@ -226,172 +170,66 @@ sub sqlize_dates {
     return ($date_bits, \@date_values);
 }
 
-#
-# Dependencies
-#
-
-sub get_blocker_ids_unique {
-    my $bug_id = shift;
-    my @ret = ($bug_id);
-    get_blocker_ids_deep($bug_id, \@ret);
-    my %unique;
-    foreach my $blocker (@ret) {
-        $unique{$blocker} = $blocker
-    }
-    return keys %unique;
-}
-
-sub get_blocker_ids_deep {
-    my ($bug_id, $ret) = @_;
+# Return all blockers of the current bug, recursively.
+sub get_blocker_ids {
+    my ($bug_id, $unique) = @_;
+    $unique ||= {$bug_id => 1};
     my $deps = Bugzilla::Bug::EmitDependList("blocked", "dependson", $bug_id);
-    push @{$ret}, @$deps;
-    foreach $bug_id (@$deps) {
-        get_blocker_ids_deep($bug_id, $ret);
+    my @unseen = grep { !$unique->{$_}++ } @$deps;
+    foreach $bug_id (@unseen) {
+        get_blocker_ids($bug_id, $unique);
     }
+    return keys %$unique;
 }
 
-#
-# Queries and data structure assembly
-#
-
-sub query_work_by_buglist {
-    my ($bugids, $start_date, $end_date) = @_;
+# Return a hashref whose key is chosen by the user (bug ID or commenter)
+# and value is a hash of the form {bug ID, commenter, time spent}.
+# So you can either view it as the time spent by commenters on each bug
+# or the time spent in bugs by each commenter.
+sub get_list {
+    my ($bugids, $start_date, $end_date, $keyname) = @_;
     my $dbh = Bugzilla->dbh;
 
     my ($date_bits, $date_values) = sqlize_dates($start_date, $end_date);
+    my $buglist = join(", ", @$bugids);
 
-    # $bugids is guaranteed to be non-empty because at least one bug is
-    # always provided to this page.
-    my $buglist = join ", ", @{$bugids};
-
-    # Returns the total time worked on each bug *per developer*, with
-    # bug descriptions and developer address
-    my $q = qq{SELECT sum(longdescs.work_time) as total_time,
-                      profiles.login_name, 
-                      longdescs.bug_id,
-                      bugs.short_desc,
-                      bugs.bug_status
-               FROM   longdescs
-               INNER JOIN profiles
+    # Returns the total time worked on each bug *per developer*.
+    my $data = $dbh->selectall_arrayref(
+            qq{SELECT SUM(work_time) AS total_time, login_name, longdescs.bug_id
+                 FROM longdescs
+           INNER JOIN profiles
                    ON longdescs.who = profiles.userid
-               INNER JOIN bugs
+           INNER JOIN bugs
                    ON bugs.bug_id = longdescs.bug_id
-               WHERE  longdescs.bug_id IN ($buglist)
-                      $date_bits } .
-            $dbh->sql_group_by('longdescs.bug_id, profiles.login_name',
-                'bugs.short_desc, bugs.bug_status, longdescs.bug_when') . qq{
-               ORDER BY longdescs.bug_when};
-    my $sth = $dbh->prepare($q);
-    $sth->execute(@{$date_values});
-    return $sth;
-}
-
-sub get_work_by_owners {
-    my $sth = query_work_by_buglist(@_);
-    my %res;
-    while (my $row = $sth->fetch) {
-        # XXX: Why do we need to check if the total time is positive
-        # instead of using SQL to do that?  Simply because MySQL 3.x's
-        # GROUP BY doesn't work correctly with aggregates. This is
-        # really annoying, but I've spent a long time trying to wrestle
-        # with it and it just doesn't seem to work. Should work OK in
-        # 4.x, though.
-        if ($row->[0] > 0) {
-            my $login_name = $row->[1];
-            push @{$res{$login_name}}, { total_time => $row->[0],
-                                         bug_id     => $row->[2],
-                                         short_desc => $row->[3],
-                                         bug_status => $row->[4] };
-        }
-    }
-    return \%res;
-}
-
-sub get_work_by_bugs {
-    my $sth = query_work_by_buglist(@_);
-    my %res;
-    while (my $row = $sth->fetch) {
-        # Perl doesn't let me use arrays as keys :-(
-        # merge in ID, status and summary
-        my $bug = join ";", ($row->[2], $row->[4], $row->[3]);
-        # XXX: see comment in get_work_by_owners
-        if ($row->[0] > 0) {
-            push @{$res{$bug}}, { total_time => $row->[0],
-                                  login_name => $row->[1], };
-        }
-    }
-    return \%res;
+                WHERE longdescs.bug_id IN ($buglist) $date_bits } .
+            $dbh->sql_group_by('longdescs.bug_id, login_name', 'longdescs.bug_when') .
+           qq{ HAVING SUM(work_time) > 0}, {Slice => {}}, @$date_values);
+
+    my %list;
+    # What this loop does is to push data having the same key in an array.
+    push(@{$list{ $_->{$keyname} }}, $_) foreach @$data;
+    return \%list;
 }
 
+# Return bugs which had no activity (a.k.a work_time = 0) during the given time range.
 sub get_inactive_bugs {
     my ($bugids, $start_date, $end_date) = @_;
     my $dbh = Bugzilla->dbh;
     my ($date_bits, $date_values) = sqlize_dates($start_date, $end_date);
-    my $buglist = join ", ", @{$bugids};
-
-    my %res;
-    # This sucks. I need to make sure that even bugs that *don't* show
-    # up in the longdescs query (because no comments were filed during
-    # the specified period) but *are* dependent on the parent bug show
-    # up in the results if they have no work done; that's why I prefill
-    # them in %res here and then remove them below.
-    my $q = qq{SELECT DISTINCT bugs.bug_id, bugs.short_desc ,
-                               bugs.bug_status
-               FROM   longdescs
-               INNER JOIN bugs
-                    ON longdescs.bug_id = bugs.bug_id
-               WHERE  longdescs.bug_id in ($buglist)};
-    my $sth = $dbh->prepare($q);
-    $sth->execute();
-    while (my $row = $sth->fetch) {
-        $res{$row->[0]} = [$row->[1], $row->[2]];
-    }
-
-    # Returns the total time worked on each bug, with description. This
-    # query differs a bit from one in the query_work_by_buglist and I
-    # avoided complicating that one just to make it more general.
-    $q = qq{SELECT sum(longdescs.work_time) as total_time,
-                   longdescs.bug_id,
-                   bugs.short_desc,
-                   bugs.bug_status
-            FROM   longdescs
-            INNER JOIN bugs
-                ON bugs.bug_id = longdescs.bug_id 
-            WHERE  longdescs.bug_id IN ($buglist)
-                   $date_bits } .
-         $dbh->sql_group_by('longdescs.bug_id',
-                            'bugs.short_desc, bugs.bug_status,
-                             longdescs.bug_when') . qq{
-            ORDER BY longdescs.bug_when};
-    $sth = $dbh->prepare($q);
-    $sth->execute(@{$date_values});
-    while (my $row = $sth->fetch) {
-        # XXX: see comment in get_work_by_owners
-        if ($row->[0] == 0) {
-            $res{$row->[1]} = [$row->[2], $row->[3]];
-        } else {
-            delete $res{$row->[1]};
-        }
-    }
-    return \%res;
-}
-
-#
-# Misc
-#
-
-sub sort_bug_keys {
-    # XXX a hack is the mother of all evils. The fact that we store keys
-    # joined by semi-colons in the workdata-by-bug structure forces us to
-    # write this evil comparison function to ensure we can process the
-    # data timely -- just pushing it through a numerical sort makes TT
-    # hang while generating output :-(
-    my $list = shift;
-    my @a;
-    my @b;
-    return sort { @a = split(";", $a); 
-                  @b = split(";", $b); 
-                  $a[0] <=> $b[0] } @{$list};
+    my $buglist = join(", ", @$bugids);
+
+    my $bugs = $dbh->selectcol_arrayref(
+        "SELECT bug_id
+           FROM bugs
+          WHERE bugs.bug_id IN ($buglist)
+            AND NOT EXISTS (
+                SELECT 1
+                  FROM longdescs
+                 WHERE bugs.bug_id = longdescs.bug_id
+                   AND work_time > 0 $date_bits)",
+         undef, @$date_values);
+
+    return $bugs;
 }
 
 #
@@ -401,18 +239,20 @@ sub sort_bug_keys {
 Bugzilla->login(LOGIN_REQUIRED);
 
 my $cgi = Bugzilla->cgi;
+my $user = Bugzilla->user;
+my $template = Bugzilla->template;
+my $vars = {};
 
 Bugzilla->switch_to_shadow_db();
 
-Bugzilla->user->in_group(Bugzilla->params->{"timetrackinggroup"})
+$user->in_group(Bugzilla->params->{"timetrackinggroup"})
     || ThrowUserError("auth_failure", {group  => "time-tracking",
                                        action => "access",
                                        object => "timetracking_summaries"});
 
 my @ids = split(",", $cgi->param('id'));
 map { ValidateBugID($_) } @ids;
-@ids = map { detaint_natural($_) && $_ } @ids;
-@ids = grep { Bugzilla->user->can_see_bug($_) } @ids;
+scalar(@ids) || ThrowUserError('no_bugs_chosen', {action => 'view'});
 
 my $group_by = $cgi->param('group_by') || "number";
 my $monthly = $cgi->param('monthly');
@@ -423,7 +263,7 @@ my $do_depends = $cgi->param('do_depends');
 my $ctype = scalar($cgi->param("ctype"));
 
 my ($start_date, $end_date);
-if ($do_report && @ids) {
+if ($do_report) {
     my @bugs = @ids;
 
     # Dependency mode requires a single bug and grabs dependents.
@@ -432,8 +272,8 @@ if ($do_report && @ids) {
             ThrowCodeError("bad_arg", { argument=>"id",
                                         function=>"summarize_time"});
         }
-        @bugs = get_blocker_ids_unique($bugs[0]);
-        @bugs = grep { Bugzilla->user->can_see_bug($_) } @bugs;
+        @bugs = get_blocker_ids($bugs[0]);
+        @bugs = grep { $user->can_see_bug($_) } @bugs;
     }
 
     $start_date = trim $cgi->param('start_date');
@@ -445,16 +285,13 @@ if ($do_report && @ids) {
         $vars->{'warn_swap_dates'} = 1;
         ($start_date, $end_date) = ($end_date, $start_date);
     }
-    ($start_date, $end_date) = check_dates($start_date, $end_date);
-
-    if ($detailed) {
-        my %detail_data;
-        my $res = include_tt_details(\%detail_data, \@bugs, $start_date, $end_date);
-
-        $vars->{'detail_data'} = $res;
+    foreach my $date ($start_date, $end_date) {
+        next unless $date;
+        validate_date($date)
+          || ThrowUserError('illegal_date', {date => $date, format => 'YYYY-MM-DD'});
     }
-  
-    # Store dates ia session cookie the dates so re-visiting the page
+
+    # Store dates in a session cookie so re-visiting the page
     # for other bugs keeps them around.
     $cgi->send_cookie(-name => 'time-summary-dates',
                       -value => join ";", ($start_date, $end_date));
@@ -475,38 +312,35 @@ if ($do_report && @ids) {
         # start/end_date aren't provided -- and clock skews will make
         # this evident!
         @parts = split_by_month($start_date, 
-                                $end_date || time2str("%Y-%m-%d", time()));
+                                $end_date || format_time(scalar localtime(time()), '%Y-%m-%d'));
     } else {
         @parts = ([$start_date, $end_date]);
     }
 
-    my %empty_hash;
-    # For each of the separate divisions, grab the relevant summaries 
+    # For each of the separate divisions, grab the relevant data.
+    my $keyname = ($group_by eq 'owner') ? 'login_name' : 'bug_id';
     foreach my $part (@parts) {
-        my ($sub_start, $sub_end) = @{$part};
-        if (@bugs) {
-            if ($group_by eq "owner") {
-                $part_data = get_work_by_owners(\@bugs, $sub_start, $sub_end);
-            } else {
-                $part_data = get_work_by_bugs(\@bugs, $sub_start, $sub_end);
-            }
-        } else {
-            # $part_data must be a reference to a hash
-            $part_data = \%empty_hash; 
-        }
-        push @part_list, $part_data;
+        my ($sub_start, $sub_end) = @$part;
+        $part_data = get_list(\@bugs, $sub_start, $sub_end, $keyname);
+        push(@part_list, $part_data);
     }
 
-    if ($inactive && @bugs) {
+    # Do we want to see inactive bugs?
+    if ($inactive) {
         $vars->{'null'} = get_inactive_bugs(\@bugs, $start_date, $end_date);
     } else {
-        $vars->{'null'} = \%empty_hash;
+        $vars->{'null'} = {};
     }
 
+    # Convert bug IDs to bug objects.
+    @bugs = map {new Bugzilla::Bug($_)} @bugs;
+
     $vars->{'part_list'} = \@part_list;
     $vars->{'parts'} = \@parts;
-
-} elsif ($cgi->cookie("time-summary-dates")) {
+    # We pass the list of bugs as a hashref.
+    $vars->{'bugs'} = {map { $_->id => $_ } @bugs};
+}
+elsif ($cgi->cookie("time-summary-dates")) {
     ($start_date, $end_date) = split ";", $cgi->cookie('time-summary-dates');
 }
 
@@ -519,8 +353,6 @@ $vars->{'detailed'} = $detailed;
 $vars->{'inactive'} = $inactive;
 $vars->{'do_report'} = $do_report;
 $vars->{'do_depends'} = $do_depends;
-$vars->{'check_time'} = \&check_time;
-$vars->{'sort_bug_keys'} = \&sort_bug_keys;
 
 my $format = $template->get_format("bug/summarize-time", undef, $ctype);
 
diff --git a/template/en/default/bug/summarize-time.html.tmpl b/template/en/default/bug/summarize-time.html.tmpl
index da1159763..a7f90d0a7 100644
--- a/template/en/default/bug/summarize-time.html.tmpl
+++ b/template/en/default/bug/summarize-time.html.tmpl
@@ -11,18 +11,19 @@
   # The Original Code is the Bugzilla Bug Tracking System.
   #
   # Contributor(s): Christian Reis <kiko@async.com.br>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [% USE date %]
 
-[% PROCESS global/variables.none.tmpl %]
+[% PROCESS "global/field-descs.none.tmpl" %]
 
 [% title = "Time Summary " %]
 [% IF do_depends %]
     [% title = title _ "for " %]
-    [% header = title _ "$terms.Bug $ids.0" FILTER bug_link(ids.0) FILTER none %]
-    [% title = title _ "$terms.Bug $ids.0: " %]
-    [% header = (header _ " (and $terms.bugs blocking it)") IF do_depends %]
+    [% header = "$terms.Bug $ids.0" FILTER bug_link(ids.0) FILTER none %]
+    [% header = title _ header _ " (and $terms.bugs blocking it)" %]
+    [% title = title _ "$terms.Bug $ids.0" %]
 [% ELSE %]
     [% title = title _ "($ids.size $terms.bugs selected)" %]
     [% header = title %]
@@ -34,48 +35,40 @@
     style_urls = ["skins/standard/summarize-time.css"]
     %]
 
-[% IF ids.size == 0 %]
+[% INCLUDE query_form %]
 
-    <p>No [% terms.bugs %] specified or visible.</p>
+[% IF do_report %]
 
-[% ELSE %]
-
-    [% INCLUDE query_form %]
-
-    [% IF do_report %]
-
-        [% global.grand_total = 0 %]
-
-        [% FOREACH workdata = part_list %]
-            [% part = parts.shift %]
-            <div align="right">
-              <h4 style="padding-right: 2em; margin: 0;">
-            [% IF part.0 or part.1 %]
-               [% part.0 OR "Up" FILTER html %] to [% part.1 OR "now" FILTER html %]
-            [% ELSE %]
-               Full summary (no period specified)
-            [% END %]
-              </h4>
-            </div>
-            [% IF group_by == "number" %]
-                [% INCLUDE number_report %]
-            [% ELSE %]
-                [% INCLUDE owner_report %]
-            [% END %]
-        [% END %]
+  [% global.grand_total = 0 %]
 
-        [% IF monthly %]
-            <h4 style="margin: 0">Total of [% global.grand_total FILTER format("%.2f") %] hours worked</h4>
-            <hr noshade size="1">
+  [% FOREACH workdata = part_list %]
+    [%# parts contains date ranges (from, to). %]
+    [% part = parts.shift %]
+    <div align="right">
+      <h4 style="padding-right: 2em; margin: 0;">
+        [% IF part.0 or part.1 %]
+          [% part.0 OR "Up" FILTER html %] to [% part.1 OR "now" FILTER html %]
+        [% ELSE %]
+          Full summary (no period specified)
         [% END %]
+      </h4>
+    </div>
+    [% IF group_by == "number" %]
+      [% INCLUDE number_report %]
+    [% ELSE %]
+      [% INCLUDE owner_report %]
+    [% END %]
+  [% END %]
 
-        [% IF null.keys.size > 0 %] 
-            [% INCLUDE inactive_report %]
-            <h4 style="margin: 0">Total of [% null.keys.size %]
-                inactive [% terms.bugs %]</h4>
-        [% END %]
+  [% IF monthly %]
+    <h4 style="margin: 0">Total of [% global.grand_total FILTER format("%.2f") %] hours worked</h4>
+    <hr noshade size="1">
+  [% END %]
 
-    [% END %]
+  [% IF null.size > 0 %]
+    [% INCLUDE inactive_report %]
+    <h4 style="margin: 0">Total of [% null.size %] inactive [% terms.bugs %]</h4>
+  [% END %]
 
 [% END %]
 
@@ -88,7 +81,7 @@
   #%]
 
 [% BLOCK owner_report %]
-    [% global.total = 0 global.bug_count = {} global.owner_count = {}%]
+    [% global.total = 0 global.bug_count = {} global.owner_count = {} %]
     <table cellpadding="4" cellspacing="0" width="90%" class="realitems owner">
         [% FOREACH owner = workdata.keys.sort %]
             [% INCLUDE do_one_owner owner=owner ownerdata=workdata.$owner
@@ -111,19 +104,13 @@
         [% bug_id = bugdata.bug_id %]
         [% global.bug_count.$bug_id = 1 %]
         [% IF detailed %]
-            [%# XXX oy what a hack %]
-            [% timerow = '<td width="100" align="right" valign="top">' _ bugdata.total_time _ '</td>' %]
-            [% INCLUDE bug_header cid=col id=bug_id bug_status=bugdata.bug_status
-                                  short_desc=bugdata.short_desc extra=timerow %]
-             [% col = col + 1 %]
+            [% INCLUDE bug_header cid=col id=bug_id bugdata=bugdata extra=1 %]
+            [% col = col + 1 %]
         [% END %]
         [% subtotal = subtotal + bugdata.total_time %]
     [% END %]
     <tr>
-      <td colspan="3">&nbsp;</td>
-      <td align="right">
-      <b>Total</b>:
-      </td>
+      <td colspan="4" align="right"><b>Total</b>:</td>
       <td align="right" class="subtotal" width="100">
         <b>[% subtotal FILTER format("%.2f") %]</b></td>
         [% global.total = global.total + subtotal %]
@@ -140,13 +127,12 @@
     [% global.total = 0 global.owner_count = {} global.bug_count = {} %]
 
     <table cellpadding="4" cellspacing="0" width="90%" class="realitems number">
-    [% keys = sort_bug_keys(workdata.keys) %]
-    [% FOREACH bug = keys %]
-        [% INCLUDE do_one_bug bug=bug bugdata=workdata.$bug
+    [% FOREACH bug = workdata.keys.nsort %]
+        [% INCLUDE do_one_bug id=bug bugdata=workdata.$bug
                               detailed=detailed %]
     [% END %]
 
-    [% additional = "$global.bug_count.size $terms.bugs &amp; 
+    [% additional = "$global.bug_count.size $terms.bugs &
                      $global.owner_count.size developers" %]
     [% INCLUDE section_total additional=additional colspan=2 %]
     </table>
@@ -154,13 +140,8 @@
 
 [% BLOCK do_one_bug %]
     [% subtotal = 0.00 cid = 0 %]
-
-    [%# hack apart the ID and summary. Sad. %]
-    [% items = bug.split(";") %]
-    [% id = items.shift %]
-    [% status = items.shift %]
     [% global.bug_count.$id = 1 %]
-    [% INCLUDE bug_header id=id bug_status=status short_desc=items.join(";") %]
+    [% INCLUDE bug_header id=id %]
 
     [% FOREACH owner = bugdata.sort("login_name") %]
         [% work_time = owner.total_time %]
@@ -184,17 +165,21 @@
       </td>
       <td align="right" class="subtotal" width="100">
         <b>[% subtotal FILTER format("%.2f") %]</b>
-      </td></tr>
-      [% global.total = global.total + subtotal %]
+      </td>
+    </tr>
+    [% global.total = global.total + subtotal %]
 [% END %]
 
 [% BLOCK bug_header %]
     <tr class="bug_header[% '2' IF cid % 2 %]">
-        <td width="10" valign="top">
-        [% INCLUDE buglink id=id %]</td>
-        <td width="10"><b>[% bug_status FILTER html %]</b></td>
-        <td colspan="2">[% short_desc FILTER html %]</td>
-        [% extra FILTER none %]
+        <td width="80" valign="top">
+          <b>[% "$terms.Bug $id" FILTER bug_link(id) FILTER none %]</b>
+        </td>
+        <td width="100"><b>[% get_status(bugs.$id.bug_status) FILTER html %]</b></td>
+        <td colspan="2">[% bugs.$id.short_desc FILTER html %]</td>
+        [% IF extra %]
+          <td align="right" valign="top">[% bugdata.total_time FILTER html %]</td>
+        [% END %]
     </tr>
 [% END %]
 
@@ -203,9 +188,8 @@
     <h3>Inactive [% terms.bugs %]</h3>
     <table cellpadding="4" cellspacing="0" width="90%" class="zeroitems">
     [% cid = 0 %]
-    [% FOREACH bug_id = null.keys.nsort %]
-        [% INCLUDE bug_header id=bug_id bug_status=null.$bug_id.1 
-                   short_desc=null.$bug_id.0 cid=cid %]
+    [% FOREACH bug_id = null.nsort %]
+        [% INCLUDE bug_header id=bug_id cid=cid %]
         [% cid = cid + 1 %]
     [% END %]
     </table>
@@ -213,20 +197,18 @@
 
 
 [% BLOCK section_total %]
-    [% IF global.total > 0 %]
+  [% IF global.total > 0 %]
     <tr class="section_total">
-        <td align="left" width="10">
-        <b>Totals</b></td>
-    <td colspan="[% colspan FILTER none %]" align="right"><b>[% additional FILTER none %]</b></td>
-    <td align="right">&nbsp;&nbsp; 
-        <b>[% global.total FILTER format("%.2f") %]</b>
-    </td></tr>
-    [% ELSE %]
-        <tr><td>
-        No time allocated during the specified period.
-        </td></tr>
-    [% END %]
-    [% global.grand_total = global.grand_total + global.total %]
+      <td><b>Totals</b></td>
+      <td colspan="[% colspan FILTER html %]" align="right"><b>[% additional FILTER html %]</b></td>
+      <td align="right"><b>[% global.total FILTER format("%.2f") %]</b></td>
+    </tr>
+  [% ELSE %]
+    <tr>
+      <td>No time allocated during the specified period.</td>
+    </tr>
+  [% END %]
+  [% global.grand_total = global.grand_total + global.total %]
 [% END %]
 
 [%#
@@ -301,20 +283,9 @@
 </tr></table>
 
 </form>
-<script type="application/x-javascript">
+<script type="text/javascript">
 <!--
    document.forms['summary'].start_date.focus()
 //--></script>
 <hr noshade size=1>
 [% END %]
-
-[%#
-  #
-  # Utility
-  #
-  #%]
-
-[% BLOCK buglink %]
-    <a href="show_bug.cgi?id=[% id FILTER url_quote %]"><b>[% terms.Bug %]&nbsp;[% id FILTER html %]</b></a>
-[% END %]
-
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index ff9c17497..241433313 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1043,7 +1043,12 @@
 
   [% ELSIF error == "no_bugs_chosen" %]
     [% title = BLOCK %]No [% terms.Bugs %] Selected[% END %]
-    You apparently didn't choose any [% terms.bugs %] to modify.
+    You apparently didn't choose any [% terms.bugs %]
+    [% IF action == "modify" %]
+      to modify.
+    [% ELSIF action == "view" %]
+      to view.
+    [% END %]
 
   [% ELSIF error == "no_bug_ids" %]
     [% title = BLOCK %]No [% terms.Bugs %] Selected[% END %]