Appends the filename to the attachment url so that clients that ignore content-dispos...

git-svn-id: http://redmine.rubyforge.org/svn/trunk@1686 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index 07fee1269be0cc59424d2098e5efa3daedff12cc..1e8f566e6cc95928e128223983653a7df2863111 100644 (file)
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -43,6 +43,9 @@ class AttachmentsController < ApplicationController
 private
   def find_project
     @attachment = Attachment.find(params[:id])
+    # Show 404 if the filename in the url is wrong
+    raise ActiveRecord::RecordNotFound if params[:filename] && params[:filename] != @attachment.filename
+    
     @project = @attachment.project
     permission = @attachment.container.is_a?(Version) ? :view_files : "view_#{@attachment.container.class.name.underscore.pluralize}".to_sym
     allowed = User.current.allowed_to?(permission, @project)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 6c69d8e6f10222404f2583c7ac84be978582eb04..6e39d093fb9904b627a23aae93454f711114422e 100644 (file)
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -47,6 +47,17 @@ module ApplicationHelper
     link_to "#{issue.tracker.name} ##{issue.id}", {:controller => "issues", :action => "show", :id => issue}, options
   end
   
+  # Generates a link to an attachment.
+  # Options:
+  # * :text - Link text (default to attachment filename)
+  # * :download - Force download (default: false)
+  def link_to_attachment(attachment, options={})
+    text = options.delete(:text) || attachment.filename
+    action = options.delete(:download) ? 'download' : 'show'
+    
+    link_to(h(text), {:controller => 'attachments', :action => action, :id => attachment, :filename => attachment.filename }, options)
+  end
+  
   def toggle_link(name, id, options={})
     onclick = "Element.toggle('#{id}'); "
     onclick << (options[:focus] ? "Form.Element.focus('#{options[:focus]}'); " : "this.blur(); ")

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 40369717851066ce32a524a3f17f8b568a282863..9e419ab43863717168e385013ae650b992d7d40c 100644 (file)
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -95,9 +95,9 @@ module IssuesHelper
       label = content_tag('strong', label)
       old_value = content_tag("i", h(old_value)) if detail.old_value
       old_value = content_tag("strike", old_value) if detail.old_value and (!detail.value or detail.value.empty?)
-      if detail.property == 'attachment' && !value.blank? && Attachment.find_by_id(detail.prop_key)
+      if detail.property == 'attachment' && !value.blank? && a = Attachment.find_by_id(detail.prop_key)
         # Link to the attachment if it has not been removed
-        value = link_to(value, :controller => 'attachments', :action => 'show', :id => detail.prop_key)
+        value = link_to_attachment(a)
       else
         value = content_tag("i", h(value)) if value
       end

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 7d6a74ebb9c8d5d1ce36975459968bcc68ded142..8f3f530c7a1d411c84244a6f5f6743d78836eb77 100644 (file)
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -26,7 +26,7 @@ class Attachment < ActiveRecord::Base
   validates_length_of :disk_filename, :maximum => 255
 
   acts_as_event :title => :filename,
-                :url => Proc.new {|o| {:controller => 'attachments', :action => 'download', :id => o.id}}
+                :url => Proc.new {|o| {:controller => 'attachments', :action => 'download', :id => o.id, :filename => o.filename}}
 
   cattr_accessor :storage_path
   @@storage_path = "#{RAILS_ROOT}/files"

diff --git a/app/views/attachments/_links.rhtml b/app/views/attachments/_links.rhtml
index 9e3ac747c7ed0e759c3d0c78295cea0414ff19ec..9aae909fee8157d5d0aa1ac7ad5e680be0bae81f 100644 (file)
--- a/app/views/attachments/_links.rhtml
+++ b/app/views/attachments/_links.rhtml
@@ -1,6 +1,6 @@
 <div class="attachments">
 <% for attachment in attachments %>
-<p><%= link_to attachment.filename, {:controller => 'attachments', :action => 'show', :id => attachment }, :class => 'icon icon-attachment' -%>
+<p><%= link_to_attachment attachment, :class => 'icon icon-attachment' -%>
 <%= h(" - #{attachment.description}") unless attachment.description.blank? %>
   <span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
   <% if options[:delete_url] %>

diff --git a/app/views/attachments/diff.rhtml b/app/views/attachments/diff.rhtml
index 4a9f5c9bdbdebcb6ee98ae5a3b8f9f70d6741179..7b64dca17a8298d0e0769e2e6def148084d69984 100644 (file)
--- a/app/views/attachments/diff.rhtml
+++ b/app/views/attachments/diff.rhtml
@@ -3,7 +3,7 @@
 <div class="attachments">
 <p><%= h("#{@attachment.description} - ") unless @attachment.description.blank? %>
    <span class="author"><%= @attachment.author %>, <%= format_time(@attachment.created_on) %></span></p>
