From 6768bec4560e3c9a31578e2879f345f4c9e77a17 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Fri, 9 Jan 2009 17:32:46 +0000
Subject: [PATCH] Fixed: no error is raised when entering invalid hours on the
 issue update form (#2465).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2251 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/time_entry.rb                   |  4 ++--
 lib/redmine/core_ext/string/conversions.rb |  4 +++-
 test/functional/issues_controller_test.rb  | 18 ++++++++++++++++++
 test/unit/time_entry_test.rb               |  3 ++-
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb
index 57a75604d..f10b179d1 100644
--- a/app/models/time_entry.rb
+++ b/app/models/time_entry.rb
@@ -32,7 +32,7 @@ class TimeEntry < ActiveRecord::Base
                 :description => :comments
   
   validates_presence_of :user_id, :activity_id, :project_id, :hours, :spent_on
-  validates_numericality_of :hours, :allow_nil => true
+  validates_numericality_of :hours, :allow_nil => true, :message => :activerecord_error_invalid
   validates_length_of :comments, :maximum => 255, :allow_nil => true
 
   def after_initialize
@@ -54,7 +54,7 @@ class TimeEntry < ActiveRecord::Base
   end
   
   def hours=(h)
-    write_attribute :hours, (h.is_a?(String) ? h.to_hours : h)
+    write_attribute :hours, (h.is_a?(String) ? (h.to_hours || h) : h)
   end
   
   # tyear, tmonth, tweek assigned where setting spent_on attributes
diff --git a/lib/redmine/core_ext/string/conversions.rb b/lib/redmine/core_ext/string/conversions.rb
index 41149f5ea..68fbcde75 100644
--- a/lib/redmine/core_ext/string/conversions.rb
+++ b/lib/redmine/core_ext/string/conversions.rb
@@ -24,7 +24,9 @@ module Redmine #:nodoc:
         def to_hours
           s = self.dup
           s.strip!
-          unless s =~ %r{^[\d\.,]+$}
+          if s =~ %r{^(\d+([.,]\d+)?)h?$}
+            s = $1
+          else
             # 2:30 => 2.5
             s.gsub!(%r{^(\d+):(\d+)$}) { $1.to_i + $2.to_i / 60.0 }
             # 2h30, 2h, 30m => 2.5, 2, 0.5
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index c8c5e09c5..107026184 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -597,6 +597,24 @@ class IssuesControllerTest < Test::Unit::TestCase
     # No email should be sent
     assert ActionMailer::Base.deliveries.empty?
   end
+  
+  def test_post_edit_with_invalid_spent_time
+    @request.session[:user_id] = 2
+    notes = 'Note added by IssuesControllerTest#test_post_edit_with_invalid_spent_time'
+    
+    assert_no_difference('Journal.count') do
+      post :edit,
+           :id => 1,
+           :notes => notes,
+           :time_entry => {"comments"=>"", "activity_id"=>"", "hours"=>"2z"}
+    end
+    assert_response :success
+    assert_template 'edit'
+    
+    assert_tag :textarea, :attributes => { :name => 'notes' },
+                          :content => notes
+    assert_tag :input, :attributes => { :name => 'time_entry[hours]', :value => "2z" }
+  end
 
   def test_bulk_edit
     @request.session[:user_id] = 2
diff --git a/test/unit/time_entry_test.rb b/test/unit/time_entry_test.rb
index f86e42eab..dd54fd1b2 100644
--- a/test/unit/time_entry_test.rb
+++ b/test/unit/time_entry_test.rb
@@ -24,6 +24,7 @@ class TimeEntryTest < Test::Unit::TestCase
     assertions = { "2"      => 2.0,
                    "21.1"   => 21.1,
                    "2,1"    => 2.1,
+                   "1,5h"   => 1.5,
                    "7:12"   => 7.2,
                    "10h"    => 10.0,
                    "10 h"   => 10.0,
@@ -40,7 +41,7 @@ class TimeEntryTest < Test::Unit::TestCase
     
     assertions.each do |k, v|
       t = TimeEntry.new(:hours => k)
-      assert_equal v, t.hours
+      assert_equal v, t.hours, "Converting #{k} failed:"
     end
   end
 end