Bug 457676 - Dependency fields (Depends On, Blocks) don't show symbolic aliases
Dependency fields (Depends On, Blocks) don't show symbolic aliases
Status: CLOSED NEXTRELEASE
Product: Bugzilla
Classification: Community
Component: Bugzilla General (Show other bugs)
3.2
All Linux
medium Severity medium (vote)
: ---
: ---
Assigned To: Noura El hawary
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-03 08:00 EDT by Jason Tibbitts
Modified: 2013-06-24 00:05 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-08-08 01:31:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description Jason Tibbitts 2008-08-03 08:00:30 EDT
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 00:08:47 EDT
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-07 22:25:09 EDT
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 00:13:52 EDT
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 01:09:25 EDT
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 01:18:34 EDT
Comment on attachment 313782 [details]
v2 add alias to dependency bug ids 

Looks good, Noura.
Comment 6 Noura El hawary 2008-08-08 01:31:03 EDT
committed to cvs , thanks Dave.

Noura

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