Refactor: move method, ProjectsController#list_files to FilesController#index.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4051 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb
new file mode 100644 (file)
index 0000000..c84ce5f
--- /dev/null
+++ b/app/controllers/files_controller.rb
@@ -0,0 +1,22 @@
+class FilesController < ApplicationController
+  menu_item :files
+
+  before_filter :find_project
+  before_filter :authorize
+
+  helper :sort
+  include SortHelper
+
+  def index
+    sort_init 'filename', 'asc'
+    sort_update 'filename' => "#{Attachment.table_name}.filename",
+                'created_on' => "#{Attachment.table_name}.created_on",
+                'size' => "#{Attachment.table_name}.filesize",
+                'downloads' => "#{Attachment.table_name}.downloads"
+                
+    @containers = [ Project.find(@project.id, :include => :attachments, :order => sort_clause)]
+    @containers += @project.versions.find(:all, :include => :attachments, :order => sort_clause).sort.reverse
+    render :layout => !request.xhr?
+  end
+
+end

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 845ab3c157f22e7e04d538e42e3157eca9a34d9e..05c8d969d6f96a2a47e34f0a9574a0680a066fcd 100644 (file)
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -18,7 +18,7 @@
 class ProjectsController < ApplicationController
   menu_item :overview
   menu_item :roadmap, :only => :roadmap
-  menu_item :files, :only => [:list_files, :add_file]
+  menu_item :files, :only => [:add_file]
   menu_item :settings, :only => :settings
   
   before_filter :find_project, :except => [ :index, :list, :add, :copy ]
@@ -248,7 +248,7 @@ class ProjectsController < ApplicationController
       if !attachments.empty? && Setting.notified_events.include?('file_added')
         Mailer.deliver_attachments_added(attachments[:files])
       end
-      redirect_to :controller => 'projects', :action => 'list_files', :id => @project
+      redirect_to :controller => 'files', :action => 'index', :id => @project
       return
     end
     @versions = @project.versions.sort
@@ -275,18 +275,6 @@ class ProjectsController < ApplicationController
     redirect_to :controller => 'projects', :action => 'settings', :tab => 'activities', :id => @project
   end
   
-  def list_files
-    sort_init 'filename', 'asc'
-    sort_update 'filename' => "#{Attachment.table_name}.filename",
-                'created_on' => "#{Attachment.table_name}.created_on",
-                'size' => "#{Attachment.table_name}.filesize",
-                'downloads' => "#{Attachment.table_name}.downloads"
-                
-    @containers = [ Project.find(@project.id, :include => :attachments, :order => sort_clause)]
-    @containers += @project.versions.find(:all, :include => :attachments, :order => sort_clause).sort.reverse
-    render :layout => !request.xhr?
-  end
-
 private
   def find_optional_project
     return true unless params[:id]

