From af4793c884c6f3eb9dff8a79865f6cfa690aa9ec Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Wed, 19 Sep 2007 02:37:11 +0000 Subject: Bug 389313: summarize_time.cgi needs some cleanup - Patch by Frédéric Buclin r/a=mkanat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- summarize_time.cgi | 320 +++++++++++++---------------------------------------- 1 file changed, 76 insertions(+), 244 deletions(-) (limited to 'summarize_time.cgi') 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 # Shane H. W. Travis -# +# Frédéric Buclin + 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); -- cgit v1.2.3-24-g4f1b