Bug entry fails if default priority does not exist due to localconfig edits

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Administration
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: David Pottage, Assigned: Frédéric Buclin)

Tracking

2.18
Bugzilla 2.22
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+
Build Identifier: Bugzilla 2.18-6 from debian unstable 

Bugzilla will refuse to accept any bugs if the default priority (A string) does
not exist in the @priorites list in localconfig file.

Reproducible: Always

Steps to Reproduce:
1. Edit the parameters to set defaultpriority=P2, and letsubmitterchoosepriority=off
2. Edit the localconfig file on the server to rename the P2 priority to
something else (I used P2-Medium). Run checksetup.pl
3. Attempt to enter a new bug.

Actual Results:  
Bugzilla produced an error (see attachment).

Expected Results:  
The checksetup.pl script should have adjusted the default priority to something
that existed.
(Reporter)

Comment 1

13 years ago
Created attachment 187212 [details]
The error message screen bugzilla generated.

Updated

13 years ago
Whiteboard: DUPME?
(Assignee)

Comment 2

13 years ago
This problem exists in both 2.18 and 2.19. I already discussed this point in IRC
a few months ago.

In 2.18, the problem is that priorities are stored in localconfig and are then
freely editable, including removing/changing the default priority.

In 2.19, priorites are editable using the Bugzilla UI (Field Values). Bugzilla
checks that no bugs still use some given priority before deleting/changing it;
but it does not check that this priority is not used by default, then generating
the same error message as the one you got when submitting a new bug.

I don't think we need to fix this bug for 2.18.x, but it worths fixing it for
2.20. mkanat, could you fix this problem for 2.20 only? I guess you only have to
add a check of the form "$priority ne $defaultpriority" somewhere in
editvalues.cgi. ;)
Assignee: database → installation
Status: UNCONFIRMED → NEW
Component: Database → Installation & Upgrading
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Whiteboard: DUPME?
Target Milestone: --- → Bugzilla 2.20
Version: unspecified → 2.18

Comment 3

13 years ago
Sure. It should be an easy fix.
Assignee: installation → mkanat
Component: Installation & Upgrading → Administration
(Assignee)

Updated

13 years ago
Whiteboard: [wanted for 2.20]

Updated

13 years ago
Summary: Fails if default priority does not exist due to localconfig edits → Bug entry fails if default priority does not exist due to localconfig edits
(Assignee)

Updated

13 years ago
Blocks: 297593
(Assignee)

Comment 4

13 years ago
Created attachment 195042 [details] [diff] [review]
patch, v1

I also did some cleanup in admin/fieldvalues/confirm-delete.html.tmpl.
Assignee: mkanat → LpSolit
Attachment #187212 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #195042 - Flags: review?(mkanat)

Comment 5

13 years ago
I was asked to review attachment 195042 [details] [diff] [review].  It looks OK to me, and it worked for
me.  It did not give me the option to delete the default value, and threw an
error if I tried to trick it.  However, I do have 2 nits:

> Index: template/en/default/admin/fieldvalues/confirm-delete.html.tmpl
> ===================================================================
> <<<snip>>>
> -      <p>Do you really want to delete this value?<p>
> <<<snip>>>
> +  Do you really want to delete this value?

NIT: Your replacement is not inside a paragraph or a table.  You should probably
have <p></p> tags.

> Index: template/en/default/global/user-error.html.tmpl
> ===================================================================
> <<<snip>>>
> +    '[% value FILTER html %]' is the default value for
> +    the '[% field FILTER html %]' field and then cannot
> +    be deleted.

NIT: Delete the word "then".

I am sure both of these could be fixed at checkin.

Comment 6

13 years ago
Comment on attachment 195042 [details] [diff] [review]
patch, v1

1) Need to reject changes if the priority is the default

NITS:

2) Select the correct page/param on the link we provide to
   editparams.cgi maybe? (?section=xxx)

same nits as Karl
Attachment #195042 - Flags: review-
(Assignee)

Updated

13 years ago
Attachment #195042 - Flags: review?(mkanat)

Comment 7

13 years ago
And 2 other things I thought of that are probably overkill:

1) Should sanitycheck check for this situation?

2) Should checksetup.pl check for this situation?
(Assignee)

Comment 8

13 years ago
(In reply to comment #6)
> 2) Select the correct page/param on the link we provide to
>    editparams.cgi maybe? (?section=xxx)

Good idea! But this requires bug 312356 to be fixed first. I will then submit
two distinct patches, one for the tip and one for 2.20.

re comment 7: not sure. In all cases, that's another bug.
Depends on: 312356
(Assignee)

Comment 9

13 years ago
Created attachment 199609 [details] [diff] [review]
patch for the tip, v2

Instead of rejecting changes to the default value, editvalues.cgi now updates
data/params to reflect the change.
Attachment #195042 - Attachment is obsolete: true
Attachment #199609 - Flags: review?(bugzilla)
(Assignee)

Comment 10

13 years ago
This patch is too invasive for 2.20. 2.20 is not broken; this patch would only
prevent admins from doing "mistakes", which is a new feature, not a bug fix => 2.22.
Whiteboard: [wanted for 2.20]
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22

Comment 11

13 years ago
Comment on attachment 199609 [details] [diff] [review]
patch for the tip, v2

I think we should tell (remind) the user that they have just implicitly changed
the default parameter.

Everything else is OK, so this can be changed to an r+ if Dave or Myk think
that the reminder is unnecessary
Attachment #199609 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 12

13 years ago
Created attachment 200140 [details] [diff] [review]
patch, v3

fix reviewer's comment
Attachment #199609 - Attachment is obsolete: true
Attachment #200140 - Flags: review?(bugzilla)

Updated

13 years ago
Attachment #200140 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

13 years ago
Flags: approval?
Flags: approval? → approval+
(Assignee)

Comment 13

13 years ago
Checking in editvalues.cgi;
/cvsroot/mozilla/webtools/bugzilla/editvalues.cgi,v  <--  editvalues.cgi
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/admin/fieldvalues/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/confirm-delete.html.tmpl,v
 <--  confirm-delete.html.tmpl
new revision: 1.4; previous revision: 1.3
done
Checking in template/en/default/admin/fieldvalues/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/list.html.tmpl,v
 <--  list.html.tmpl
new revision: 1.3; previous revision: 1.2
done
Checking in template/en/default/admin/fieldvalues/updated.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/fieldvalues/updated.html.tmpl,v
 <--  updated.html.tmpl
new revision: 1.3; previous revision: 1.2
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.136; previous revision: 1.135
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.