Linkify bug numbers in dependency fields on show_activity.cgi

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Creating/Changing Bugs
P4
enhancement
RESOLVED FIXED
16 years ago
11 years ago

People

(Reporter: Jonas Jørgensen, Assigned: David D. Kilzer (ddk))

Tracking

(Blocks: 1 bug)

unspecified
Bugzilla 3.0
Dependency tree / graph
Bug Flags:
approval +

Details

(URL)

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

16 years ago
When viewing the activity history of a bug, it would be nice to have the bug 
numbers which have been added to or removed from the BugsThisDependsOn or 
OtherBugsDependingOnThis fields linkified.
Priority: -- → P4
Target Milestone: --- → Bugzilla 2.20

Comment 1

16 years ago
Created attachment 91869 [details] [diff] [review]
changes to DumpBugActivity to provide show_bug links for dependencies

catches [case-insentively] BugsThisDependsOn/dependson,
OtherBugsDependingOnThis/blocked, in an attempt to be backwards-compatible with
previous BZ's.

Comment 2

16 years ago
Created attachment 91895 [details] [diff] [review]
linkify dependent bugs

tweak to the tmpl to show show_bug.cgi link for 'BugsThisDependsOn' and
'OtherBugsDependingOnThis' fields.
Attachment #91869 - Attachment is obsolete: true

Comment 3

15 years ago
*** Bug 204482 has been marked as a duplicate of this bug. ***

Comment 4

15 years ago
Created attachment 123821 [details] [diff] [review]
a perl solution to use GetBugLink on the bugids in the activity log

attachment 91895 [details] [diff] [review] is a template solution (which does not use GetBugLink())

This patch is an alternative, perl solution, using GetBugLink()

Updated

15 years ago
Attachment #123821 - Flags: review?

Comment 5

14 years ago
Comment on attachment 123821 [details] [diff] [review]
a perl solution to use GetBugLink on the bugids in the activity log

Apart from the following code nits AND the fact that it causes my runtests.pl
to fail in 008filter without an error message, this works and is fine with me.
Requesting review from Dave; would like to hear whether adding this
infrastructure is considered worth it and if cgi.pl is the right place.

>+    my $ret= $bugs;

Nit: space before =.

>+    if (defined $bugs &&
>+        $bugs ne "") {

Nit: No need for this wrap.

>+        $ret = join(", ",
>+                    @buglinks);

And neither this one.
Attachment #123821 - Flags: review? → review?(justdave)

Updated

14 years ago
Summary: [RFE] Linkify bug numbers in dependency fields on show_activity.cgi → Linkify bug numbers in dependency fields on show_activity.cgi
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22

Comment 7

13 years ago
Comment on attachment 91895 [details] [diff] [review]
linkify dependent bugs

Does not work if you change several dependencies at once.
Attachment #91895 - Attachment is obsolete: true

Comment 8

13 years ago
GavinS, is it possible to have it out of CGI.pl?

Comment 9

13 years ago
Comment on attachment 123821 [details] [diff] [review]
a perl solution to use GetBugLink on the bugids in the activity log

Looks like we should stop adding new functions to CGI.pl...
Attachment #123821 - Flags: review?(justdave) → review-

Comment 10

13 years ago
Created attachment 176365 [details] [diff] [review]
Patch v1

This patch adds a new column to fielddefs to say if a field is a bugid. This is
then used int the tempate to format the buglink.

Note: if bug 283919 is checked in, this patch will bitrot - and vice-versa.

Updated

13 years ago
Attachment #176365 - Flags: review?(LpSolit)

Comment 11

13 years ago
Comment on attachment 176365 [details] [diff] [review]
Patch v1

1) What happens when several bugs are added or removed from the dependency
list? Are they correctly parsed? I don't think so.

2) I dislike the idea of adding a new attribute for such a minor thing. As
isbugid only depends on the field name, I would prefer a new subroutine
IsBugID() which defines isbugid for you.

If you disagree with my review, ask myk or justdave to overwrite my decision.
Attachment #176365 - Flags: review?(LpSolit) → review-

Comment 12

13 years ago
(In reply to comment #11)
> (From update of attachment 176365 [details] [diff] [review] [edit])
> 1) What happens when several bugs are added or removed from the dependency
> list? Are they correctly parsed? I don't think so.

