Closed Bug 561296 Opened 10 years ago Closed 10 years ago

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

Categories

(Bugzilla :: Administration, task)

task
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 3.6

People

(Reporter: selsky, Assigned: mkanat)

Details

Attachments

(2 files, 3 obsolete files)

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
I can reproduce, using Bugzilla 3.7.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking3.6.1+
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 3.6
Version: unspecified → 3.6
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)?
Severity: minor → normal
Attached patch v1 (HEAD) (obsolete) — Splinter Review
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.
Assignee: administration → mkanat
Status: NEW → ASSIGNED
Attachment #441054 - Flags: review?(LpSolit)
Attached patch v2 (HEAD) (obsolete) — Splinter Review
Oops, v1 was the wrong version of the patch.
Attachment #441054 - Attachment is obsolete: true
Attachment #441056 - Flags: review?(LpSolit)
Attachment #441054 - Flags: review?(LpSolit)
Attached patch v2 (3.6)Splinter Review
Very simple fix, for the 3.6 branch.
Attachment #441057 - Flags: review?(LpSolit)
Attached patch v3 (HEAD) (obsolete) — Splinter Review
Check !$value in check_inactive.
Attachment #441056 - Attachment is obsolete: true
Attachment #441068 - Flags: review?(LpSolit)
Attachment #441056 - Flags: review?(LpSolit)
Hey hey. This is a blocker. Can I get a review?
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.
Attachment #441068 - Flags: review?(LpSolit) → review+
Comment on attachment 441057 [details] [diff] [review]
v2 (3.6)

r=LpSolit
Attachment #441057 - Flags: review?(LpSolit) → review+
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.
Attachment #441068 - Flags: review+ → review-
Attached patch v4 (HEAD)Splinter Review
Okay, here's a patch with those two things fixed.
Attachment #441068 - Attachment is obsolete: true
Attachment #445113 - Flags: review?(LpSolit)
Comment on attachment 445113 [details] [diff] [review]
v4 (HEAD)

r=LpSolit
Attachment #445113 - Flags: review?(LpSolit) → review+
Flags: approval3.6+
Flags: approval+
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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Summary: Cannot update priority name when it is the default priority → Cannot update a field value's name when it is the default value
Flags: testcase?
You need to log in before you can comment on or make changes to this bug.