Bug 330718: showdependencytree.cgi needs some cleanup - Patch by Frédéric Buclin <LpSolit@gmail.com> r=wicked a=justdave

2 files changed, 34 insertions, 86 deletions

diff --git a/showdependencytree.cgi b/showdependencytree.cgi
index 1a1e51d3a..b2cf29a57 100755
--- a/showdependencytree.cgi
+++ b/showdependencytree.cgi
@@ -22,16 +22,16 @@
 #                 Andreas Franke <afranke@mathweb.org>
 #                 Christian Reis <kiko@async.com.br>
 #                 Myk Melez <myk@mozilla.org>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 use strict;
 
 use lib qw(.);
 require "globals.pl";
 use Bugzilla;
-use Bugzilla::User;
 use Bugzilla::Bug;
 
-Bugzilla->login();
+my $user = Bugzilla->login();
 
 my $cgi = Bugzilla->cgi;
 my $template = Bugzilla->template;
@@ -48,6 +48,7 @@ my $dbh = Bugzilla->switch_to_shadow_db();
 # bug that the user is authorized to access.
 my $id = $cgi->param('id');
 ValidateBugID($id);
+my $current_bug = new Bugzilla::Bug($id, $user->id);
 
 my $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0;
 
@@ -58,19 +59,12 @@ if ($maxdepth !~ /^\d+$/) { $maxdepth = 0 };
 # Main Section                                                                 #
 ################################################################################
 
-# The column/value to select as the target milestone for bugs,
-# either the target_milestone column or the empty string value
-# (for installations that don't use target milestones).  Makes
-# it easier to query the database for bugs because we don't
-# have to embed a conditional statement into each query.
-my $milestone_column = Param('usetargetmilestone') ? "target_milestone" : "''";
-
 # Stores the greatest depth to which either tree goes.
 my $realdepth = 0;
 
 # Generate the tree of bugs that this bug depends on and a list of IDs
 # appearing in the tree.
-my $dependson_tree = { $id => GetBug($id) };
+my $dependson_tree = { $id => $current_bug };
 my $dependson_ids = {};
 GenerateTree($id, "dependson", 1, $dependson_tree, $dependson_ids);
 $vars->{'dependson_tree'} = $dependson_tree;
@@ -78,7 +72,7 @@ $vars->{'dependson_ids'} = [keys(%$dependson_ids)];
 
 # Generate the tree of bugs that this bug blocks and a list of IDs
 # appearing in the tree.
-my $blocked_tree = { $id => GetBug($id) };
+my $blocked_tree = { $id => $current_bug };
 my $blocked_ids = {};
 GenerateTree($id, "blocked", 1, $blocked_tree, $blocked_ids);
 $vars->{'blocked_tree'} = $blocked_tree;
@@ -89,7 +83,6 @@ $vars->{'realdepth'}      = $realdepth;
 $vars->{'bugid'}          = $id;
 $vars->{'maxdepth'}       = $maxdepth;
 $vars->{'hide_resolved'}  = $hide_resolved;
-$vars->{'canedit'}        = UserInGroup("editbugs");
 
 print $cgi->header();
 $template->process("bug/dependency-tree.html.tmpl", $vars)
