470262 - Show alias if available in show_bug.cgi for bug dependencies, otherwise show bug id

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[David Lawrence [:dkl]]

Attachment: Patch to show alias in bug dependencies for show_bug.cgi (v1)

Attaching a patch that we use at Red Hat to substitute the alias of a bug instead of the bug id for the link text in the bug dependency lists.

Comment 1

[Max Kanat-Alexander]

Comment on attachment 353697 [details] [diff] [review]
Patch to show alias in bug dependencies for show_bug.cgi (v1)

>-    my ($bug_num, $link_text, $comment_num) = @_;
>+    my ($bug_num, $link_text, $comment_num, $use_alias) = @_;

  This this is trunk, we can do some rearch.

  Here's what I'd like to see for this function:

  * link_text should be auto-populated (this maybe should be done in a separate bug, later).
  * The last two positional parameters should become named parameters in a hashref instead.

>+    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);

  We're getting so many fields now, just create a bug object.

Comment 2

[Vitaly Fedrushkov]

Then you get them referenced the same way in comments.  How do we auto-linkify 'bug bug-alias'?

Comment 3

[Max Kanat-Alexander]

(In reply to comment #2)
> Then you get them referenced the same way in comments.

  No, the patch doesn't do that.

>  How do we auto-linkify 'bug bug-alias'?

  We don't.

Comment 4

[David Lawrence [:dkl]]

Attachment: Patch to show alias in bug dependencies for show_bug.cgi (v2)

Patch that uses named parameters instead of ordered parameters for Bugzilla::Template::get_bug_link(). 

Please review
Dave

Comment 5

[Vitaly Fedrushkov]

As we don't auto-linkify aliases (no good idea on how to do this anyway), maybe we should not hide bug number entirely: if alias is visualized, you're still one click away from its number.

What about adding ID to link title if alias is defined?  Or have per-user setting on a feature?

Comment 6

[Max Kanat-Alexander]

Comment on attachment 354272 [details] [diff] [review]
Patch to show alias in bug dependencies for show_bug.cgi (v2)

This looks pretty good, if it works. FWIW, you don't need to do "defined $hash->{key} && $hash->{key}", because undef is false.

I still have to test this, though.

Comment 7

[Max Kanat-Alexander]

(In reply to comment #5)
> What about adding ID to link title if alias is defined?  Or have per-user
> setting on a feature?

  No, I think this is fine. Aliases are far more meaningful than bug numbers, and in the rare case where you'd still need to know the number, you can just go to the bug.

Comment 8

[David Lawrence [:dkl]]

(In reply to comment #6)
> (From update of attachment 354272 [details] [diff] [review])
> This looks pretty good, if it works. FWIW, you don't need to do "defined
> $hash->{key} && $hash->{key}", because undef is false.
> 
> I still have to test this, though.

I tested this locally and worked properly. I will submit new patch without the defined part.

Dave

Comment 9

[David Lawrence [:dkl]]

Attachment: Patch to show alias in bug dependencies for show_bug.cgi (v3)

Removed if defined parts. Please take a look.

Dave

Comment 10

[Max Kanat-Alexander]

Comment on attachment 354304 [details] [diff] [review]
Patch to show alias in bug dependencies for show_bug.cgi (v3)

>Index: Bugzilla/Template.pm
>+    my $bug       = Bugzilla::Bug->new($bug_num);
>+    my $bug_state = $bug->bug_status;
>+    my $bug_res   = $bug->resolution;
>+    my $bug_desc  = $bug->short_desc;

  If there is no such bug, this will now die instead of just silently not doing anything, because $bug will be undef.

>+    if ($options->{use_alias} && $bug->alias) {
>+        $link_text = $bug->alias;

  Should you really be overriding the link_text that was input here? Or should we just be replacing anything numeric in it with the alias?

>+      [% depbug FILTER bug_link(depbug, use_alias => 1) FILTER none %][% " " %]

  That needs to be { use_alias => 1 }, I'm pretty sure.

  Also, bug_link really shouldn't need a "FILTER none" after it, but I suppose that's another bug.

Comment 11

[David Lawrence [:dkl]]

Attachment: Patch to show alias in bug dependencies for show_bug.cgi (v4)

(In reply to comment #10)
> (From update of attachment 354304 [details] [diff] [review])
> >Index: Bugzilla/Template.pm
> >+    my $bug       = Bugzilla::Bug->new($bug_num);
> >+    my $bug_state = $bug->bug_status;
> >+    my $bug_res   = $bug->resolution;
> >+    my $bug_desc  = $bug->short_desc;
> 
>   If there is no such bug, this will now die instead of just silently not doing
> anything, because $bug will be undef.
> 

I converted back to loading the data directly using a db call. We can convert to a Bug object in another bug.

> >+    if ($options->{use_alias} && $bug->alias) {
> >+        $link_text = $bug->alias;
> 
>   Should you really be overriding the link_text that was input here? Or should
> we just be replacing anything numeric in it with the alias?

I added a check to see if $link_text is an integer and if so, then replace with the alias if defined.
> 
> >+      [% depbug FILTER bug_link(depbug, use_alias => 1) FILTER none %][% " " %]
> 
>   That needs to be { use_alias => 1 }, I'm pretty sure.
> 

http://template-toolkit.org/docs/manual/Variables.html#section_Passing_Parameters_and_Returning_Values

"Named parameters may also be specified. These are automatically collected into a single hash array which is passed by reference as the last  parameter to the sub-routine. Named parameters can be specified using either => or = and can appear anywhere in the argument list."

Using { use_alias => 1 } will not work in the template itself from what I can tell in my development.

Comment 12

[Max Kanat-Alexander]

Comment on attachment 358894 [details] [diff] [review]
Patch to show alias in bug dependencies for show_bug.cgi (v4)

Looks good! :-)

Comment 13

[David Lawrence [:dkl]]

tip:
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.96; previous revision: 1.95
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.148; previous revision: 1.147
done

Comment 14

[Max Kanat-Alexander]

Added to the release notes for Bugzilla 3.4 in bug 494037.