-<p><%= link_to l(:button_download), {:controller => 'attachments', :action => 'download', :id => @attachment } -%>
+<p><%= link_to_attachment @attachment, :text => l(:button_download), :download => true -%>
    <span class="size">(<%= number_to_human_size @attachment.filesize %>)</span></p>
 
 </div>

diff --git a/app/views/attachments/file.rhtml b/app/views/attachments/file.rhtml
index 7988d0aedcdaf07dd9766ad37d05beffd5061d50..468c6b666a2b68b8aec3aa2933dc8fbe23e1ccf6 100644 (file)
--- a/app/views/attachments/file.rhtml
+++ b/app/views/attachments/file.rhtml
@@ -3,7 +3,7 @@
 <div class="attachments">
 <p><%= h("#{@attachment.description} - ") unless @attachment.description.blank? %>
    <span class="author"><%= @attachment.author %>, <%= format_time(@attachment.created_on) %></span></p>
-<p><%= link_to l(:button_download), {:controller => 'attachments', :action => 'download', :id => @attachment } -%>
+<p><%= link_to_attachment @attachment, :text => l(:button_download), :download => true -%>
    <span class="size">(<%= number_to_human_size @attachment.filesize %>)</span></p>
 
 </div>

diff --git a/app/views/projects/list_files.rhtml b/app/views/projects/list_files.rhtml
index 43687c50a49d64fe14c2a864bd3f1c78e9108903..79e41f16dd05113bb6d1029c947f4c244bb99bf6 100644 (file)
--- a/app/views/projects/list_files.rhtml
+++ b/app/views/projects/list_files.rhtml
@@ -23,8 +23,7 @@
   <% for file in version.attachments %>                
   <tr class="<%= cycle("odd", "even") %>">
     <td></td>
-    <td><%= link_to(h(file.filename), {:controller => 'attachments', :action => 'download', :id => file},
-                                      :title => file.description) %></td>
+    <td><%= link_to_attachment file, :download => true, :title => file.description %></td>
     <td align="center"><%= format_time(file.created_on) %></td>
     <td align="center"><%= number_to_human_size(file.filesize) %></td>
     <td align="center"><%= file.downloads %></td>

diff --git a/config/routes.rb b/config/routes.rb
index a77a8833b2c8ce46abeea6bdb08d5cd2ebdf9a10..c34052f86b9a951bd6f77962f4ce38570d86095c 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -32,8 +32,10 @@ ActionController::Routing::Routes.draw do |map|
     omap.repositories_revision 'repositories/revision/:id/:rev', :action => 'revision'
   end
   
-  map.connect 'attachments/:id', :controller => 'attachments', :action => 'show'
-  
+  map.connect 'attachments/:id', :controller => 'attachments', :action => 'show', :id => /\d+/
+  map.connect 'attachments/:id/:filename', :controller => 'attachments', :action => 'show', :id => /\d+/, :filename => /.*/
+  map.connect 'attachments/download/:id/:filename', :controller => 'attachments', :action => 'download', :id => /\d+/, :filename => /.*/
+   
   # Allow downloading Web Service WSDL as a file with an extension
   # instead of a file named 'wsdl'
   map.connect ':controller/service.wsdl', :action => 'wsdl'

diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb
index af73eb77ed4266f8133f4fe8a8bbb4e4cb78bb67..06a6343bae5bc35cea2fabe0966a13e61758ddf5 100644 (file)
--- a/test/functional/attachments_controller_test.rb
+++ b/test/functional/attachments_controller_test.rb
@@ -33,6 +33,21 @@ class AttachmentsControllerTest < Test::Unit::TestCase
     User.current = nil
   end
   
+  def test_routing
+    assert_routing('/attachments/1', :controller => 'attachments', :action => 'show', :id => '1')
+    assert_routing('/attachments/1/filename.ext', :controller => 'attachments', :action => 'show', :id => '1', :filename => 'filename.ext')
+    assert_routing('/attachments/download/1', :controller => 'attachments', :action => 'download', :id => '1')
+    assert_routing('/attachments/download/1/filename.ext', :controller => 'attachments', :action => 'download', :id => '1', :filename => 'filename.ext')
+  end
+  
+  def test_recognizes
+    assert_recognizes({:controller => 'attachments', :action => 'show', :id => '1'}, '/attachments/1')
+    assert_recognizes({:controller => 'attachments', :action => 'show', :id => '1'}, '/attachments/show/1')
+    assert_recognizes({:controller => 'attachments', :action => 'show', :id => '1', :filename => 'filename.ext'}, '/attachments/1/filename.ext')
+    assert_recognizes({:controller => 'attachments', :action => 'download', :id => '1'}, '/attachments/download/1')
+    assert_recognizes({:controller => 'attachments', :action => 'download', :id => '1', :filename => 'filename.ext'},'/attachments/download/1/filename.ext')
+  end
+  
   def test_show_diff
     get :show, :id => 5
     assert_response :success