Refactor: Moved ApplicationController#attach_files to the Attachment model

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

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 4cc6713..c2fd2c2 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -257,27 +257,6 @@ class ApplicationController < ActionController::Base
     self.class.read_inheritable_attribute('accept_key_auth_actions') || []
   end
   
-  # TODO: move to model
-  def attach_files(obj, attachments)
-    attached = []
-    unsaved = []
-    if attachments && attachments.is_a?(Hash)
-      attachments.each_value do |attachment|
-        file = attachment['file']
-        next unless file && file.size > 0
-        a = Attachment.create(:container => obj, 
-                              :file => file,
-                              :description => attachment['description'].to_s.strip,
-                              :author => User.current)
-        a.new_record? ? (unsaved << a) : (attached << a)
-      end
-      if unsaved.any?
-        flash[:warning] = l(:warning_attachments_not_saved, unsaved.size)
-      end
-    end
-    attached
-  end
-
   # Returns the number of objects that should be displayed
   # on the paginated list
   def per_page_option

diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb
index 9d9c5a7..135ae21 100644 (file)
--- a/app/controllers/documents_controller.rb
+++ b/app/controllers/documents_controller.rb
@@ -47,7 +47,8 @@ class DocumentsController < ApplicationController
   def new
     @document = @project.documents.build(params[:document])    
     if request.post? and @document.save        
-      attach_files(@document, params[:attachments])
+      attachments = Attachment.attach_files(@document, params[:attachments])
+      flash[:warning] = attachments[:flash] if attachments[:flash]
       flash[:notice] = l(:notice_successful_create)
       redirect_to :action => 'index', :project_id => @project
     end
@@ -67,8 +68,10 @@ class DocumentsController < ApplicationController
   end
   
   def add_attachment
-    attachments = attach_files(@document, params[:attachments])
-    Mailer.deliver_attachments_added(attachments) if !attachments.empty? && Setting.notified_events.include?('document_added')
+    attachments = Attachment.attach_files(@document, params[:attachments])
+    flash[:warning] = attachments[:flash] if attachments[:flash]
+
+    Mailer.deliver_attachments_added(attachments[:files]) if attachments.present? && attachments[:files].present? && Setting.notified_events.include?('document_added')
     redirect_to :action => 'show', :id => @document
   end
 

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 7be137e..4ac1116 100644 (file)
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -158,7 +158,8 @@ class IssuesController < ApplicationController
       @issue.status = (@allowed_statuses.include? requested_status) ? requested_status : default_status
       call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue })
       if @issue.save
-        attach_files(@issue, params[:attachments])
+        attachments = Attachment.attach_files(@issue, params[:attachments])
+        flash[:warning] = attachments[:flash] if attachments[:flash]
         flash[:notice] = l(:notice_successful_create)
         call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})
         respond_to do |format|
@@ -561,8 +562,6 @@ private
 
   # 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
     if params[:time_entry] && params[:time_entry][:hours].present? && User.current.allowed_to?(:log_time, @project)
       @time_entry = TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => Date.today)
@@ -571,8 +570,9 @@ private
     end
 
     if @issue.valid?
