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

diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb
index c84ce5f51b13a9a32710f604845875953e4587d7..fe5eb48c8b4a4d3fc305177a5ac59ca71a2e7c7e 100644 (file)
--- 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 05c8d969d6f96a2a47e34f0a9574a0680a066fcd..61c7a7c71c5e4e0cf96a928375ff36b9d1b9b0d5 100644 (file)
--- 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 2b2e5e870cbff185f6f461748e47dca1ecfd5547..66f5d12b9ad55a2065565ed8fe897a426481ccca 100644 (file)
--- a/app/views/files/index.html.erb
+++ b/app/views/files/index.html.erb
@@ -1,5 +1,5 @@
 <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>
 
 <h2><%=l(:label_attachment_plural)%></h2>

diff --git a/app/views/files/new.html.erb b/app/views/files/new.html.erb
new file mode 100644 (file)
index 0000000..bbb3b17
--- /dev/null
+++ b/app/views/files/new.html.erb
@@ -0,0 +1,16 @@
+<h2><%=l(:label_attachment_new)%></h2>
+
+<%= error_messages_for 'attachment' %>
+<div class="box">
+<% form_tag({ :action => 'new', :id => @project }, :multipart => true, :class => "tabular") do %>
+
+<% if @versions.any? %>
+<p><label for="version_id"><%=l(:field_version)%></label>
+<%= select_tag "version_id", content_tag('option', '') +
+                                                                                                                options_from_collection_for_select(@versions, "id", "name") %></p>
+<% end %>
+
+<p><label><%=l(:label_attachment_plural)%></label><%= render :partial => 'attachments/form' %></p>
+</div>
+<%= submit_tag l(:button_add) %>
+<% end %> 

diff --git a/app/views/projects/add_file.rhtml b/app/views/projects/add_file.rhtml
deleted file mode 100644 (file)
index ab9c735..0000000
--- a/app/views/projects/add_file.rhtml
+++ /dev/null
@@ -1,16 +0,0 @@
-<h2><%=l(:label_attachment_new)%></h2>
-
-<%= error_messages_for 'attachment' %>
-<div class="box">
-<% form_tag({ :action => 'add_file', :id => @project }, :multipart => true, :class => "tabular") do %>
-
-<% if @versions.any? %>
-<p><label for="version_id"><%=l(:field_version)%></label>
-<%= select_tag "version_id", content_tag('option', '') +
-                                                                                                                options_from_collection_for_select(@versions, "id", "name") %></p>
-<% end %>
-
-<p><label><%=l(:label_attachment_plural)%></label><%= render :partial => 'attachments/form' %></p>
-</div>
-<%= submit_tag l(:button_add) %>
-<% end %> 

diff --git a/config/routes.rb b/config/routes.rb
index 270bab4a0b2b644a354b1af192059933e3a0bcab..9c8a2f6ae8d752e09b7d61d89f1aeaab43da4a39 100644 (file)
--- 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 726a2ad92fe012df17b127ee1bb8252c9c000d55..745c6d750f8dc24b859c376749a476f400a634d7 100644 (file)
--- 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 9085021833569dfd0feea2a2572404cac2142455..838caee70e3d17828e1a541769cf329f69acf2f9 100644 (file)
--- 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 b22efd6ab4585a516f3253dabbcad51cfdc09c68..9d1582af087204500f9746003d81a82c59825fd4 100644 (file)
--- 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 cc1f451cfdf1e48e677a85bc3c3ff0a915f8455e..66836c4f93c1a96d6a4e267da0a42d78e0fcef68 100644 (file)
--- 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'