Closed Bug 239346 Opened 20 years ago Closed 20 years ago

Need a hook after a bug's comments

Categories

(Bugzilla :: User Interface, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: nb+bz, Assigned: nb+bz)

References

()

Details

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.6) Gecko/20040325
Build Identifier: 

The P4DTI Bugzilla integration with Perforce adds a section to a bug's display,
which appears after the comments.  In pre-templated versions of Bugzilla, this
was hard-coded in to bug_form.pl.  In 2.16.x, it was done with a custom
edit.html.tmpl template.  The right thing to do now is obviously to use a hook
which appears right after a bug's comments.  I have an "aftercomments" hook in
comments.html.tmpl which does just that.

Reproducible: Always
Steps to Reproduce:
Here is the simple patch to add the hook.
The URL link is to a screen shot of a Bugzilla bug in a P4DTI installation.  The
shot was taken with Bugzilla 2.12, but the intended behaviour hasn't changed
since then.
See also bug 207514 (Incorporate P4DTI patches into Bugzilla).
This looks fine.  The only question is whether you want the Perforce UI to
appear only on show_bug.cgi or also on long_list.cgi (i.e. when the user clicks
the "Long List" button at the bottom of a bug list).

If we put the hook in comments.html.tmpl, then unless the Perforce templates do
some additional tests for context, they'll get processed every time
comments.html.tmpl is included, which a quick grep of the source code says is
edit.html.tmpl (used by show_bug.cgi), show-multiple.html.tmpl (used by
long_list.cgi), and process/midair.html.tmpl (used by process_bug.cgi when two
users' changes conflict).

If you just want the Perforce UI in show_bug.cgi, then it would be better to
place the hook into edit.html.tmpl after the "PROCESS comments.html.tmpl" call
on line 536.  What do you think?
> This looks fine.  The only question is whether you want the Perforce UI to
> appear only on show_bug.cgi or also on long_list.cgi (i.e. when the user clicks
> the "Long List" button at the bottom of a bug list).

Yes, we want it for both.  In particular, we want it for "Format For Printing"
(which is long_list.cgi) as well as for show_bug.cgi.  We will need conditional
code to display the P4DTI section differently in these two cases, and to avoid
displaying it at all in the midair.html.tmpl case (when start_at > 0).  The
alternative for us would be to have two hooks, one where you suggest in
edit.html.tmpl and the other in show_multiple.html.tmpl.

Note that the current P4DTI patch does not show up in "Format For Printing" (in
2.16.x), but this is a bug in the patch.
Comment on attachment 145253 [details] [diff] [review]
patch for comments.html.tmpl

Nick: you probably want to avoid the process/midair.html.tmpl case, though...

Gerv
Attachment #145253 - Flags: review+
Flags: approval?
Assignee: myk → Nick.Barnes
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.18
a= justdave on condition that you're okay with it hitting in the mid-air template...
Flags: approval? → approval+
Dave: I'll add a comment just above the hook noting that anyone using it should
be aware that this template is called from multiple places, and they may need to
check in their hook template.

Hmm... ideally, the docs for hooks would tell you how to do that.

Gerv
Fixed.

Checking in template/en/default/bug/comments.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v
 <--  comments.html.tmpl
new revision: 1.10; previous revision: 1.9
done

Gerv
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Great, thanks.
FYI, here is my draft of the actual hook file for the P4DTI.  You can see how
I'm using mode=='edit' and start_at == 0 to distinguish the various use cases.
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: