Closed Bug 137751 Opened 18 years ago Closed 14 years ago

Linkify bug numbers in dependency fields on show_activity.cgi

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: jonasj, Assigned: ddkilzer)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 7 obsolete files)

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
catches [case-insentively] BugsThisDependsOn/dependson,
OtherBugsDependingOnThis/blocked, in an attempt to be backwards-compatible with
previous BZ's.
Attached patch linkify dependent bugs (obsolete) — Splinter Review
tweak to the tmpl to show show_bug.cgi link for 'BugsThisDependsOn' and
'OtherBugsDependingOnThis' fields.
Attachment #91869 - Attachment is obsolete: true
*** Bug 204482 has been marked as a duplicate of this bug. ***
attachment 91895 [details] [diff] [review] is a template solution (which does not use GetBugLink())

This patch is an alternative, perl solution, using GetBugLink()
Attachment #123821 - Flags: review?
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)
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 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
GavinS, is it possible to have it out of CGI.pl?
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-
Attached patch Patch v1 (obsolete) — Splinter Review
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.
Attachment #176365 - Flags: review?(LpSolit)
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-
(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.
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.
Attached patch Patch v2 (obsolete) — Splinter Review
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 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)
Depends on: 283581
Blocks: 316826
Assignee: myk → create-and-change
QA Contact: mattyt-bugzilla → default-qa
Target Milestone: Bugzilla 2.22 → ---
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.24
Attached patch Patch v3 (obsolete) — Splinter Review
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?
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?
Attached patch Patch v4 (obsolete) — Splinter Review
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-
Attached patch v5Splinter Review
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+
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
Closed: 14 years ago
Resolution: --- → FIXED
Keywords: relnote
*** Bug 359671 has been marked as a duplicate of this bug. ***
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.