522428 - Bugzilla::Bug->create should be case-insensitive for global select fields

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement)

Description

[Max Kanat-Alexander]

Right now, Bugzilla::Bug->create is case-sensitive for fields like priority, even though Bugzilla itself is not usually case-sensitive about those fields.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

I accomplished it by using Bugzilla::Field::Choice->check, which also allowed me to do a lot of nice cleanup.

Note that check_multi_select no longer has trick_taint in it because the return value of _check_single_select is untainted always.

Comment 2

[Frédéric Buclin]

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

> sub _check_multi_select_field {
>+        push(@checked_values, $invocant->_check_select_field($field, $value));

Arguments are passed in the wrong order, see below.

> sub _check_select_field {
>     my ($invocant, $value, $field) = @_;


Didn't test it.

Comment 3

[Max Kanat-Alexander]

Attachment: v2

Thanks for catching that! :-)

Comment 4

[Frédéric Buclin]

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

>+    if ($resolution ne '') {
>+        $resolution = $self->_check_select_field($resolution, 'resolution');
>+    }

You must do this check unconditionally. At this point, it's impossible for $resolution to be ''; this is caught earlier in _check_resolution() (this subroutine isn't called for open bugs). So please remove the if () check.


> sub _check_select_field {
>-    $value = trim($value);
>+    my $object = Bugzilla::Field::Choice->type($field)->check($value);

You no longer trim() the input value, meaning that e.g. 'major ' is no longer accepted as a valid severity. Bugzilla::Object->check() trims the input value to check for its existence, but that's not the value it passes to new(), so we still have to trim it somewhere.


r=LpSolit with both comments fixed

Comment 5

[Max Kanat-Alexander]

(In reply to comment #4)
> You must do this check unconditionally. At this point, it's impossible for
> $resolution to be ''; this is caught earlier in _check_resolution() (this
> subroutine isn't called for open bugs). So please remove the if () check.

  Okay, I'll attach a new patch that fixes that.

> You no longer trim() the input value, meaning that e.g. 'major ' is no longer
> accepted as a valid severity. Bugzilla::Object->check() trims the input value
> to check for its existence, but that's not the value it passes to new(), so we
> still have to trim it somewhere.

  Ah, instead of fixing that in this patch, I've fixed it in Bugzilla::Object, in bug 519584, and I'll wait for that to go in before I check in this patch.

Comment 6

[Max Kanat-Alexander]

Attachment: v3

Carrying forward r+.

Comment 7

[Max Kanat-Alexander]

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.291; previous revision: 1.290
done