Refactor: move method, ProjectsController#add_file to FilesController#new.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4052 e93f8b46-1217-0410-a6f0-8f06a7374b81
pull/351/head
Eric Davis 14 years ago
parent daa8eaa9ae
commit b5e90972d8
  1. 15
      app/controllers/files_controller.rb
  2. 16
      app/controllers/projects_controller.rb
  3. 2
      app/views/files/index.html.erb
  4. 2
      app/views/files/new.html.erb
  5. 4
      config/routes.rb
  6. 2
      lib/redmine.rb
  7. 38
      test/functional/files_controller_test.rb
  8. 36
      test/functional/projects_controller_test.rb
  9. 4
      test/integration/routing_test.rb

@ -19,4 +19,19 @@ class FilesController < ApplicationController
render :layout => !request.xhr? render :layout => !request.xhr?
end 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 end

@ -18,7 +18,6 @@
class ProjectsController < ApplicationController class ProjectsController < ApplicationController
menu_item :overview menu_item :overview
menu_item :roadmap, :only => :roadmap menu_item :roadmap, :only => :roadmap
menu_item :files, :only => [:add_file]
menu_item :settings, :only => :settings menu_item :settings, :only => :settings
before_filter :find_project, :except => [ :index, :list, :add, :copy ] before_filter :find_project, :except => [ :index, :list, :add, :copy ]
@ -239,21 +238,6 @@ class ProjectsController < ApplicationController
@project = nil @project = nil
end 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 def save_activities
if request.post? && params[:enumerations] if request.post? && params[:enumerations]
Project.transaction do Project.transaction do

@ -1,5 +1,5 @@
<div class="contextual"> <div class="contextual">
<%= 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' %>
</div> </div>
<h2><%=l(:label_attachment_plural)%></h2> <h2><%=l(:label_attachment_plural)%></h2>

@ -2,7 +2,7 @@
<%= error_messages_for 'attachment' %> <%= error_messages_for 'attachment' %>
<div class="box"> <div class="box">
<% 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? %> <% if @versions.any? %>
<p><label for="version_id"><%=l(:field_version)%></label> <p><label for="version_id"><%=l(:field_version)%></label>

@ -182,7 +182,7 @@ ActionController::Routing::Routes.draw do |map|
project_views.connect 'projects/:id.:format', :action => 'show' project_views.connect 'projects/:id.:format', :action => 'show'
project_views.connect 'projects/:id/:action', :action => /destroy|settings/ 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', :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/:id/settings/:tab', :action => 'settings'
project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new' project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new'
end end
@ -199,7 +199,7 @@ ActionController::Routing::Routes.draw do |map|
project_actions.connect 'projects', :action => 'add' project_actions.connect 'projects', :action => 'add'
project_actions.connect 'projects.:format', :action => 'add', :format => /xml/ 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/: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' project_actions.connect 'projects/:id/activities/save', :action => 'save_activities'
end end

@ -102,7 +102,7 @@ Redmine::AccessControl.map do |map|
end end
map.project_module :files do |map| 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 map.permission :view_files, :files => :index, :versions => :download
end end

@ -26,4 +26,42 @@ class FilesControllerTest < ActionController::TestCase
:attributes => { :href => '/attachments/download/9/version_file.zip' } :attributes => { :href => '/attachments/download/9/version_file.zip' }
end 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 end

@ -317,42 +317,6 @@ class ProjectsControllerTest < ActionController::TestCase
assert_nil Project.find_by_id(1) assert_nil Project.find_by_id(1)
end 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 def test_archive
@request.session[:user_id] = 1 # admin @request.session[:user_id] = 1 # admin
post :archive, :id => 1 post :archive, :id => 1

@ -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/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/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", :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/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", :controller => 'activities', :action => 'index', :id => '33'
should_route :get, "/projects/33/activity.atom", :controller => 'activities', :action => 'index', :id => '33', :format => 'atom' 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.xml", :controller => 'projects', :action => 'add', :format => 'xml'
should_route :post, "/projects/4223/edit", :controller => 'projects', :action => 'edit', :id => '4223' 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/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/archive", :controller => 'projects', :action => 'archive', :id => '64'
should_route :post, "/projects/64/unarchive", :controller => 'projects', :action => 'unarchive', :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' should_route :post, "/projects/64/activities/save", :controller => 'projects', :action => 'save_activities', :id => '64'

Loading…
Cancel
Save