308709 - Misleading confirmation when entering an invalid sort key for a field value

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Teemu Mannermaa (:wicked)]

In editvalues.cgi, entering a too big sort key leads to a confirmation message
like "Updated field value sortkey to: '99999999999999999999'." This is
misleading because the sortkey was actually set to 32767 which is the maximum
value allowed for a sortkey. Either correct the confirmation value or throw an
user error for invalid sortkeys. Note that entering a negative sort key already
shows an error message.

Comment 1

[Frédéric Buclin]

I saw the same problem a few days ago when working on a patch.

Comment 2

[Sunil Joshi [:sjoshi]]

Before that,
I checked adding negative sortkey in milestone and i was able to do that.. but from 
editvalues.cgi its not allowing me.

Are we allowing negative sortkey values in bugzilla?
If yes we need to fix this here also and allow negative values of sortkey in editvalues.cgi.The fix for this is going to be something like while doing the same in editmilestone.cgi page.

Let me know your thoughts on this.

Comment 3

[Frédéric Buclin]

No negative sortkeys allowed for field values. Milestones are an exception. The only legal values are 0 - 32767.

Comment 4

[Sunil Joshi [:sjoshi]]

Attachment: Patch

Comment 5

[Frédéric Buclin]

Comment on attachment 661604 [details] [diff] [review]
Patch

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

>+    (detaint_natural($value)  && $value <= MAX_SMALLINT)

Nit: remove the extra whitespace before &&.



>=== modified file 'template/en/default/global/user-error.html.tmpl'

>-    The sortkey '[% sortkey FILTER html %]' for the 
>-    [%+ field.description FILTER html %] field is not a valid 
>-    (positive) number.
>+    The sort key <em>'[% sortkey FILTER html %]'</em> must be an integer
>+    between 0 and [% constants.MAX_SMALLINT FILTER none %].

Leave "sortkey" alone (do not split it). Also, there is no reason to remove [%+ field.description FILTER html %] from the error message. Please write:

  The sortkey '[% sortkey FILTER html %]' for the [%+ field.description FILTER html %]
  field must be an integer between 0 and [% constants.MAX_SMALLINT FILTER none %].

Comment 6

[Sunil Joshi [:sjoshi]]

Attachment: Re-Patch, incorporating review comments.

Comment 7

[Frédéric Buclin]

Comment on attachment 661618 [details] [diff] [review]
Re-Patch, incorporating review comments.

r=LpSolit

Comment 8

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Field/Choice.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8397.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/Field/Choice.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8393.