diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb index c84ce5f51b..fe5eb48c8b 100644 --- a/app/controllers/files_controller.rb +++ b/app/controllers/files_controller.rb @@ -19,4 +19,19 @@ class FilesController < ApplicationController render :layout => !request.xhr? end + # TODO: split method into new (GET) and create (POST) + def new + if request.post? + container = (params[:version_id].blank? ? @project : @project.versions.find_by_id(params[:version_id])) + attachments = Attachment.attach_files(container, params[:attachments]) + render_attachment_warning_if_needed(container) + + if !attachments.empty? && Setting.notified_events.include?('file_added') + Mailer.deliver_attachments_added(attachments[:files]) + end + redirect_to :controller => 'files', :action => 'index', :id => @project + return + end + @versions = @project.versions.sort + end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 05c8d969d6..61c7a7c71c 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -18,7 +18,6 @@ class ProjectsController < ApplicationController menu_item :overview menu_item :roadmap, :only => :roadmap - menu_item :files, :only => [:add_file] menu_item :settings, :only => :settings before_filter :find_project, :except => [ :index, :list, :add, :copy ] @@ -239,21 +238,6 @@ class ProjectsController < ApplicationController @project = nil end - def add_file - if request.post? - container = (params[:version_id].blank? ? @project : @project.versions.find_by_id(params[:version_id])) - attachments = Attachment.attach_files(container, params[:attachments]) - render_attachment_warning_if_needed(container) - - if !attachments.empty? && Setting.notified_events.include?('file_added') - Mailer.deliver_attachments_added(attachments[:files]) - end - redirect_to :controller => 'files', :action => 'index', :id => @project - return - end - @versions = @project.versions.sort - end - def save_activities if request.post? && params[:enumerations] Project.transaction do diff --git a/app/views/files/index.html.erb b/app/views/files/index.html.erb index 2b2e5e870c..66f5d12b9a 100644 --- a/app/views/files/index.html.erb +++ b/app/views/files/index.html.erb @@ -1,5 +1,5 @@
-<%= link_to_if_authorized l(:label_attachment_new), {:controller => 'projects', :action => 'add_file', :id => @project}, :class => 'icon icon-add' %> +<%= link_to_if_authorized l(:label_attachment_new), {:controller => 'files', :action => 'new', :id => @project}, :class => 'icon icon-add' %>

<%=l(:label_attachment_plural)%>

diff --git a/app/views/projects/add_file.rhtml b/app/views/files/new.html.erb similarity index 82% rename from app/views/projects/add_file.rhtml rename to app/views/files/new.html.erb index ab9c7352db..bbb3b17334 100644 --- a/app/views/projects/add_file.rhtml +++ b/app/views/files/new.html.erb @@ -2,7 +2,7 @@ <%= error_messages_for 'attachment' %>
-<% form_tag({ :action => 'add_file', :id => @project }, :multipart => true, :class => "tabular") do %> +<% form_tag({ :action => 'new', :id => @project }, :multipart => true, :class => "tabular") do %> <% if @versions.any? %>

