561296 - Cannot update a field value's name when it is the default value

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Matt Selsky [:selsky]]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_3; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.17 Safari/533.4
Build Identifier: Bugzilla 3.6+

Now that priorities P1 through P5 have been replaced with Highest through Lowest, I am trying to update my installation via editvalues.cgi  I was able to change P1 and P3 to P5 successfully.  However, I can't change P2 since it is the default value for new bugs.  Error message is:

Software error:

Param defaultpriority is not valid: Must be a legal priority value: one of Highest, Normal, Low, Lowest at Bugzilla/Config.pm line 108.

I had to change the "defaultpriority" parameter on the bug fields parameter page to allow this change to go through.

It would be nice if the defaultpriority parameter changed automatically when I renamed the priority.

I am running the latest CVS version as of a few minutes ago.

Reproducible: Always

Comment 1

[Frédéric Buclin]

I can reproduce, using Bugzilla 3.7.

Comment 2

[Frédéric Buclin]

The whole error is (when renaming P4, my default priority, to P44):

editvalues.cgi: Param defaultpriority is not valid: Must be a legal priority value: one of P1, P2, P3, P5 at Bugzilla/Config.pm line 108.
 at /usr/lib/perl5/vendor_perl/5.10.1/CGI/Carp.pm line 354
	CGI::Carp::realdie('[Fri Apr 23 14:38:08 2010] editvalues.cgi: Param defaultprior...') called at /usr/lib/perl5/vendor_perl/5.10.1/CGI/Carp.pm line 455
	CGI::Carp::die('Param defaultpriority is not valid: Must be a legal priority ...') called at Bugzilla/Config.pm line 108
	Bugzilla::Config::SetParam('defaultpriority', 'P44') called at Bugzilla/Field/Choice.pm line 173
	Bugzilla::Field::Choice::update('Bugzilla::Field::Choice::priority=HASH(0xa2c9c38)') called at /var/www/html/bugzilla/editvalues.cgi line 190


SetParam() calls check_priority() which calls get_legal_field_values() to get the list of valid priorities. Could it be that get_legal_field_values() cannot access the priority being edited because the transaction is not done yet (the list only mentions unaltered priorites)?

Comment 3

[Max Kanat-Alexander]

Attachment: v1 (HEAD)

Actually, the problem turned out to be that set_name was getting called before set_inactive, and because of the way that is_default is implemented, is_active was getting set to 0. (Because we changed the name, so is_default no longer returned true.)

Here is a very involved fix, for HEAD, that uses defined_is_active instead of checking is_default || is_static, and improves the checker for isactive in Bugzilla::Field::Choice.

I'll also attach a much simpler and smaller fix, for the branch.

Comment 4

[Max Kanat-Alexander]

Attachment: v2 (HEAD)

Oops, v1 was the wrong version of the patch.

Comment 5

[Max Kanat-Alexander]

Attachment: v2 (3.6)

Very simple fix, for the 3.6 branch.

Comment 6

[Max Kanat-Alexander]

Attachment: v3 (HEAD)

Check !$value in check_inactive.

Comment 7

[Max Kanat-Alexander]

Hey hey. This is a blocker. Can I get a review?

Comment 8

[Frédéric Buclin]

Comment on attachment 441068 [details] [diff] [review]
v3 (HEAD)

>=== modified file 'template/en/default/admin/fieldvalues/edit.html.tmpl'

>+           [% IF !(value.is_default OR value.is_static) %]
>+             <input id="defined_is_active" name="defined_is_active" value="1">
>+           [% END %]

You must add type="hidden", else you get a text field with "1" in it. r=LpSolit with this fix.

Comment 9

[Frédéric Buclin]

Comment on attachment 441057 [details] [diff] [review]
v2 (3.6)

r=LpSolit

Comment 10

[Frédéric Buclin]

Comment on attachment 441068 [details] [diff] [review]
v3 (HEAD)

>=== modified file 'Bugzilla/Field/Choice.pm'

>+sub _check_isactive {
>+            ThrowUserError('fieldvalue_is_default', 
>+                           { value => $invocant, field => $invocant->field });

Oh, I just realized that "param_name" must also be passed to the error message. Please fix that and my previous comment.

Comment 11

[Max Kanat-Alexander]

Attachment: v4 (HEAD)

Okay, here's a patch with those two things fixed.

Comment 12

[Frédéric Buclin]

Comment on attachment 445113 [details] [diff] [review]
v4 (HEAD)

r=LpSolit

Comment 13

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified editvalues.cgi
modified Bugzilla/CGI.pm
modified Bugzilla/Field/Choice.pm
modified template/en/default/admin/fieldvalues/edit.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7170.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified editvalues.cgi
Committed revision 7097.