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

RESOLVED FIXED in Bugzilla 2.18

Status

()

defect
RESOLVED FIXED
16 years ago
7 years ago

People

(Reporter: goobix, Assigned: goobix)

Tracking

unspecified
Bugzilla 2.18
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 3 obsolete attachments)

Assignee

Description

16 years ago
request.cgi should use $cgi-> instead of $::FORM.
Assignee

Comment 1

16 years ago
I've got a diff in my tree. Need to check how accurate it is and prepate it for r?.
Blocks: 225818
No longer blocks: 234876
Status: NEW → ASSIGNED
Assignee

Comment 2

16 years ago
Posted patch Version 1 (obsolete) — Splinter Review
Assignee

Updated

16 years ago
Target Milestone: --- → Bugzilla 2.18
Assignee

Comment 3

16 years ago
Posted patch Version 2 (obsolete) — Splinter Review
Assignee

Updated

16 years ago
Attachment #142591 - Attachment is obsolete: true
Assignee

Updated

16 years ago
Attachment #142854 - Flags: review?(kiko)

Comment 4

16 years ago
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.
Attachment #142854 - Flags: review?(kiko) → review-
Assignee

Comment 5

16 years ago
Posted patch Version 3 (obsolete) — Splinter Review
Thanks for your comments!

If you r+ it please a? it for me.
Attachment #142854 - Attachment is obsolete: true
Assignee

Updated

16 years ago
Attachment #142912 - Flags: review?(kiko)

Comment 6

16 years ago
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*.
Attachment #142912 - Flags: review?(kiko) → review-
Assignee

Comment 7

16 years ago
Posted patch Version 4Splinter Review
Yeah, good catch!
Assignee

Updated

16 years ago
Attachment #142912 - Attachment is obsolete: true
Assignee

Updated

16 years ago
Attachment #143006 - Flags: review?(kiko)

Updated

16 years ago
Attachment #143006 - Flags: review?(kiko) → review+
Assignee

Updated

16 years ago
Flags: approval?
Flags: approval? → approval+
Assignee

Comment 8

16 years ago
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
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Summary: request.cgi should use $cgi-> instead of $::FORM → request.cgi should use $cgi-> instead of %::FORM
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.