diff --git a/config/routes.rb b/config/routes.rb index 270bab4a0b..9c8a2f6ae8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -182,7 +182,7 @@ ActionController::Routing::Routes.draw do |map| project_views.connect 'projects/:id.:format', :action => 'show' project_views.connect 'projects/:id/:action', :action => /destroy|settings/ project_views.connect 'projects/:id/files', :controller => 'files', :action => 'index' - project_views.connect 'projects/:id/files/new', :action => 'add_file' + project_views.connect 'projects/:id/files/new', :controller => 'files', :action => 'new' project_views.connect 'projects/:id/settings/:tab', :action => 'settings' project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new' end @@ -199,7 +199,7 @@ ActionController::Routing::Routes.draw do |map| project_actions.connect 'projects', :action => 'add' project_actions.connect 'projects.:format', :action => 'add', :format => /xml/ project_actions.connect 'projects/:id/:action', :action => /edit|destroy|archive|unarchive/ - project_actions.connect 'projects/:id/files/new', :action => 'add_file' + project_actions.connect 'projects/:id/files/new', :controller => 'files', :action => 'new' project_actions.connect 'projects/:id/activities/save', :action => 'save_activities' end diff --git a/lib/redmine.rb b/lib/redmine.rb index 726a2ad92f..745c6d750f 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -102,7 +102,7 @@ Redmine::AccessControl.map do |map| end map.project_module :files do |map| - map.permission :manage_files, {:projects => :add_file}, :require => :loggedin + map.permission :manage_files, {:files => :new}, :require => :loggedin map.permission :view_files, :files => :index, :versions => :download end diff --git a/test/functional/files_controller_test.rb b/test/functional/files_controller_test.rb index 9085021833..838caee70e 100644 --- a/test/functional/files_controller_test.rb +++ b/test/functional/files_controller_test.rb @@ -26,4 +26,42 @@ class FilesControllerTest < ActionController::TestCase :attributes => { :href => '/attachments/download/9/version_file.zip' } end + def test_add_file + set_tmp_attachments_directory + @request.session[:user_id] = 2 + Setting.notified_events = ['file_added'] + ActionMailer::Base.deliveries.clear + + assert_difference 'Attachment.count' do + post :new, :id => 1, :version_id => '', + :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}} + assert_response :redirect + end + assert_redirected_to 'projects/ecookbook/files' + a = Attachment.find(:first, :order => 'created_on DESC') + assert_equal 'testfile.txt', a.filename + assert_equal Project.find(1), a.container + + mail = ActionMailer::Base.deliveries.last + assert_kind_of TMail::Mail, mail + assert_equal "[eCookbook] New file", mail.subject + assert mail.body.include?('testfile.txt') + end + + def test_add_version_file + set_tmp_attachments_directory + @request.session[:user_id] = 2 + Setting.notified_events = ['file_added'] + + assert_difference 'Attachment.count' do + post :new, :id => 1, :version_id => '2', + :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}} + assert_response :redirect + end + assert_redirected_to 'projects/ecookbook/files' + a = Attachment.find(:first, :order => 'created_on DESC') + assert_equal 'testfile.txt', a.filename + assert_equal Version.find(2), a.container + end + end diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index b22efd6ab4..9d1582af08 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -317,42 +317,6 @@ class ProjectsControllerTest < ActionController::TestCase assert_nil Project.find_by_id(1) end - def test_add_file - set_tmp_attachments_directory - @request.session[:user_id] = 2 - Setting.notified_events = ['file_added'] - ActionMailer::Base.deliveries.clear - - assert_difference 'Attachment.count' do - post :add_file, :id => 1, :version_id => '', - :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}} - end - assert_redirected_to 'projects/ecookbook/files' - a = Attachment.find(:first, :order => 'created_on DESC') - assert_equal 'testfile.txt', a.filename - assert_equal Project.find(1), a.container - - mail = ActionMailer::Base.deliveries.last - assert_kind_of TMail::Mail, mail - assert_equal "[eCookbook] New file", mail.subject - assert mail.body.include?('testfile.txt') - end - - def test_add_version_file - set_tmp_attachments_directory - @request.session[:user_id] = 2 - Setting.notified_events = ['file_added'] - - assert_difference 'Attachment.count' do - post :add_file, :id => 1, :version_id => '2', - :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}} - end - assert_redirected_to 'projects/ecookbook/files' - a = Attachment.find(:first, :order => 'created_on DESC') - assert_equal 'testfile.txt', a.filename - assert_equal Version.find(2), a.container - end - def test_archive @request.session[:user_id] = 1 # admin post :archive, :id => 1 diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index cc1f451cfd..66836c4f93 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -173,7 +173,7 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/4223/settings/members", :controller => 'projects', :action => 'settings', :id => '4223', :tab => 'members' should_route :get, "/projects/567/destroy", :controller => 'projects', :action => 'destroy', :id => '567' should_route :get, "/projects/33/files", :controller => 'files', :action => 'index', :id => '33' - should_route :get, "/projects/33/files/new", :controller => 'projects', :action => 'add_file', :id => '33' + should_route :get, "/projects/33/files/new", :controller => 'files', :action => 'new', :id => '33' should_route :get, "/projects/33/roadmap", :controller => 'versions', :action => 'index', :project_id => '33' should_route :get, "/projects/33/activity", :controller => 'activities', :action => 'index', :id => '33' should_route :get, "/projects/33/activity.atom", :controller => 'activities', :action => 'index', :id => '33', :format => 'atom' @@ -182,7 +182,7 @@ class RoutingTest < ActionController::IntegrationTest should_route :post, "/projects.xml", :controller => 'projects', :action => 'add', :format => 'xml' should_route :post, "/projects/4223/edit", :controller => 'projects', :action => 'edit', :id => '4223' should_route :post, "/projects/64/destroy", :controller => 'projects', :action => 'destroy', :id => '64' - should_route :post, "/projects/33/files/new", :controller => 'projects', :action => 'add_file', :id => '33' + should_route :post, "/projects/33/files/new", :controller => 'files', :action => 'new', :id => '33' should_route :post, "/projects/64/archive", :controller => 'projects', :action => 'archive', :id => '64' should_route :post, "/projects/64/unarchive", :controller => 'projects', :action => 'unarchive', :id => '64' should_route :post, "/projects/64/activities/save", :controller => 'projects', :action => 'save_activities', :id => '64'