Fixes Issue#save_issue_with_child_records so that time entry do not get saved if...

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3664 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/issue.rb b/app/models/issue.rb
index e2c85e1db40863412abca08166ed05672a99ed10..263cae1329a5a746d5132eaeb12c73821deedf02 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -484,34 +484,35 @@ class Issue < ActiveRecord::Base
   # Saves an issue, time_entry, attachments, and a journal from the parameters
   # Returns false if save fails
   def save_issue_with_child_records(params, existing_time_entry=nil)
-    if params[:time_entry] && params[:time_entry][:hours].present? && User.current.allowed_to?(:log_time, project)
-      @time_entry = existing_time_entry || TimeEntry.new
-      @time_entry.project = project
-      @time_entry.issue = self
-      @time_entry.user = User.current
-      @time_entry.spent_on = Date.today
-      @time_entry.attributes = params[:time_entry]
-      self.time_entries << @time_entry
-    end
-
-    if valid?
-      attachments = Attachment.attach_files(self, params[:attachments])
-
-      attachments[:files].each {|a| @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
-      # TODO: Rename hook
-      Redmine::Hook.call_hook(:controller_issues_edit_before_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
-      begin
-        if save
-          # TODO: Rename hook
-          Redmine::Hook.call_hook(:controller_issues_edit_after_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
-          return true
-        else
-          return false
+    Issue.transaction do
+      if params[:time_entry] && params[:time_entry][:hours].present? && User.current.allowed_to?(:log_time, project)
+        @time_entry = existing_time_entry || TimeEntry.new
+        @time_entry.project = project
+        @time_entry.issue = self
+        @time_entry.user = User.current
+        @time_entry.spent_on = Date.today
+        @time_entry.attributes = params[:time_entry]
+        self.time_entries << @time_entry
+      end
+  
+      if valid?
+        attachments = Attachment.attach_files(self, params[:attachments])
+  
+        attachments[:files].each {|a| @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
+        # TODO: Rename hook
+        Redmine::Hook.call_hook(:controller_issues_edit_before_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
+        begin
+          if save
+            # TODO: Rename hook
+            Redmine::Hook.call_hook(:controller_issues_edit_after_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
+          else
+            raise ActiveRecord::Rollback
+          end
+        rescue ActiveRecord::StaleObjectError
+          attachments[:files].each(&:destroy)
+          errors.add_to_base l(:notice_locking_conflict)
+          raise ActiveRecord::Rollback
         end
-      rescue ActiveRecord::StaleObjectError
-        attachments[:files].each(&:destroy)
-        errors.add_to_base l(:notice_locking_conflict)
-        return false
       end
     end
   end

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 37561ec699e76ff279072a7fb89214023d605b2f..9c7d4f1c9b2b56b68911901691ba10fed99a387c 100644 (file)
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -827,7 +827,7 @@ class IssuesControllerTest < ActionController::TestCase
       put :update,
            :id => 1,
            :notes => '2.5 hours added',
-           :time_entry => { :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first }
+           :time_entry => { :hours => '2.5', :comments => 'test_put_update_with_note_and_spent_time', :activity_id => TimeEntryActivity.first }
     end
     assert_redirected_to :action => 'show', :id => '1'
     
@@ -837,7 +837,7 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal '2.5 hours added', j.notes
     assert_equal 0, j.details.size
     
-    t = issue.time_entries.find(:first, :order => 'id DESC')
+    t = issue.time_entries.find_by_comments('test_put_update_with_note_and_spent_time')
     assert_not_nil t
     assert_equal 2.5, t.hours
     assert_equal spent_hours_before + 2.5, issue.spent_hours
@@ -985,15 +985,18 @@ class IssuesControllerTest < ActionController::TestCase
     @request.session[:user_id] = 2
 
     assert_no_difference 'Journal.count' do
-      assert_no_difference 'Attachment.count' do
-        put :update,
-              :id => issue.id,
-              :issue => {
-                :fixed_version_id => 4,
-                :lock_version => (issue.lock_version - 1)
-              },
-              :notes => '',
-              :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}
+      assert_no_difference 'TimeEntry.count' do
+        assert_no_difference 'Attachment.count' do
+          put :update,
+                :id => issue.id,
+                :issue => {
+                  :fixed_version_id => 4,
+                  :lock_version => (issue.lock_version - 1)
+                },
+                :notes => '',
+                :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}},
+                :time_entry => { :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first }
+        end
       end
     end