Bug 457676

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.

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 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

Created attachment 313782 [details]
v2 add alias to dependency bug ids 

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

Comment on attachment 313782 [details]
v2 add alias to dependency bug ids 

Looks good, Noura.

committed to cvs , thanks Dave.

Noura