371030 - View the standard bug list as a dependency tree

Status: NEW

(Bugzilla :: Dependency Views, enhancement, P3)

Description

[Kevin Benton]

We need the ability to display a list of bugs from within a meta bug per installation.  So, if it were implemented, users could see the entire dependency list like a list of action items with statuses and owners...

[bugid] [assigned-to] [pri] [sev] [status] [product] [component] [short_desc]

Implementing this in AJAX means that users will be able to see buglist.cgi output from within the bug.  The challenge will be passing a parameter to buglist.cgi telling it not to display the header or footer.  This should be fairly easy to achieve.

Comment 1

[Kevin Benton]

I am unable to work this bug at this time due to other priorities.  Reassigning to default assignee.

Comment 2

[Steve Roscio]

I've hacked 3.0.3 (mostly just buglist.cgi) to display a bug hierarchy in the list.  It's got little [+] / [-] icons to expand or collapse the sub-bugs in the bug list.  Essentially, I've conceptually joined the dependency tree and the bug list.  

Is this close to what you want?  See attachment for a screenshot.

We use it in our team to help us keep track of which bug/task goes with which requirement, and the status, assignee, etc.  The dependency tree or graph just didn't show enuf info for our needs.

Comment 3

[Steve Roscio]

Attachment: Draft 1 - Nested buglist of dependencies

Comment 4

[Kevin Benton]

Nice - how do you decide when to place a dashed <hr> ?

If you're in Fort Collins, maybe we should go have lunch together sometime. :-)

Comment 5

[Steve Roscio]

Hi Kevin -

I'm down the road in Colorado Springs.  But I do go up to FTC from time to time.

When I generate the rows of bugs, I put some more classes into the TR:

   bz_depth_nnn   -- Indicates the "depth", nnn is 0, 1, 2, ... 
   This class lets me know indentation level, font relative size (100%, 
   95%, 90%, etc) via CSS.  Also what to expand/collapse when someone
   clicks the [+]/[-] icons.

   bz_treeup_nnn  -- Indicates that this item is "up" a level in the tree
   from the prior level.  This is how I know to add the top line via CSS
   'border-top' and if it's solid, dashed, 2px, 1px, etc.  I only do it for
   the first few levels deep, otherwise it gets too annoying.

I also hacked the odd/even background coloring to only change on the outer two levels.  Too busy looking otherwise, imho.

By the way, it's not working perfect.  On IE6 sometimes the top border lines are drawn partially over the line above; and also IE6 forced me to do some other CSS compromises.  FF seems to do much better.  So suggestions welcome on how to improve my hacks, if you want to look at the changes I did.

- Steve

Comment 6

[Steve Roscio]

Attachment: Screengrab of link to dependency list from show_bug

Comment 7

[Steve Roscio]

Attachment: How to get dependency list, from adv search page

Comment 8

[miketosh]

(In reply to comment #5)
> By the way, it's not working perfect.  On IE6 sometimes the top border lines
> are drawn partially over the line above; and also IE6 forced me to do some
> other CSS compromises.  FF seems to do much better.  So suggestions welcome on
> how to improve my hacks, if you want to look at the changes I did.

Have you checked those hacks on IE7?  I like this fix, and would suggest it gets picked up for 3.4 or 4.0.  But make sure that the 'include dependencies' checkbox gets saved as a user preference as well.

Comment 9

[Max Kanat-Alexander]

I dunnow if this is necessarily needed, since we have the (simple) dependency tree, and you can view that as a bug list...do you have some use cases?

CC'ing Guy to get an UI review or thoughts or something.

Comment 10

[Guy Pyrzak]

I agree with you Max that I can't really think of any use cases. Instead this is more along the lines of "hey look what i did". I think in most cases you are at a bug and want to see what depends/blocks it such as the dependency tree or the graph.

The only big use case I can think of is the searching via linking use case. To be specific this use case is where a user is searching for some unknown thing (think like in wikipedia). They type in a search term and find a bunch of bugs but not exactly what they are looking for. In the slightly unlikely event that a user doesn't dive into the bug but stays on the buglist.cgi page and sees the + sign an indication that there are associated bugs, they might expand them and look at the associated bugs. 

Another use case is seeing how far a along the depends on bugs are before an actual bug gets along. So for example if you use bugzilla to track 5 high level enhancements and you want to see how far the sub bugs are. So you do a search for the high level bugs and then you are able to expand the enhancements you are interested in etc.

As far as UI thoughts, I think we should make sure it is clear which bugs under the + sign are the blocks on and which are the depends on. We should also make sure that this works without JS, or maybe this is available via non-js methods so it's ok not to have js support it.

In general right now there isn't a way to show dependencies and blockers on the buglist.cgi page and that might be helpful to users to get context and understanding of a particular bug, so I do think it would be helpful.

Comment 11

[Steve Roscio]

Howdy -

Re: Use Case -- I did this change for use with my work projects.  We needed some simple way to see the status/owners/estimates of tasks & bugs related to user stories and requirements, without having to click back and forth on each task/bug.  

So essentially we're using it as a lightweight project management tool.  In particular, when we do our scrum meetings we get the dependency list for the parent project's task, then we go thru each task/bug assigned to that sprint.  

We also use it as a report for management to show them our progress; they want to know how each task relates to requirements and user stories and the progress within each.   I would have preferred to tie XPlanner and Bugzilla together, but this was the quicker route and it filled the need for us.

Re: IE7; I just put up IE7 on a winders box and tried it, and it looks and acts the same as FF2, so I think it's OK.  But I don't use winders much tho I'll do so for a while to make sure there's no surprises.  Of course, testing my own code doesn't count as an official test!

Re: User Preferences saving - it does not do that now.  Once my "real work" stops getting in the way :-) I'll attempt that change too.

Re: Non-JS, I'll need your help on how to do that without JavaScript, if this is a change y'all want.  The only way I can think of doing it without JavaScript is to do a round trip to the server whenever the +/- is clicked.  I don't know if that would be acceptable to a user.

Comment 12

[Guy Pyrzak]

Re: Non-JS we could use CSS, there some very cool tricks you can do with mouseovers etc. Maybe something like this? 

http://www.howtocreate.co.uk/tutorials/testMenu.html

Comment 13

[Marc Schumann [:Wurblzap]]

I think it'd be a good thing if bug lists and dependency lists were essentially the same thing, technically. We'd merge two distinct UIs, streamlining Bugzilla by removing clutter. And dependency lists would all of a sudden be configurable in what columns they show, which I miss dearly in our current dependency lists.

Comment 14

[miketosh]

(In reply to comment #13)
I very much agree with Marc.  Doing a search and including any dependent bugs is FAR more useful than plugging in bug numbers in to specifically find dependencies.

Take for example the Build-lead/Release-manager.  They go to advanced search, click on the upcoming target milestone, check off 'Also include dependencies' and WHAM!!!  They now have a full list of all bugs, their current states, a customizable column list, and the ability to change multiple, send emails to owners, or export as CSV.  All without much change.

As for JS, there are several parts of bugzilla that require JS (advanced search select boxes, editing the bug summary and dependency fields) so I don't think it NEEDS to be done in CSS.  But that is just fluff to me.  I could care less if you made it collapse-able via JS or CSS or not at all.  Any way is fine.  If you DO go with JS, make sure that the rows are all displayed by default then hide them via JS if that is what you want so that those with JS off won't be affected.

Comment 15

[Max Kanat-Alexander]

Please note that miketosh's comments are just comments, not directions from the Bugzilla Project.

Okay, I'd be fine with this if we eliminate the current dependency tree in favor of this.

I think it'd be confusing to see both blockers and dependencies as a tree under the same bug--we should just have dependencies under the bug, and maybe an option to switch to blockers or something like that.

Comment 16

[miketosh]

(In reply to comment #15)
> Please note that miketosh's comments are just comments, not directions from the
> Bugzilla Project.

Yes, just comments, and personal opinions as a bug-user.  :-)

Comment 17

[Steve Roscio]

Thanx, Guy, for the CSS pointer.  I'll look into doing it with :hover too.

The way I do it right now is so that all rows (dependent bugs) are open by default, and I use JS to collapse / re-expand rows, including ctrl+click to open just the next level.  The JS just toggles a class on the rows, the CSS does the rest.

I made a patch file and will attach it to this BZ, so you can see what I did.  It needs work since, for one, this is a patch against 3.0.3, not the current devo version.  Also I didn't settle on calling this a "dependency list" until doing the templates, so I'm not consistent in the code on what I used for variable names, etc (you'll see 'hierarchical tree list' and other such jibberish).  Too, I only did HTML templates; I don't really understand the other template types.  And I also cleaned up whitespace, capitalization, and other nits in buglist.cgi which I probably shouldn't have done since it makes the diff much bigger.

I have three small PNG images (plus, minus, dot) - how do those get put into the patch file?

This is my first patch, be gentle please!

Comment 18

[Steve Roscio]

Attachment: Draft patch to 3.0.3 - do not use for real

Comment 19

[Kevin Benton]

Marc, I concur.  Having two UI's adds complexity.  I think users are generally savvy enough to understand expandable subtrees.  If they're not, it's really easy to educate them on how they work.

Comment 20

[Kevin Benton]

Comment on attachment 314356 [details] [diff] [review]
Draft patch to 3.0.3 - do not use for real

Steve - first, thanks for posting your patch.  Contributions are always welcome / appreciated.

When you would like others to look at your patch for the purpose of review, you should set the review flag on the attachment to ?.  If you want someone in particular to review your code, to the right of the ?, there is a place to put the name of the person you'd like to do the review.  That will send email to the appropriate individuals so they know that you're looking for a review.  In order for code to be accepted into production, it requires at least two reviews by authorized reviewers and one approval from an authorized approver.  A reviewer should be the person to set the approve flag for you.  Having said that...

? patch.diff
? images/treeclose.png
? images/treedot.png
? images/treeopen.png
? js/expanding-table.js

You'll want to edit your patches down so that only what you want published is noted.

--- buglist.cgi	20 Sep 2007 21:54:10 -0000	1.351.2.7
+++ buglist.cgi	8 Apr 2008 16:21:44 -0000
@@ -1,6 +1,7 @@
 #!/usr/bin/perl -wT
 # -*- Mode: perl; indent-tabs-mode: nil -*-
 #
+#

Adding the extra blank line is generally not welcome because some say the headers in files like these is already too long.  You're right, however, that you do need to add your name to the list of contributors.

@@ -364,6 +374,70 @@
     return $groups;
 }
 
+################################################################################
+# Recursive Tree Generation Function
+################################################################################
+
+#
+# Given a list of bug records (as returned from the d/b: a list of lists),
+#  recurse thru each and insert dependent bug records using the same
+#  query as that which fetched the parents.
...

I think I would rather see this implemented in Search.pm rather than implementing this from buglist.cgi.  Why, you ask?  Creating a method like this at the .cgi level will mean that it must be duplicated to be reproduced.  OTOH, if it's implemented in the .pm, it makes the code reusable from other programs (.cgi's).  It also should help by putting all the handling of SQL where it belongs in the modules rather than being called directly from the CGI (yes, we're moving in that direction and we're not there yet, but we want to avoid code that directly interacts with the database at the CGI level).

