138904 - Should post_bug templates be separate?

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P2)

Description

[Matthew Tuck [:CodeMachine]]

I think enter_bug/post_bug comment templates are really cool and all, but I'm
slightly worried about the implementation and think we should consider making
some small changes before 2.16.

My issue is that there is only one post_bug template, comment.txt.tmpl, whereas
there can be multiple enter_bug formats.  Now at first glance this might not
matter, as you can use product determination in comment.txt.tmpl to distinguish
comment templates.

However, I can imagine people wanting to redistribute there changed templates,
and this essentially prohibits this action.  Because they are all in one file,
it is possible to slot a new enter_bug template into default but not a new
post_bug template, and therefore editing will always be required.  I would much
prefer if you could slot in templates.

So basically each enter_bug template would pass a format name through to
post_bug.cgi and post_bug.cgi would call the appropriate
comment-format.txt.tmpl.  The normal template would be comment-simple.txt.tmpl
or comment-unprocessed.txt.tmpl.  This strikes me as easy for 2.16, and I think
its worth doing before our first release of this feature.

Comment 1

[Matthew Tuck [:CodeMachine]]

Sorry, I meant slot it into "custom" not "default".

Comment 2

[Gervase Markham [:gerv]]

Yes; this makes good sense. I have patches ready to attach to bug 138284 which
pass through "format=simple" from choose-product to enter-bug, and from there it
could be submitted and looked at by post-bug, which would call the appropriate
template if it exists.

I'll knock up a patch in the next few days.

Gerv

Comment 3

[Gervase Markham [:gerv]]

I can't fix this until we've converted the CGIs to use the new template names.
But it would work something like this:

my $template_name = "bug/create/comment";
$template_name .= ($::FORM{'format'} ? "-$::FORM{'format'}" : "";

$template->process("$template_name.txt.tmpl", $vars, \$comment)
  || ThrowTemplateError($template->error());

And the standard template would have to be renamed back from
initial-comment.txt.tmpl to comment.txt.tmpl, to avoid the names getting too
long. But that would be fine.

Gerv

Comment 4

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This patch does the job. It would also mean cvs-removing
initial-comment.txt.tmpl and checking it back in as comment.txt.tmpl. This step
should be noted by any reviewer.

Gerv

Comment 5

[Gervase Markham [:gerv]]

Note: we can't use ValidateOutputFormat because it assumes that the format of
the default template is HTML.

Gerv

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 80825 [details] [diff] [review]
Patch v.1

>Index: post_bug.cgi

>+$template_name .= ($::FORM{'format'} ? "-$::FORM{'format'}" : "";

There's a syntax error here (lack of closing parenthesis).

Generally speaking, it might be better to add a "contenttype"
parameter to ValidateOutputFormat than bypass that function, 
although the primary benefit may only be a better error message.

Comment 7

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Syntactically correct patch.

> Generally speaking, it might be better to add a "contenttype"
> parameter to ValidateOutputFormat than bypass that function, 
> although the primary benefit may only be a better error message.

Yes, quite possibly - although (having had a quick look) it seems that doing
that is not completely trivial, so I hope it's OK if we leave it to another
bug.

Gerv

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 81198 [details] [diff] [review]
Patch v.2

>Yes, quite possibly - although (having had a quick look) it seems that doing
>that is not completely trivial, so I hope it's OK if we leave it to another
>bug.

Yes, but please file that bug before resolving this one.

Also, see bug 140460, which this patch exposes.

2xr=myk

Comment 9

[Gervase Markham [:gerv]]

Fixed. Further discussion in bug 140460.

Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.50; previous revision: 1.49
done
Checking in comment.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/comment.txt.tmpl,v
 <--  comment.txt.tmpl
initial revision: 1.1
done
Removing initial-comment.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/initial-comment.txt.tmpl,v
 <--  initial-comment.txt.tmpl
new revision: delete; previous revision: 1.1
done

Gerv