Bug 457676 - Dependency fields (Depends On, Blocks) don't show symbolic aliases
Summary: Dependency fields (Depends On, Blocks) don't show symbolic aliases
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Bugzilla
Classification: Community
Component: Bugzilla General
Version: 3.2
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Noura El hawary
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-08-03 12:00 UTC by Jason Tibbitts
Modified: 2013-06-24 04:05 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-08-08 05:31:03 UTC


Attachments (Terms of Use)
v1 adding alias to dependencies in show_bug.cgi (3.67 KB, patch)
2008-08-08 02:25 UTC, Noura El hawary
dkl: review-
Details | Diff
v2 add alias to dependency bug ids (3.24 KB, patch)
2008-08-08 05:09 UTC, Noura El hawary
dkl: review+
Details | Diff

Description Jason Tibbitts 2008-08-03 12:00:30 UTC
It seems that since the switch to 3.2, the ticket display pages don't show symbolic names for tickets in the dependency fields.  For example, bug 177841 used to show as FE-NEEDSPONSOR when a ticket blocked it, but not it only shows as 177841.  This makes the practice of using blocker bugs a bit less useful as everyone must now remember the numeric equivalents or mouse over the dependency fields and hope that the summary is recognizable.

Comment 1 David Lawrence 2008-08-04 04:08:47 UTC
Verified that this is true. I also agree that it is useful to have. I will look into getting this functionality into the next update.

Comment 2 Noura El hawary 2008-08-08 02:25:09 UTC
Created attachment 313772 [details]
v1 adding alias to dependencies in show_bug.cgi

attached is a patch to add aliases to dependson and blocked field bug ids if they exist. Sorry Dave I like that bug so i stole it from you  :).  Please review and let me know what you think, also it is applied to bz-web2 for testing.

Noura

Comment 3 David Lawrence 2008-08-08 04:13:52 UTC
Comment on attachment 313772 [details]
v1 adding alias to dependencies in show_bug.cgi

>+    my ($bug_state, $bug_res, $bug_desc, $alias) =
>+        $dbh->selectrow_array('SELECT bugs.bug_status, resolution, short_desc, alias
>                                FROM bugs WHERE bugs.bug_id = ?',
>                                undef, $bug_num);
> 
>+    if ($use_alias) {
>+        $link_text = $alias;
>+    }
>+

Need to also check to make sure $alias is not empty, otherwise the $link_text would be empty.

if ($use_alias && $alias) {
    $link_text = $alias;
}

>+            # REDHAT EXTENSION START 457676
>+            # added alias paran to subroutine

Nit: param is misspelled in the comment.

>             bug_link => [ sub {
>-                              my ($context, $bug) = @_;
>+                              my ($context, $bug, $use_alias) = @_;
>                               return sub {
>                                   my $text = shift;
>-                                  return get_bug_link($bug, $text);
>+                                  return get_bug_link($bug, $text, undef ,$use_alias);

Nit: space in front of comma before $use_alias.

>+  [% use_alias = 1 %]
>   <th class="field_label">
>     <label for="[% dep.fieldname %]"[% " accesskey=\"$accesskey\"" IF accesskey %]>
>     [% dep.title %]</label>:
>@@ -1127,10 +1128,24 @@
>                value="[% bug.${dep.fieldname}.join(', ') %]">
>       [% END %]
>     </span>
>-    
>+
>+    [%# REDHAT EXTENSION START 457676 %] 
>+    [% deps = bug.${dep.fieldname}.join(', ') %]
>+    [% IF deps %]
>+      <b>
>+        [% "(" %]
>+        [% FOREACH depbug = bug.${dep.fieldname} %]
>+          [% depbug FILTER bug_link(depbug, use_alias) FILTER none %][% " " %]
>+        [% END %]
>+        [% ")" %]
>+        </b>
>+    [% END %]
>+    [%# REDHAT EXTENSION END 457676 %]
>+
>     [% FOREACH depbug = bug.${dep.fieldname} %]
>       [% depbug FILTER bug_link(depbug) FILTER none %][% " " %]
>     [% END %]
>+    

The above seems over complicated to me. Why not just do the normal FOREACH loop and pass in 1 for use_alias. If there is an alias for the bug id then the alias will be displayed in the place of the bug id. No need to to place them within ( ) parens. Also it looks like you are looping through all of the dependencies twice which is not efficient. Also looking at the HTML source I see alot of blank hrefs inside the ( ) parens that didn't have aliases so are blank.

So I like the stuff you did in Bugzilla/Template.pm. For the template, I would just do the following passing in 1 for $use_alias:

    [% FOREACH depbug = bug.${dep.fieldname} %]
      [% depbug FILTER bug_link(depbug, 1) FILTER none %][% " " %]
    [% END %]

Dave

Comment 4 Noura El hawary 2008-08-08 05:09:25 UTC
Created attachment 313782 [details]
v2 add alias to dependency bug ids 

Thanks for the review Dave, attached is apatch with your suggestions. Noura

Comment 5 David Lawrence 2008-08-08 05:18:34 UTC
Comment on attachment 313782 [details]
v2 add alias to dependency bug ids 

Looks good, Noura.

Comment 6 Noura El hawary 2008-08-08 05:31:03 UTC
committed to cvs , thanks Dave.

Noura


Note You need to log in before you can comment on or make changes to this bug.