+sub BuildTree {
+    my ($parent_list,   # ref to Parent (outer) bug list
+        $selectfields,  # ref to selected fields list
+        $orderfields,   # ref to list of field order strings
+        $sawbug,        # ref to hash of bugs we've already seen
+        $depth) = @_;

Below, you note that $maxdepth is global.  I suggest keeping it as a parameter.  This further encapsulates the code, reducing coupling.  Actually, if you don't want to make it a parameter to this function, if you're in Search.pm, you can make $maxdepth a property of Search.pm so you can get away with setting it once (using a mutator method).

+    my @newlist = ();
+    foreach my $rowref (@$parent_list) {

One other note - while lots of comments can be helpful, I personally prefer to avoid comments.  If my code documents itself well, the need for comments decreases dramatically.  That makes it easier to maintain my code because I don't have to maintain two sets of information when I only want to change one thing (the code, not code and comments).  So - rather than using my $zid = ... # ID of parent bug, I would write my $parent_bug_id = ...  That makes it much more difficult to misinterpret the value of $zid later on because the variable name tells the reader what the variable should contain.  You may want to have a look at Damian Conway's Perl Best Practices when you have a chance.

I could go on, but I have another project I have to get back to.  I'm sure others will be glad to help as well.  If you need a review, please feel free to ask me.  This is a feature we'd like to implement here at AMD. :-)

Again - thanks for posting the patch!  It looks like you're on the right track. :-)

Comment 21

[Frédéric Buclin]

(In reply to comment #20)
> In
> order for code to be accepted into production, it requires at least two reviews
> by authorized reviewers and one approval from an authorized approver.

Huh? That's wrong. One review + approval is enough. We never require 2 r+.


> One other note - while lots of comments can be helpful, I personally prefer to
> avoid comments.  If my code documents itself well, the need for comments
> decreases dramatically.

This doesn't mean comments should be avoided. In some cases, you have no other choices than to explain what the code is doing and why. That's why we have POD in Bugzilla modules.

Comment 22

[Kevin Benton]

(In reply to comment #21)
> (In reply to comment #20)
> > In
> > order for code to be accepted into production, it requires at least two reviews
> > by authorized reviewers and one approval from an authorized approver.
> 
> Huh? That's wrong. One review + approval is enough. We never require 2 r+.

My bad - it seems that we used to require two r+ and one a+, but that was two-three years ago.  Glad to learn that only one r+ is needed.  Sorry about that, Steve.

> > One other note - while lots of comments can be helpful, I personally prefer to
> > avoid comments.  If my code documents itself well, the need for comments
> > decreases dramatically.
> 
> This doesn't mean comments should be avoided. In some cases, you have no other
> choices than to explain what the code is doing and why. That's why we have POD
> in Bugzilla modules.

On comments - I'm learning that maintaining comments along with code just means more maintenance, especially when a little refactoring can eliminate the need for certain comments.  Are comments always bad?  No.  I just prefer to avoid comments when I can make my code tell the story for me. To me - POD is for functional documentation - hence the reason I differ.

Comment 23

[Max Kanat-Alexander]

  The development process for Bugzilla is described here, and new contributors should read it:

  http://wiki.mozilla.org/Bugzilla:Developers


  Comments should say why, not how or what. Ideally they should be short.

Comment 24

[Steve Roscio]

Thanx all, great feedback.  I'll work on it some more and toss it back here to see what ya think.  I'll assign this BZ to myself for the duration.

Comment 25

[bigstijn]

Somehow related to bug 12992

Comment 26

[Steve Roscio]

Hey Guys - sorry for going off the radar for so long; lots of upheavals at work.  And it's less than 90 days 'til Halloween season ;-)
I'm still here and still plan to work this BZ.
- Steve

Comment 27

[Arcel]

(In reply to comment #10)
> In general right now there isn't a way to show dependencies and blockers on the
> buglist.cgi page and that might be helpful to users to get context and
> understanding of a particular bug, so I do think it would be helpful.

This part of the comment does exist as bug 102048 .

Comment 28

[bigstijn]

bug 371030, bug 12992, bug 120687 seems to be dupes of each other.

Comment 30

[Frédéric Buclin]

I agree with wurblzap that dependency trees and buglists should be merged, and displayed as a buglist (I really love the screenshot in draft 1). This would not only let users decide which information to display, but also let users with editbugs privs edit several bugs at once. And for managers, I agree that this would be a cool way to see what needs to be done (e.g. you do a query displaying all bugs having blocking+, and the buglist would also display bugs blocking them).

I don't think it's useful to display bugs blocked by the bugs in the buglist, at least not in this bug. Displaying bugs blocking the bugs in the buglist, as suggested in this bug, is much more useful. I'm all for it!

UI and backend implementations should be discussed with pyrzak and mkanat, though.

Steve, still alive? :)

Comment 31

[Max Kanat-Alexander]

  Hmm. Well, here are my thoughts on it:

  * I agree that it would be useful for managers to be able to see dependencies in the bug list. Also, it would be good to have customizable columns for the dependency view.

  * I'm concerned about the performance aspect of running the entire dependency tree for every bug in the list. (It would lead to performance on buglist.cgi like the performance of duplicates.cgi, where we have to run down chains, but probably even worse because the chains are more complicated.) Perhaps if the generation of the full tree was done by AJAX, this problem could be avoided, although that would be a lot of AJAX calls for long chains, unless we made a custom WebServices method to determine this information.

  * What I like about the dependency tree view currently is that it's focused on a single task, and so it can do that task really well and lay things out nicely with sensible defaults.

  * This would probably be pretty difficult with a YUI DataTable, which is what the buglist is going to become.

Comment 32

[Albert Ting]

I know we're moving away from YUI, but somebody did create a hierarchical datatable that could work for this feature, noted at http://yuiblog.com/blog/2010/04/14/treeble-using-nested-yui-2-datasources-for-row-expansion.

Comment 33

[Albert Ting]

Attachment: sample

For what it's worth, I've attached a patch that can generate the dependency content based on an advanced search query, which means you can include any number of columns. I purposely avoided indenting/modifying existing code to make the patch simpler to maintain on my end.

With the patch, one can use Treeble to display the results in a YUI-style datatable.  I have a modified table.html.tmpl yet it is highly customized.

Comment 34

[Kevin Benton]

Comment on attachment 8656271 [details] [diff] [review]
sample

I'm out of the loop in Bugzilla development today, but am adding r? to this attachment to help it get some review traction. :-)

Comment 35

[April Price]

Attachment: same problem

Thanks from holmtapocro1989.