diff --git a/app/views/files/index.html.erb b/app/views/files/index.html.erb
new file mode 100644 (file)
index 0000000..2b2e5e8
--- /dev/null
+++ b/app/views/files/index.html.erb
@@ -0,0 +1,46 @@
+<div class="contextual">
+<%= link_to_if_authorized l(:label_attachment_new), {:controller => 'projects', :action => 'add_file', :id => @project}, :class => 'icon icon-add' %>
+</div>
+
+<h2><%=l(:label_attachment_plural)%></h2>
+
+<% delete_allowed = User.current.allowed_to?(:manage_files, @project) %>
+
+<table class="list files">
+  <thead><tr>
+    <%= sort_header_tag('filename', :caption => l(:field_filename)) %>
+    <%= sort_header_tag('created_on', :caption => l(:label_date), :default_order => 'desc') %>
+    <%= sort_header_tag('size', :caption => l(:field_filesize), :default_order => 'desc') %>
+    <%= sort_header_tag('downloads', :caption => l(:label_downloads_abbr), :default_order => 'desc') %>
+    <th>MD5</th>
+    <th></th>
+  </tr></thead>
+  <tbody>
+<% @containers.each do |container| %>  
+  <% next if container.attachments.empty? -%>
+       <% if container.is_a?(Version) -%>
+  <tr>
+       <th colspan="6" align="left">
+               <%= link_to(h(container), {:controller => 'versions', :action => 'show', :id => container}, :class => "icon icon-package") %>
+               </th>
+       </tr>
+       <% end -%>
+  <% container.attachments.each do |file| %>           
+  <tr class="file <%= cycle("odd", "even") %>">
+    <td class="filename"><%= link_to_attachment file, :download => true, :title => file.description %></td>
+    <td class="created_on"><%= format_time(file.created_on) %></td>
+    <td class="filesize"><%= number_to_human_size(file.filesize) %></td>
+    <td class="downloads"><%= file.downloads %></td>
+    <td class="digest"><%= file.digest %></td>
+    <td align="center">
+    <%= link_to(image_tag('delete.png'), {:controller => 'attachments', :action => 'destroy', :id => file},
+                                                                                                                                                                :confirm => l(:text_are_you_sure), :method => :post) if delete_allowed %>
+    </td>
+  </tr>                
+  <% end
+  reset_cycle %>
+<% end %>
+  </tbody>
+</table>
+
+<% html_title(l(:label_attachment_plural)) -%>

diff --git a/app/views/projects/list_files.rhtml b/app/views/projects/list_files.rhtml
deleted file mode 100644 (file)
index 2b2e5e8..0000000
--- a/app/views/projects/list_files.rhtml
+++ /dev/null
@@ -1,46 +0,0 @@
-<div class="contextual">
-<%= link_to_if_authorized l(:label_attachment_new), {:controller => 'projects', :action => 'add_file', :id => @project}, :class => 'icon icon-add' %>
-</div>
-
-<h2><%=l(:label_attachment_plural)%></h2>
-
-<% delete_allowed = User.current.allowed_to?(:manage_files, @project) %>
-
-<table class="list files">
-  <thead><tr>
-    <%= sort_header_tag('filename', :caption => l(:field_filename)) %>
-    <%= sort_header_tag('created_on', :caption => l(:label_date), :default_order => 'desc') %>
-    <%= sort_header_tag('size', :caption => l(:field_filesize), :default_order => 'desc') %>
-    <%= sort_header_tag('downloads', :caption => l(:label_downloads_abbr), :default_order => 'desc') %>
-    <th>MD5</th>
-    <th></th>
-  </tr></thead>
-  <tbody>
-<% @containers.each do |container| %>  
-  <% next if container.attachments.empty? -%>
-       <% if container.is_a?(Version) -%>
-  <tr>
-       <th colspan="6" align="left">
-               <%= link_to(h(container), {:controller => 'versions', :action => 'show', :id => container}, :class => "icon icon-package") %>
-               </th>
-       </tr>
-       <% end -%>
-  <% container.attachments.each do |file| %>           
-  <tr class="file <%= cycle("odd", "even") %>">
-    <td class="filename"><%= link_to_attachment file, :download => true, :title => file.description %></td>
-    <td class="created_on"><%= format_time(file.created_on) %></td>
-    <td class="filesize"><%= number_to_human_size(file.filesize) %></td>
-    <td class="downloads"><%= file.downloads %></td>
-    <td class="digest"><%= file.digest %></td>
-    <td align="center">
-    <%= link_to(image_tag('delete.png'), {:controller => 'attachments', :action => 'destroy', :id => file},
-                                                                                                                                                                :confirm => l(:text_are_you_sure), :method => :post) if delete_allowed %>
-    </td>
-  </tr>                
-  <% end
-  reset_cycle %>
-<% end %>
-  </tbody>
-</table>
-
-<% html_title(l(:label_attachment_plural)) -%>

