Bug 406271

LOC estimation:

in 3.2 there is still the template
template/en/default/bug/create/comment.txt.tmpl as how it is in 2_18 we just
have to add IF ELSE to include the comment customization for the EIT product ,,
which is about 25 LOC 


Unit Test:

Would be a selenium test to verify that for creating new bugs under the EIT
product the customized comment text is the one that will appear in the
description text box.

Basically every action in the web ui is selenium line so the test for this case
would be about 25 LOC

TOTAL LOC = 25 + 25 = 50 LOC

Please Note changed selenium test estimation from LOC to time estimation

Created attachment 299257 [details]
v1 for customizing bug entry comment

This patch include new hook 'defaultcontent' added to template
template/en/default/bug/create/create.html.tmpl that connects to the template
hook under new extension :
extensions/enter_bug_comment/template/en/bug/create/create-defaultcontent.html.tmpl


and it will add the defaultcomment that is usually in all bug reports as we
have it in 2.18 and for the EIT product it will add its customized comment to
the bug entry report.

Please review when you can and let me know what you think.

Thanks,
Noura

spent alot of time trying to figure out how the template hook works

I have the patch applied to bugdev.devel.redhat.com if you would like to test it
there 

Comment on attachment 299257 [details]
v1 for customizing bug entry comment

>+[% USE Bugzilla %]
>+[% Bugzilla.cgi.param("comment") %]
>+[% IF Bugzilla.cgi.param("comment") %]
>+  [% Bugzilla.cgi.param("comment") %]

Why are you printing Bugzilla.cgi.param('comment') twice?

[% IF Bugzilla.cgi.param("comment") %]
  [% Bugzilla.cgi.param("comment") %]

should be enough.

Also there is already a $vars->{comment} being passed in from enter_bug.cgi
line 456 so you could just do:

[% IF comment %]
  [% comment FILTER none %]

>+[% ELSE %]
>+    [% IF Bugzilla.cgi.param("product") == "EIT" %]

Also here you can just use product.name. No need to import Bugzilla.cgi.

[% ELSE %]
    [% IF product.name == "EIT" %]


>           # by global/textarea.html.tmpl. So we must not escape the comment here. %]
>         [% comment FILTER none %]
>       [%- END %]
>+  
>+      [% defaultcontent = Hook.process("defaultcontent") %]
>       [% INCLUDE global/textarea.html.tmpl
>          name           = 'comment'
>          id             = 'comment'

On line 457, the template is checking to see if the bug is being cloned by
checking for cloned_bug_id. If true defaultcontent is assigned text for the
cloned bug. You are overriding defaultcontent with
Hook.process'defaultcontent'). So you may need to move the cloned_bug_id
defaultcontent block into the extension so if bugs are being cloned they can
still get the cloned comment prepended to the text. Or if we leave the
cloned_bug_id text in create.html.tmpl you can do

% defaultcontent = defaultcontent _ Hook.process("defaultcontent") %]

Created attachment 299273 [details]
v2 for customizing bug entry comment

Thanks for the review Dave,, Another patch with your suggestions.

Noura

Created attachment 299281 [details]
v3 for customizing bug entry comment

Please ignore previous patch and review this one. 

Noura

Comment on attachment 299281 [details]
v3 for customizing bug entry comment

Actually after testing this, it doesn't work right as I originally suggested.
Notes below

>+[% IF comment %]
>+  [% comment FILTER none %]

'comment' is already included in the defaultcontent BLOCK in create.html.tmpl
so it will print
comment twice if we do this.

Change this to 

[% IF defaultcontent %]
  [% defaultcontent FILTER none %]

The rest of create-defaultcontent.html.tmpl looks fine.

>+[% ELSE %]
>+  [% IF product.name == "EIT" %]
>+Environment:
>+------------
>+BIOS Version:
>+BIOS Type (EFI or Legacy):
[snip]

>           # by global/textarea.html.tmpl. So we must not escape the comment here. %]
>         [% comment FILTER none %]
>       [%- END %]
>+  
>+      [% defaultcontent = defaultcontent _ Hook.process("defaultcontent") %]

Since 'comment' var is already included in the defaultcontent BLOCK above we
can just print defaultcontent value in the extension template as stated above.
So you can change the above to only be:

	[% defaultcontent = Hook.process("defaultcontent") %]

Also make sure to add the [%# REDHAT EXTENSION START <bugid> %] tags around the
Hook.process() part.

After these changes then r=dkl and go ahead and checkin.

Dave

Created attachment 299421 [details]
v4 for customizing bug entry comment

Thanks for the review Dave ,, I have made the fixes and committed the attached
patch to cvs.

Noura