From 0fd7e2d696cf2d94da8b4cbcdb8fa4b36eb395fd Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Tue, 2 Mar 2010 19:26:03 +0000 Subject: [PATCH] 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 --- app/controllers/application_controller.rb | 21 ------------------ app/controllers/documents_controller.rb | 9 +++++--- app/controllers/issues_controller.rb | 10 ++++----- app/controllers/messages_controller.rb | 9 +++++--- app/controllers/projects_controller.rb | 6 +++-- app/controllers/wiki_controller.rb | 9 +++++--- app/models/attachment.rb | 27 +++++++++++++++++++++++ 7 files changed, 54 insertions(+), 37 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4cc671396..c2fd2c2f5 100644 --- 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 9d9c5a7d0..135ae2198 100644 --- 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 7be137e8b..4ac11162c 100644 --- 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 bab0e1e19..70226a711 100644 --- 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 ef137bd8a..58790c28f 100644 --- 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 fb472636b..e0e159a00 100644 --- 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 ace291ec1..221618c6a 100644 --- 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)