From 1b90703157a182d37496ae4c8e3c430681abddc0 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Tue, 14 Sep 2010 16:24:07 +0000 Subject: [PATCH] Refactor: convert FilesController to a restful resource. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4085 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/files_controller.rb | 4 ++-- app/views/files/index.html.erb | 2 +- app/views/files/new.html.erb | 2 +- config/routes.rb | 7 +------ lib/redmine.rb | 2 +- test/functional/files_controller_test.rb | 6 +++--- test/integration/routing_test.rb | 6 +++--- 7 files changed, 12 insertions(+), 17 deletions(-) diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb index 0a4903d08..72a4a2411 100644 --- a/app/controllers/files_controller.rb +++ b/app/controllers/files_controller.rb @@ -1,7 +1,7 @@ class FilesController < ApplicationController menu_item :files - before_filter :find_project + before_filter :find_project_by_project_id before_filter :authorize helper :sort @@ -31,6 +31,6 @@ class FilesController < ApplicationController if !attachments.empty? && Setting.notified_events.include?('file_added') Mailer.deliver_attachments_added(attachments[:files]) end - redirect_to :controller => 'files', :action => 'index', :id => @project + redirect_to project_files_path(@project) end end diff --git a/app/views/files/index.html.erb b/app/views/files/index.html.erb index 66f5d12b9..20710402b 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 => 'files', :action => 'new', :id => @project}, :class => 'icon icon-add' %> +<%= link_to_if_authorized l(:label_attachment_new), new_project_file_path(@project), :class => 'icon icon-add' %>

<%=l(:label_attachment_plural)%>

diff --git a/app/views/files/new.html.erb b/app/views/files/new.html.erb index 870a315c9..d9d1b6ee1 100644 --- a/app/views/files/new.html.erb +++ b/app/views/files/new.html.erb @@ -2,7 +2,7 @@ <%= error_messages_for 'attachment' %>
-<% form_tag({ :action => 'create', :id => @project }, :multipart => true, :class => "tabular") do %> +<% form_tag(project_files_path(@project), :multipart => true, :class => "tabular") do %> <% if @versions.any? %>

diff --git a/config/routes.rb b/config/routes.rb index 152b90f54..c85f23eb2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -181,6 +181,7 @@ ActionController::Routing::Routes.draw do |map| :unarchive => :post } do |project| project.resource :project_enumerations, :as => 'enumerations', :only => [:update, :destroy] + project.resources :files, :only => [:index, :new, :create] end # Destroy uses a get request to prompt the user before the actual DELETE request @@ -189,15 +190,9 @@ ActionController::Routing::Routes.draw do |map| # TODO: port to be part of the resources route(s) map.with_options :controller => 'projects' do |project_mapper| project_mapper.with_options :conditions => {:method => :get} do |project_views| - project_views.connect 'projects/:id/files', :controller => 'files', :action => 'index' - project_views.connect 'projects/:id/files/new', :controller => 'files', :action => 'new' project_views.connect 'projects/:id/settings/:tab', :controller => 'projects', :action => 'settings' project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new' end - - project_mapper.with_options :conditions => {:method => :post} do |project_actions| - project_actions.connect 'projects/:id/files/new', :controller => 'files', :action => 'create' - end end map.with_options :controller => 'activities', :action => 'index', :conditions => {:method => :get} do |activity| diff --git a/lib/redmine.rb b/lib/redmine.rb index 9d59390c1..f8f564220 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -198,7 +198,7 @@ Redmine::MenuManager.map :project_menu do |menu| :if => Proc.new { |p| p.wiki && !p.wiki.new_record? } menu.push :boards, { :controller => 'boards', :action => 'index', :id => nil }, :param => :project_id, :if => Proc.new { |p| p.boards.any? }, :caption => :label_board_plural - menu.push :files, { :controller => 'files', :action => 'index' }, :caption => :label_file_plural + menu.push :files, { :controller => 'files', :action => 'index' }, :caption => :label_file_plural, :param => :project_id menu.push :repository, { :controller => 'repositories', :action => 'show' }, :if => Proc.new { |p| p.repository && !p.repository.new_record? } menu.push :settings, { :controller => 'projects', :action => 'settings' }, :last => true diff --git a/test/functional/files_controller_test.rb b/test/functional/files_controller_test.rb index 2f6f009e7..6035c4be5 100644 --- a/test/functional/files_controller_test.rb +++ b/test/functional/files_controller_test.rb @@ -12,7 +12,7 @@ class FilesControllerTest < ActionController::TestCase end def test_index - get :index, :id => 1 + get :index, :project_id => 1 assert_response :success assert_template 'index' assert_not_nil assigns(:containers) @@ -33,7 +33,7 @@ class FilesControllerTest < ActionController::TestCase ActionMailer::Base.deliveries.clear assert_difference 'Attachment.count' do - post :create, :id => 1, :version_id => '', + post :create, :project_id => 1, :version_id => '', :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}} assert_response :redirect end @@ -54,7 +54,7 @@ class FilesControllerTest < ActionController::TestCase Setting.notified_events = ['file_added'] assert_difference 'Attachment.count' do - post :create, :id => 1, :version_id => '2', + post :create, :project_id => 1, :version_id => '2', :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}} assert_response :redirect end diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 1867a60b0..38c1b2a0e 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -171,15 +171,15 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/1.xml", :controller => 'projects', :action => 'show', :id => '1', :format => 'xml' should_route :get, "/projects/4223/settings", :controller => 'projects', :action => 'settings', :id => '4223' should_route :get, "/projects/4223/settings/members", :controller => 'projects', :action => 'settings', :id => '4223', :tab => 'members' - should_route :get, "/projects/33/files", :controller => 'files', :action => 'index', :id => '33' - should_route :get, "/projects/33/files/new", :controller => 'files', :action => 'new', :id => '33' + should_route :get, "/projects/33/files", :controller => 'files', :action => 'index', :project_id => '33' + should_route :get, "/projects/33/files/new", :controller => 'files', :action => 'new', :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.atom", :controller => 'activities', :action => 'index', :id => '33', :format => 'atom' should_route :post, "/projects", :controller => 'projects', :action => 'create' should_route :post, "/projects.xml", :controller => 'projects', :action => 'create', :format => 'xml' - should_route :post, "/projects/33/files/new", :controller => 'files', :action => 'create', :id => '33' + should_route :post, "/projects/33/files", :controller => 'files', :action => 'create', :project_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'