diff --git a/config/routes.rb b/config/routes.rb
index 1414e1000ed5882ca4a70fb7ed4f6d5387ebcb6d..270bab4a0b2b644a354b1af192059933e3a0bcab 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -181,7 +181,7 @@ ActionController::Routing::Routes.draw do |map|
       project_views.connect 'projects/:id', :action => 'show'
       project_views.connect 'projects/:id.:format', :action => 'show'
       project_views.connect 'projects/:id/:action', :action => /destroy|settings/
-      project_views.connect 'projects/:id/files', :action => 'list_files'
+      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/settings/:tab', :action => 'settings'
       project_views.connect 'projects/:project_id/issues/:copy_from/copy', :controller => 'issues', :action => 'new'

diff --git a/lib/redmine.rb b/lib/redmine.rb
index ecfc4625287f424fdbd58ccc97076d135cc3c885..726a2ad92fe012df17b127ee1bb8252c9c000d55 100644 (file)
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -103,7 +103,7 @@ Redmine::AccessControl.map do |map|
   
   map.project_module :files do |map|
     map.permission :manage_files, {:projects => :add_file}, :require => :loggedin
-    map.permission :view_files, :projects => :list_files, :versions => :download
+    map.permission :view_files, :files => :index, :versions => :download
   end
     
   map.project_module :wiki do |map|
@@ -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 => 'projects', :action => 'list_files' }, :caption => :label_file_plural
+  menu.push :files, { :controller => 'files', :action => 'index' }, :caption => :label_file_plural
   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
new file mode 100644 (file)
index 0000000..9085021
--- /dev/null
+++ b/test/functional/files_controller_test.rb
@@ -0,0 +1,29 @@
+require File.dirname(__FILE__) + '/../test_helper'
+
+class FilesControllerTest < ActionController::TestCase
+  fixtures :all
+  
+  def setup
+    @controller = FilesController.new
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    @request.session[:user_id] = nil
+    Setting.default_language = 'en'
+  end
+
+  def test_index
+    get :index, :id => 1
+    assert_response :success
+    assert_template 'index'
+    assert_not_nil assigns(:containers)
+    
+    # file attached to the project
+    assert_tag :a, :content => 'project_file.zip',
+                   :attributes => { :href => '/attachments/download/8/project_file.zip' }
+    
+    # file attached to a project's version
+    assert_tag :a, :content => 'version_file.zip',
+                   :attributes => { :href => '/attachments/download/9/version_file.zip' }
+  end
+
+end

diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb
index 36d60c859e366b004c3c6a89d75c3438d7d6a936..b22efd6ab4585a516f3253dabbcad51cfdc09c68 100644 (file)
--- a/test/functional/projects_controller_test.rb
+++ b/test/functional/projects_controller_test.rb
@@ -353,21 +353,6 @@ class ProjectsControllerTest < ActionController::TestCase
     assert_equal Version.find(2), a.container
   end
   
-  def test_list_files
-    get :list_files, :id => 1
-    assert_response :success
-    assert_template 'list_files'
-    assert_not_nil assigns(:containers)
-    
-    # file attached to the project
-    assert_tag :a, :content => 'project_file.zip',
-                   :attributes => { :href => '/attachments/download/8/project_file.zip' }
-    
-    # file attached to a project's version
-    assert_tag :a, :content => 'version_file.zip',
-                   :attributes => { :href => '/attachments/download/9/version_file.zip' }
-  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 590bd911904893a16851c1bb21ab70b768779a3a..cc1f451cfdf1e48e677a85bc3c3ff0a915f8455e 100644 (file)
--- a/test/integration/routing_test.rb
+++ b/test/integration/routing_test.rb
@@ -172,7 +172,7 @@ class RoutingTest < ActionController::IntegrationTest
     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/567/destroy", :controller => 'projects', :action => 'destroy', :id => '567'
-    should_route :get, "/projects/33/files", :controller => 'projects', :action => 'list_files', :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/roadmap", :controller => 'versions', :action => 'index', :project_id => '33'
     should_route :get, "/projects/33/activity", :controller => 'activities', :action => 'index', :id => '33'