315605 - Bugzilla::Field::check_form_field() should not take a CGI object as parameter

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Frédéric Buclin]

When using non-cgi scripts, no CGI object is (or should be) available, and so we cannot use Bugzilla::Field::check_form_field*() to validate fields. These methods should accept a hash of the form {field_name => $field_value} instead of $cgi.

This problem appears at least in importxml.pl.

Comment 1

[Max Kanat-Alexander]

Instead of a hash, why not just check_field_defined($value, $fieldName)?

Comment 2

[Max Kanat-Alexander]

Oh also, we should remove the word "form" from the function name, since it won't only be working on forms anymore.

Comment 3

[Frédéric Buclin]

Attachment: patch, v1

I finally wrote ($name, $value, ...) instead of a hash, as suggested by mkanat.

Comment 4

[Teemu Mannermaa (:wicked)]

Comment on attachment 202321 [details] [diff] [review]
patch, v1

>Index: Bugzilla/Field.pm
>===================================================================
>-#                 Fr&#65533;&#65533;ic Buclin <[e-mail]>
>+#                 Frédéric Buclin <[e-mail]>

This was a nasty change and made my patch attempt to fail for some reason. It took few hours but I finally got it to apply. :)

Otherwise this seems to change code logic nicely.

Comment 5

[Dave Miller [:justdave]]

This is too invasive to a critical procedure for 2.22 this close to release.  Lets's check it in after we branch.

Comment 6

[Frédéric Buclin]

The patch has completely bitrotten due to custom fields. I will update it later this week.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 202321 [details] [diff] [review]
patch, v1

FWIW, we were going to just move the check_form_field sub to CGI.pm... that would have worked, also.

Comment 8

[Max Kanat-Alexander]

(In reply to comment #7)
> (From update of attachment 202321 [details] [diff] [review] [edit])
> FWIW, we were going to just move the check_form_field sub to CGI.pm... that
> would have worked, also.

  Oh, I see why you didn't do that. Okay :-)

Comment 9

[Frédéric Buclin]

Attachment: unbitrotten patch, v1.1

The patch I'm checking in.

Comment 10

[Frédéric Buclin]

Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.136; previous revision: 1.135
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.305; previous revision: 1.304
done
Checking in Bugzilla/Field.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field.pm,v  <--  Field.pm
new revision: 1.11; previous revision: 1.10
done