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

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[David Pottage]

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.

Comment 1

[David Pottage]

Attachment: The error message screen bugzilla generated.

Comment 2

[Frédéric Buclin]

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. ;)

Comment 3

[Max Kanat-Alexander]

Sure. It should be an easy fix.

Comment 4

[Frédéric Buclin]

Attachment: patch, v1

I also did some cleanup in admin/fieldvalues/confirm-delete.html.tmpl.

Comment 5

[A. Karl Kornel]

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

[GavinS]

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

Comment 7

[GavinS]

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?

Comment 8

[Frédéric Buclin]

(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.

Comment 9

[Frédéric Buclin]

Attachment: patch for the tip, v2

Instead of rejecting changes to the default value, editvalues.cgi now updates
data/params to reflect the change.

Comment 10

[Frédéric Buclin]

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.

Comment 11

[GavinS]

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

Comment 12

[Frédéric Buclin]

Attachment: patch, v3

fix reviewer's comment

Comment 13

[Frédéric Buclin]

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