Bug 271276 : Cannot enter negative time for 'Hours Worked'

Patch by Shane H. W. Travis <travis@sedsystems.ca>   r=LpSolit  a=justdave

3 files changed, 48 insertions, 24 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index e231f93b0..4168cac19 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -501,10 +501,29 @@ sub EmitDependList {
 }
 
 sub ValidateTime {
-  my ($time, $field) = @_;
-  if ($time > 99999.99 || $time < 0 || !($time =~ /^(?:\d+(?:\.\d*)?|\.\d+)$/)) {
-    ThrowUserError("need_positive_number", {field => "$field"}, "abort");
-  }
+    my ($time, $field) = @_;
+
+    # regexp verifies one or more digits, optionally followed by a period and
+    # zero or more digits, OR we have a period followed by one or more digits
+    # (allow negatives, though, so people can back out errors in time reporting)
+    if ($time !~ /^-?(?:\d+(?:\.\d*)?|\.\d+)$/) {
+        ThrowUserError("number_not_numeric",
+                       {field => "$field", num => "$time"},
+                       "abort");
+    }
+
+    # Only the "work_time" field is allowed to contain a negative value.
+    if ( ($time < 0) && ($field ne "work_time") ) {
+        ThrowUserError("number_too_small",
+                       {field => "$field", num => "$time", min_num => "0"},
+                       "abort");
+    }
+
+    if ($time > 99999.99) {
+        ThrowUserError("number_too_large",
+                       {field => "$field", num => "$time", max_num => "99999.99"},
+                       "abort");
+    }
 }
 
 sub GetComments {
diff --git a/globals.pl b/globals.pl
index 0b7d23bc2..1bd53261f 100644
--- a/globals.pl
+++ b/globals.pl
@@ -109,23 +109,19 @@ $::SIG{PIPE} = 'IGNORE';
 sub AppendComment {
     my ($bugid, $who, $comment, $isprivate, $timestamp, $work_time) = @_;
     $work_time ||= 0;
-    
+
+    if ($work_time) {
+        require Bugzilla::Bug;
+        Bugzilla::Bug::ValidateTime($work_time, "work_time");
+    }
+
     # Use the date/time we were given if possible (allowing calling code
     # to synchronize the comment's timestamp with those of other records).
     $timestamp = ($timestamp ? SqlQuote($timestamp) : "NOW()");
-    
+
     $comment =~ s/\r\n/\n/g;     # Get rid of windows-style line endings.
     $comment =~ s/\r/\n/g;       # Get rid of mac-style line endings.
 
-    # allowing negatives though so people can back out errors in time reporting
-    if (defined $work_time) {
-       # regexp verifies one or more digits, optionally followed by a period and
-       # zero or more digits, OR we have a period followed by one or more digits
-       if ($work_time !~ /^-?(?:\d+(?:\.\d*)?|\.\d+)$/) { 
-          ThrowUserError("need_numeric_value", {}, "abort");
-       }
-    } else { $work_time = 0 };
-
     if ($comment =~ /^\s*$/) {  # Nothin' but whitespace
         return;
     }
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index e849e7827..41661886c 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -715,15 +715,6 @@
     You must specify a component to help determine the new owner of these 
     [% terms.bugs %].
 
-  [% ELSIF error == "need_numeric_value" %]
-    [% title = "Numeric Value Required" %]
-    Hours requires a numeric value.
-
-  [% ELSIF error == "need_positive_number" %]
-    [% title = "Positive Number Required" %]
-    The <em>[% field_descs.$field FILTER html %]</em> field requires a positive
-    number.
-
   [% ELSIF error == "need_product" %]
     [% title = "Product Required" %]
     You must specify a product to help determine the new owner of these [% terms.bugs %].
@@ -800,6 +791,24 @@
     Either no products have been defined to enter [% terms.bugs %] against or you have not
     been given access to any.
 
+  [% ELSIF error == "number_not_numeric" %]
+    [% title = "Numeric Value Required" %]
+    The value '[% num FILTER html %]' in the 
+    <em>[% field_descs.$field FILTER html %]</em> field 
+    is not a numeric value.
+
+  [% ELSIF error == "number_too_large" %]
+    [% title = "Number Too Large" %]
+    The value '[% num FILTER html %]' in the 
+    <em>[% field_descs.$field FILTER html %]</em> field 
+    is more than the maximum allowable value of '[% max_num FILTER html %]'.
+
+  [% ELSIF error == "number_too_small" %]
+    [% title = "Number Too Small" %]
+    The value '[% num FILTER html %]' 
+    in the <em>[% field_descs.$field FILTER html %]</em> field 
+    is less than the minimum allowable value of '[% min_num FILTER html %]'.
+
   [% ELSIF error == "old_password_incorrect" %]
     [% title = "Incorrect Old Password" %]
     You did not enter your old password correctly.