I'll give you that, I didn't test with more than one. I'd guess no, too.

> 2) I dislike the idea of adding a new attribute for such a minor thing. As
> isbugid only depends on the field name, I would prefer a new subroutine
> IsBugID() which defines isbugid for you.

I disagree here - looking at attachment 173177 [details] [diff] [review] on bug 91037 (Custom Field)
assuming that Myk's way was the way forward with custom fields, I would need to
go and hack the bugzilla function you suggest to say my new field is a bug id
and to link it. With this method, I could just specify it on the custom fields form.

Comment 13

13 years ago
OK, justdave seems to think a function is better, so I'll probably just combine
bits of both pathces that aren't obsolete so that the functions aren't in the
global .pl files.

Comment 14

13 years ago
Created attachment 176617 [details] [diff] [review]
Patch v2

Patch v2

Removed it from a Database Attribute to be a function call instead.
Attachment #176365 - Attachment is obsolete: true
Attachment #176617 - Flags: review?(LpSolit)

Comment 15

13 years ago
Comment on attachment 176617 [details] [diff] [review]
Patch v2

Theres a problem in this that I only noticed when trying it on landfill. It
fails due to UserInGroup not being a visible function.

Cancelling review, and will set depends on bug 283581
Attachment #176617 - Flags: review?(LpSolit)

Updated

13 years ago
Depends on: 283581

Updated

12 years ago
Blocks: 316826

Updated

12 years ago
Assignee: myk → create-and-change
QA Contact: mattyt-bugzilla → default-qa
Target Milestone: Bugzilla 2.22 → ---
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.24
(Assignee)

Comment 16

12 years ago
Created attachment 215494 [details] [diff] [review]
Patch v3

Previous patches had bitrotten since CGI.pl was removed, so I marked them all obsolete.

Added a new 'bug_list_link' filter to Bugzilla/Template.pm and added special cases for 'blocked' and 'dependson' in template/en/default/bug/activity/table.html.tmpl to call the new filter.
Assignee: create-and-change → ddkilzer
Attachment #123821 - Attachment is obsolete: true
Attachment #176617 - Attachment is obsolete: true
Attachment #215494 - Flags: review?
(Assignee)

Comment 17

12 years ago
Comment on attachment 215494 [details] [diff] [review]
Patch v3

Nit (copy-paste):

>   # Contributor(s): Gervase Markham <gerv@gerv.net>
>+  # Contributor(s): David D. Kilzer <ddkilzer@kilzer.net>
Attachment #215494 - Attachment filename: bug-137751-v1.diff → bug-137751-v3.diff
Attachment #215494 - Attachment is obsolete: true
Attachment #215494 - Flags: review?
(Assignee)

Comment 18

12 years ago
Created attachment 215497 [details] [diff] [review]
Patch v4

Same as Patch v3 except fixed copy-paste nit from Comment #17.
Attachment #215497 - Flags: review?
Comment on attachment 215497 [details] [diff] [review]
Patch v4

Eep, makes test suite fail:

not ok 220 - (en/default) template/en/default/bug/activity/table.html.tmpl has unfiltered directives:
#   91: change.removed FILTER bug_list_link
#   111: change.added FILTER bug_list_link
# --ERROR
#     Failed test (t/008filter.t at line 131)
Attachment #215497 - Flags: review? → review-

Comment 20

12 years ago
Created attachment 217543 [details] [diff] [review]
v5
Attachment #215497 - Attachment is obsolete: true
Attachment #217543 - Flags: review?(wicked+bz)
Comment on attachment 217543 [details] [diff] [review]
v5

>Index: template/en/default/bug/activity/table.html.tmpl
>===================================================================
>+                  [% change.removed FILTER bug_list_link FILTER none%]

Nit: Add missing space between none and % before commit.
Attachment #217543 - Flags: review?(wicked+bz) → review+
Flags: approval?
Make sure to credit both ddkilzer and timeless for the patch in the checkin comment.
Flags: approval? → approval+

Comment 23

12 years ago
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.46; previous revision: 1.45
done
Checking in template/en/default/bug/activity/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/activity/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.12; previous revision: 1.11
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Keywords: relnote

Comment 24

11 years ago
*** Bug 359671 has been marked as a duplicate of this bug. ***

Comment 25

11 years ago
Added to the release notes as part of bug 349423.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.