371070 - Move basic validations from process_bug.cgi into Bugzilla::Bug

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Max Kanat-Alexander]

This is just like the very first thing we did to Bug.pm-ize post_bug. Some fields have very simple validators, like the "comment" field, and we can just directly use our Bugzilla::Bug->_check functions in process_bug to simplify those checks.

Comment 1

[Max Kanat-Alexander]

For process_bug, certain fields can be validated once, even on a multiple-bug change, and they are simple validations. Those are the ones we're going to move, here.

Comment 2

[Max Kanat-Alexander]

Attachment: v1

This is really, really basic stuff. It just gets some of the validations out of process_bug and into Bugzilla::Bug. It gave me a feel for where to go next, too.

Comment 3

[Frédéric Buclin]

Comment on attachment 255845 [details] [diff] [review]
v1

>Index: process_bug.cgi

> foreach my $field ("estimated_time", "work_time", "remaining_time") {
>     if (defined $cgi->param($field)) {
>-        my $er_time = trim($cgi->param($field));
>-        if ($er_time ne $cgi->param('dontchange')) {
>-            Bugzilla::Bug::ValidateTime($er_time, $field);
>-        }
>+        $cgi->param($field, $bug->_check_time($cgi->param($field), $field));
>     }
> }

If you are changing several bugs at once, you get an error:

 The value '--do_not_change--' in the Orig. Est. field is not a numeric value.

You have to do this check only if ($er_time ne $cgi->param('dontchange')).


>+    my $prod = $bug->_check_product($cgi->param('product'));

Nit: please update $cgi->param('product') with $prod->name, as you did for the component.



>Index: Bugzilla/Bug.pm

> sub _check_bug_file_loc {
>     my ($invocant, $url) = @_;
>+    $url = '' if !defined($url);

Are you sure this won't overwrite the URL field when changing several bugs at once? Most of the time, we ignore undefined fields, but consider empty fields as their new value.


>+    # Creation-only checks
>+    if (!ref $invocant) {
>+        $comment = ' ' if $comment eq '' && !ref($invocant);

You already know that !ref($invocant) is true. That's the block you are in.

Comment 4

[Max Kanat-Alexander]

Attachment: v2

URL can't be changed in a mass-change.

I fixed everything else.

Comment 5

[Frédéric Buclin]

Comment on attachment 257373 [details] [diff] [review]
v2

Looks good. r=LpSolit

Comment 6

[Max Kanat-Alexander]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.352; previous revision: 1.351
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.173; previous revision: 1.172
done