Closed Bug 289372 Opened 15 years ago Closed 15 years ago

Remove %FORM from editflagtypes.cgi

Categories

(Bugzilla :: Bugzilla-General, defect)

2.19.2
defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 2.20

People

(Reporter: LpSolit, Assigned: wicked)

References

Details

Attachments

(1 file, 1 obsolete file)

Another job for wicked. ;)
Attached patch Remove %FORM infestation, V1 (obsolete) — Splinter Review
Changes FORM hash to CGI object in editflagtypes.cgi. Also validateID() and
validateSortKey() subs are changed to return validated values. I did try to
test editing flag types.
Attachment #180107 - Flags: review?(LpSolit)
Comment on attachment 180107 [details] [diff] [review]
Remove %FORM infestation, V1

>@@ -109,7 +110,7 @@
> 
> 
> sub edit {
>-    $action eq 'enter' ? validateTargetType() : validateID();
>+    $action eq 'enter' ? validateTargetType() : (my $id = validateID());

Nit: The ": (my $foo = ...)" is not my favourite notation. But if this works,
let's go with that.


>@@ -146,12 +147,12 @@
>     else { 
>-        $vars->{'type'} = { 'target_type' => $::FORM{'target_type'} , 
>+        $vars->{'type'} = { 'target_type' => $cgi->param('target_type'), 
>                             'inclusions'  => ["__Any__:__Any__"] };
>     }

Add a scalar() for target_type.


>@@ -215,7 +216,7 @@
>-    validateSortKey();
>+    my $sortkey = validateSortKey();

validateSortKey() should update $cgi->param('sortkey') instead.


> sub validateName {
>-    $::FORM{'name'}
>-      && $::FORM{'name'} !~ /[ ,]/
>-      && length($::FORM{'name'}) <= 50
>-      || ThrowUserError("flag_type_name_invalid", { name => $::FORM{'name'} });
>+    $cgi->param('name')
>+      && $cgi->param('name') !~ /[ ,]/
>+      && length($cgi->param('name')) <= 50
>+      || ThrowUserError("flag_type_name_invalid", { name => $cgi->param('name') });
> }

Add a scalar() for name as the user may not give a flagtype name.


> sub validateSortKey {
...
>+    return $sortkey;
> }

Do not return a value but refresh $cgi->param('sortkey') instead.


> sub validateGroups {
>-      my $name = $::FORM{$col};
>-      $::FORM{$col} ||= "NULL";
>+      my $name = $cgi->param($col);
>+      $cgi->param($col) || $cgi->param($col, "NULL");

I prefer $cgi->param($col, "NULL") unless $name;


>+      my $id = FetchOneColumn();
>+      if (!$id) {
>         ThrowUserError("group_unknown", { name => $name });
>       }
>+      $cgi->param($col, $id);

Please use $gid instead of $id in validateGroups() to distinguish more easily
between flagtype ID ($id) and group ID ($gid).
Attachment #180107 - Flags: review?(LpSolit) → review-
Affirmative.
Attachment #180107 - Attachment is obsolete: true
Attachment #180170 - Flags: review?(LpSolit)
Comment on attachment 180170 [details] [diff] [review]
Remove %FORM infestation, V2

works as expected. r=LpSolit
Attachment #180170 - Flags: review?(LpSolit) → review+
Status: NEW → ASSIGNED
Flags: approval?
Target Milestone: --- → Bugzilla 2.20
Flags: approval? → approval+
Checking in editflagtypes.cgi;
/cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v  <--  editflagtypes.cgi
new revision: 1.18; previous revision: 1.17
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.