-      attachments = attach_files(@issue, params[:attachments])
-      attachments.each {|a| @journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
+      attachments = Attachment.attach_files(@issue, params[:attachments])
+      flash[:warning] = attachments[:flash] if attachments[:flash]
+      attachments[:files].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
         if !@journal.new_record?

diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index bab0e1e..70226a7 100644 (file)
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -62,7 +62,8 @@ class MessagesController < ApplicationController
     end
     if request.post? && @message.save
       call_hook(:controller_messages_new_after_save, { :params => params, :message => @message})
-      attach_files(@message, params[:attachments])
+      attachments = Attachment.attach_files(@message, params[:attachments])
+      flash[:warning] = attachments[:flash] if attachments[:flash]
       redirect_to :action => 'show', :id => @message
     end
   end
@@ -75,7 +76,8 @@ class MessagesController < ApplicationController
     @topic.children << @reply
     if !@reply.new_record?
       call_hook(:controller_messages_reply_after_save, { :params => params, :message => @reply})
-      attach_files(@reply, params[:attachments])
+      attachments = Attachment.attach_files(@reply, params[:attachments])
+      flash[:warning] = attachments[:flash] if attachments[:flash]
     end
     redirect_to :action => 'show', :id => @topic, :r => @reply
   end
@@ -88,7 +90,8 @@ class MessagesController < ApplicationController
       @message.sticky = params[:message]['sticky']
     end
     if request.post? && @message.update_attributes(params[:message])
-      attach_files(@message, params[:attachments])
+      attachments = Attachment.attach_files(@message, params[:attachments])
+      flash[:warning] = attachments[:flash] if attachments[:flash]
       flash[:notice] = l(:notice_successful_update)
       @message.reload
       redirect_to :action => 'show', :board_id => @message.board, :id => @message.root, :r => (@message.parent_id && @message.id)

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index ef137bd..58790c2 100644 (file)
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -303,9 +303,11 @@ class ProjectsController < ApplicationController
   def add_file
     if request.post?
       container = (params[:version_id].blank? ? @project : @project.versions.find_by_id(params[:version_id]))
-      attachments = attach_files(container, params[:attachments])
+      attachments = Attachment.attach_files(container, params[:attachments])
+      flash[:warning] = attachments[:flash] if attachments[:flash]
+
       if !attachments.empty? && Setting.notified_events.include?('file_added')
-        Mailer.deliver_attachments_added(attachments)
+        Mailer.deliver_attachments_added(attachments[:files])
       end
       redirect_to :controller => 'projects', :action => 'list_files', :id => @project
       return

diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb
index fb47263..e0e159a 100644 (file)
--- a/app/controllers/wiki_controller.rb
+++ b/app/controllers/wiki_controller.rb
@@ -76,7 +76,8 @@ class WikiController < ApplicationController
       @content.version = @page.content.version
     else
       if !@page.new_record? && @content.text == params[:content][:text]
-        attach_files(@page, params[:attachments])
+        attachments = Attachment.attach_files(@page, params[:attachments])
+        flash[:warning] = attachments[:flash] if attachments[:flash]
         # don't save if text wasn't changed
         redirect_to :action => 'index', :id => @project, :page => @page.title
         return
@@ -87,7 +88,8 @@ class WikiController < ApplicationController
       @content.author = User.current
       # if page is new @page.save will also save content, but not if page isn't a new record
       if (@page.new_record? ? @page.save : @content.save)
-        attach_files(@page, params[:attachments])
+        attachments = Attachment.attach_files(@page, params[:attachments])
+        flash[:warning] = attachments[:flash] if attachments[:flash]
         call_hook(:controller_wiki_edit_after_save, { :params => params, :page => @page})
         redirect_to :action => 'index', :id => @project, :page => @page.title
       end
@@ -211,7 +213,8 @@ class WikiController < ApplicationController
 
   def add_attachment
     return render_403 unless editable?
-    attach_files(@page, params[:attachments])
+    attachments = Attachment.attach_files(@page, params[:attachments])
+    flash[:warning] = attachments[:flash] if attachments[:flash]
     redirect_to :action => 'index', :page => @page.title
   end
 

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index ace291e..221618c 100644 (file)
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -133,6 +133,33 @@ class Attachment < ActiveRecord::Base
   def readable?
     File.readable?(diskfile)
   end
+
+  # Bulk attaches a set of files to an object
+  #
+  # Returns a Hash of the results:
+  # :files => array of the attached files
+  # :unsaved => array of the files that could not be attached
+  # :flash => warning message
+  def self.attach_files(obj, attachments)
+    attached = []
+    unsaved = []
+    flash = nil
+    if attachments && attachments.is_a?(Hash)
+      attachments.each_value do |attachment|
+        file = attachment['file']
+        next unless file && file.size > 0
+        a = Attachment.create(:container => obj, 
+                              :file => file,
+                              :description => attachment['description'].to_s.strip,
+                              :author => User.current)
+        a.new_record? ? (unsaved << a) : (attached << a)
+      end
+      if unsaved.any?
+        flash = l(:warning_attachments_not_saved, unsaved.size)
+      end
+    end
+    {:files => attached, :flash => flash, :unsaved => unsaved}
+  end
   
 private
   def sanitize_filename(value)