Refactor: Extracted saving logic out of #update and into a utility method

Now #issue_update can be refactored and push to the Model(s) while keeping
the public interface (#update) clean.

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

1 files changed, 34 insertions, 29 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index d30924a80..d7419bea3 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -191,44 +191,21 @@ class IssuesController < ApplicationController
     end
   end
 
-  #--
-  # Start converting to the Rails REST controllers
-  #++
   def update
     update_issue_from_params
 
-    if request.get?
-      # nop
-    else
-      @time_entry = TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => Date.today)
-      @time_entry.attributes = params[:time_entry]
-      if (@time_entry.hours.nil? || @time_entry.valid?) && @issue.valid?
-        attachments = attach_files(@issue, params[:attachments])
-        attachments.each {|a| @journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
-        call_hook(:controller_issues_edit_before_save, { :params => params, :issue => @issue, :time_entry => @time_entry, :journal => @journal})
-        if @issue.save
-          # Log spend time
-          if User.current.allowed_to?(:log_time, @project)
-            @time_entry.save
-          end
-          if !@journal.new_record?
-            # Only send notification if something was actually changed
-            flash[:notice] = l(:notice_successful_update)
-          end
-          call_hook(:controller_issues_edit_after_save, { :params => params, :issue => @issue, :time_entry => @time_entry, :journal => @journal})
-          respond_to do |format|
-            format.html { redirect_back_or_default({:action => 'show', :id => @issue}) }
-            format.xml  { head :ok }
-          end
-          return
-        end
+    if issue_update
+      respond_to do |format|
+        format.html { redirect_back_or_default({:action => 'show', :id => @issue}) }
+        format.xml  { head :ok }
       end
-      # failure
+    else
       respond_to do |format|
         format.html { render :action => 'edit' }
         format.xml  { render :xml => @issue.errors, :status => :unprocessable_entity }
       end
     end
+    
   rescue ActiveRecord::StaleObjectError
     # Optimistic locking exception
     flash.now[:error] = l(:notice_locking_conflict)
@@ -579,4 +556,32 @@ private
     end
 
   end
+
+  # TODO: Temporary utility method for #update.  Should be split off
+  # and moved to the Issue model (accepts_nested_attributes_for maybe?)
+  # TODO: move attach_files to the model so this can be moved to the
+  # model also
+  def issue_update
+    @time_entry = TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => Date.today)
+    @time_entry.attributes = params[:time_entry]
+    if (@time_entry.hours.nil? || @time_entry.valid?) && @issue.valid?
+      attachments = attach_files(@issue, params[:attachments])
+      attachments.each {|a| @journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
+      call_hook(:controller_issues_edit_before_save, { :params => params, :issue => @issue, :time_entry => @time_entry, :journal => @journal})
+      if @issue.save
+        # Log spend time
+        if User.current.allowed_to?(:log_time, @project)
+          @time_entry.save
+        end
+        if !@journal.new_record?
+          # Only send notification if something was actually changed
+          flash[:notice] = l(:notice_successful_update)
+        end
+        call_hook(:controller_issues_edit_after_save, { :params => params, :issue => @issue, :time_entry => @time_entry, :journal => @journal})
+        return true
+      end
+    end
+    # failure, returns false
+
+  end
 end