Last Comment Bug 561296 - Cannot update a field value's name when it is the default value
: Cannot update a field value's name when it is the default value
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Administration (show other bugs)
: 3.6
: All All
: -- normal (vote)
: Bugzilla 3.6
Assigned To: Max Kanat-Alexander
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-22 23:26 PDT by Matt Selsky [:selsky]
Modified: 2011-09-23 17:22 PDT (History)
2 users (show)
LpSolit: approval+
LpSolit: approval3.6+
LpSolit: blocking3.6.1+
LpSolit: testcase?
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (HEAD) (4.08 KB, patch)
2010-04-23 09:34 PDT, Max Kanat-Alexander
no flags Details | Diff | Splinter Review
v2 (HEAD) (4.33 KB, patch)
2010-04-23 09:35 PDT, Max Kanat-Alexander
no flags Details | Diff | Splinter Review
v2 (3.6) (799 bytes, patch)
2010-04-23 09:37 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review
v3 (HEAD) (4.35 KB, patch)
2010-04-23 10:24 PDT, Max Kanat-Alexander
LpSolit: review-
Details | Diff | Splinter Review
v4 (HEAD) (4.52 KB, patch)
2010-05-13 07:58 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Splinter Review

Description Matt Selsky [:selsky] 2010-04-22 23:26:10 PDT
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 2010-04-23 05:39:15 PDT
I can reproduce, using Bugzilla 3.7.
Comment 2 Frédéric Buclin 2010-04-23 06:51:49 PDT
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 2010-04-23 09:34:59 PDT
Created attachment 441054 [details] [diff] [review]
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 2010-04-23 09:35:38 PDT
Created attachment 441056 [details] [diff] [review]
v2 (HEAD)

Oops, v1 was the wrong version of the patch.
Comment 5 Max Kanat-Alexander 2010-04-23 09:37:41 PDT
Created attachment 441057 [details] [diff] [review]
v2 (3.6)

Very simple fix, for the 3.6 branch.
Comment 6 Max Kanat-Alexander 2010-04-23 10:24:42 PDT
Created attachment 441068 [details] [diff] [review]
v3 (HEAD)

Check !$value in check_inactive.
Comment 7 Max Kanat-Alexander 2010-05-01 17:43:15 PDT
Hey hey. This is a blocker. Can I get a review?
Comment 8 Frédéric Buclin 2010-05-13 06:24:49 PDT
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 2010-05-13 06:30:11 PDT
Comment on attachment 441057 [details] [diff] [review]
v2 (3.6)

r=LpSolit
Comment 10 Frédéric Buclin 2010-05-13 06:32:56 PDT
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 2010-05-13 07:58:33 PDT
Created attachment 445113 [details] [diff] [review]
v4 (HEAD)

Okay, here's a patch with those two things fixed.
Comment 12 Frédéric Buclin 2010-05-13 08:58:22 PDT
Comment on attachment 445113 [details] [diff] [review]
v4 (HEAD)

r=LpSolit
Comment 13 Max Kanat-Alexander 2010-05-14 07:27:32 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.