@@ -102,15 +95,19 @@ $template->process("bug/dependency-tree.html.tmpl", $vars)
 sub GenerateTree {
     # Generates a dependency tree for a given bug.  Calls itself recursively
     # to generate sub-trees for the bug's dependencies.
-    
     my ($bug_id, $relationship, $depth, $bugs, $ids) = @_;
-    
-    # Query the database for bugs with the given dependency relationship.
-    my @dependencies = GetDependencies($bug_id, $relationship);
-    
+
+    my @dependencies;
+    if ($relationship eq 'dependson') {
+        @dependencies = @{$bugs->{$bug_id}->dependson};
+    }
+    else {
+        @dependencies = @{$bugs->{$bug_id}->blocked};
+    }
+
     # Don't do anything if this bug doesn't have any dependencies.
     return unless scalar(@dependencies);
-    
+
     # Record this depth in the global $realdepth variable if it's farther 
     # than we've gone before.
     $realdepth = max($realdepth, $depth);
@@ -120,7 +117,7 @@ sub GenerateTree {
         # its sub-tree if we haven't already done so (which happens
         # when bugs appear in dependency trees multiple times).
         if (!$bugs->{$dep_id}) {
-            $bugs->{$dep_id} = GetBug($dep_id);
+            $bugs->{$dep_id} = new Bugzilla::Bug($dep_id, $user->id);
             GenerateTree($dep_id, $relationship, $depth+1, $bugs, $ids);
         }
 
@@ -128,63 +125,14 @@ sub GenerateTree {
         # if it exists, if we haven't exceeded the maximum depth the user 
         # wants the tree to go, and if the dependency isn't resolved 
         # (if we're ignoring resolved dependencies).
-        if ($bugs->{$dep_id}->{'exists'}
+        if (!$bugs->{$dep_id}->{'error'}
             && (!$maxdepth || $depth <= $maxdepth) 
-            && ($bugs->{$dep_id}->{'open'} || !$hide_resolved))
+            && ($bugs->{$dep_id}->{'isopened'} || !$hide_resolved))
         {
-            push (@{$bugs->{$bug_id}->{'dependencies'}}, $dep_id);
+            # Due to AUTOLOAD in Bug.pm, we cannot add 'dependencies'
+            # as a bug object attribute from here.
+            push(@{$bugs->{'dependencies'}->{$bug_id}}, $dep_id);
             $ids->{$dep_id} = 1;
         }
     }
 }
-
-sub GetBug {
-    # Retrieves the necessary information about a bug, stores it in the bug cache,
-    # and returns it to the calling code.
-    my ($id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $bug = {};
-    if (Bugzilla->user->can_see_bug($id)) {
-        ($bug->{'exists'},
-         $bug->{'status'},
-         $bug->{'resolution'},
-         $bug->{'summary'},
-         $bug->{'milestone'},
-         $bug->{'assignee_id'},
-         $bug->{'assignee_email'}) = $dbh->selectrow_array(
-                "SELECT 1,
-                        bug_status, 
-                        resolution,
-                        short_desc, 
-                        $milestone_column, 
-                        assignee.userid, 
-                        assignee.login_name
-                   FROM bugs
-             INNER JOIN profiles AS assignee
-                     ON bugs.assigned_to = assignee.userid
-                  WHERE bugs.bug_id = ?", undef, $id);
-     }
-    
-    $bug->{'open'} = $bug->{'exists'} && is_open_state($bug->{'status'});
-    $bug->{'dependencies'} = [];
-    
-    return $bug;
-}
-
-sub GetDependencies {
-    # Returns a list of dependencies for a given bug.
-    my ($id, $relationship) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $bug_type = ($relationship eq "blocked") ? "dependson" : "blocked";
-    
-    my $dependencies = $dbh->selectcol_arrayref(
-              "SELECT $relationship
-                 FROM dependencies 
-                WHERE $bug_type = ?
-             ORDER BY $relationship", undef, $id);
-    
-    return @$dependencies;
-}
-
diff --git a/template/en/default/bug/dependency-tree.html.tmpl b/template/en/default/bug/dependency-tree.html.tmpl
index 8fd0c18e6..4b8c7c396 100644
--- a/template/en/default/bug/dependency-tree.html.tmpl
+++ b/template/en/default/bug/dependency-tree.html.tmpl
@@ -24,7 +24,7 @@
 
 [% PROCESS global/variables.none.tmpl %]
 
-[% filtered_desc = blocked_tree.$bugid.summary FILTER html %]
+[% filtered_desc = blocked_tree.$bugid.short_desc FILTER html %]
 [% PROCESS global/header.html.tmpl
    title = "Dependency tree for $terms.Bug $bugid"
    h1    = "Dependency tree for 
@@ -76,7 +76,7 @@
   [% IF ids.size > 0 %]
     ([% IF maxdepth -%]Up to [% maxdepth %] level[% "s" IF maxdepth > 1 %] deep | [% END -%]
     <a href="buglist.cgi?bug_id=[% ids.join(",") %]">view as [% terms.bug %] list</a>
-    [% IF canedit && ids.size > 1 %]
+    [% IF user.groups.editbugs && ids.size > 1 %]
       | <a href="buglist.cgi?bug_id=[% ids.join(",") %]&amp;tweak=1">change several</a>
     [% END %])
     <ul class="tree">
@@ -92,12 +92,12 @@
 [% BLOCK display_tree %]
   [%# INTERFACE
     #   - bugid: the ID of the bug being displayed
-    #   - tree: a hash of bug objects
+    #   - tree: a hash of bug objects and of bug dependencies
     #%]
   [% bug = tree.$bugid %]
   <li>
-    [%- INCLUDE bullet bugid=bugid bug=bug -%]
-    <span class="summ[% "_deep" IF bug.dependencies.size %]" 
+    [%- INCLUDE bullet bugid=bugid tree=tree -%]
+    <span class="summ[% "_deep" IF tree.dependencies.$bugid.size %]" 
           id="[% bugid FILTER html %]" 
           [% IF global.seen.$bugid %]
             onMouseover="duplicatedover('[% bugid FILTER html %]')"
@@ -109,9 +109,9 @@
       <b><a title="Already displayed above; click to locate"
             onclick="duplicated('[% bugid FILTER html %]')"
             href="#b[% bugid %]">(*)</a></b>
-    [% ELSIF bug.dependencies.size %]
+    [% ELSIF tree.dependencies.$bugid.size %]
       <ul>
-        [% FOREACH depid = bug.dependencies %]
+        [% FOREACH depid = tree.dependencies.$bugid %]
           [% INCLUDE display_tree bugid=depid %]
         [% END %]
       </ul>
@@ -121,7 +121,7 @@
 [% END %]
 
 [% BLOCK bullet %]
-  [% IF bug.dependencies.size > 0 && ! global.seen.$bugid %]
+  [% IF tree.dependencies.$bugid.size && ! global.seen.$bugid %]
     [% extra_class = " b_open" %]
     [% extra_args = 'onclick="return doToggle(this, event)"' %] 
   [% END %]
@@ -132,21 +132,21 @@
 [% END %]
 
 [% BLOCK buglink %]
-  [% isclosed = !bug.open %]
+  [% isclosed = !bug.isopened %]
   [% FILTER closed(isclosed) -%]
     <a title="[% INCLUDE buginfo bug=bug %]"
        href="show_bug.cgi?id=[% bugid %]">
       <b>[%- bugid %]:</b>
-      <span class="summ_text">[%+ bug.summary FILTER html %]</span>
+      <span class="summ_text">[%+ bug.short_desc FILTER html %]</span>
       <span class="summ_info">[[% INCLUDE buginfo %]]</span>
     </a>
   [% END %]
 [% END %]
 
 [% BLOCK buginfo %]
-  [% bug.status FILTER html %]
-  [%+ bug.resolution FILTER html %]; assigned to [% bug.assignee_email FILTER html %]; 
-  [% "Target: " _ bug.milestone IF bug.milestone %]
+  [% bug.bug_status FILTER html -%] [%+ bug.resolution FILTER html %];
+  [%-%] assigned to [% bug.assigned_to.login FILTER html %];
+  [%-%][% " Target: " _ bug.target_milestone IF bug.target_milestone %]
 [% END %]
 
 [%###########################################################################%]