289372 - Remove %FORM from editflagtypes.cgi

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Frédéric Buclin]

Another job for wicked. ;)

Comment 1

[Teemu Mannermaa (:wicked)]

Attachment: Remove %FORM infestation, V1

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.

Comment 2

[Frédéric Buclin]

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).

Comment 3

[Teemu Mannermaa (:wicked)]

Attachment: Remove %FORM infestation, V2

Affirmative.

Comment 4

[Frédéric Buclin]

Comment on attachment 180170 [details] [diff] [review]
Remove %FORM infestation, V2

works as expected. r=LpSolit

Comment 5

[Frédéric Buclin]

Checking in editflagtypes.cgi;
/cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v  <--  editflagtypes.cgi
new revision: 1.18; previous revision: 1.17
done