236019 - request.cgi should use $cgi-> instead of %::FORM

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Vlad Dascalu]

request.cgi should use $cgi-> instead of $::FORM.

Comment 1

[Vlad Dascalu]

I've got a diff in my tree. Need to check how accurate it is and prepate it for r?.

Comment 2

[Vlad Dascalu]

Attachment: Version 1

Comment 3

[Vlad Dascalu]

Attachment: Version 2

Comment 4

[Christian Reis]

Comment on attachment 142854 [details] [diff] [review]
Version 2

>Index: request.cgi
> 
>+# initialize the $cgi variable
>+my $cgi = Bugzilla->cgi;

No need for the comment :)

>+    if (defined($cgi->param('requester')) && $cgi->param('requester') ne "") {

Don't need the parenthesis to defined here.. this happens in other parts of the
patch which would be nice to see removed.

>@@ -185,14 +190,14 @@
>         }
>         if (!$has_attachment_type) { push(@excluded_columns, 'attachment') }
>         
>-        push(@criteria, "flagtypes.name = " . SqlQuote($::FORM{'type'}));
>-        push(@excluded_columns, 'type') unless $::FORM{'do_union'};
>+        push(@criteria, "flagtypes.name = " . SqlQuote($form_type));
>+        push(@excluded_columns, 'type') unless $form_type;

Errr, unless do_union :-P

>+    my $form_group = $cgi->param('group');

[...]

>     $vars->{'excluded_columns'} = \@excluded_columns;
>-    $vars->{'group_field'} = $::FORM{'group'};
>+    $vars->{'group_field'} = $cgi->param('group');

Hmm, don't you want to use $form_group here?

Looks okay, but a hard read.

Comment 5

[Vlad Dascalu]

Attachment: Version 3

Thanks for your comments!

If you r+ it please a? it for me.

Comment 6

[Christian Reis]

Comment on attachment 142912 [details] [diff] [review]
Version 3

Sorry I didn't catch this before: it's probably best to define $cgi in queue()
local's scope, and supply status and group as arguments to validate*.

Comment 7

[Vlad Dascalu]

Attachment: Version 4

Yeah, good catch!

Comment 8

[Vlad Dascalu]

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