Feature #7056 » compress_the_all_attachments_in_issue.patch
| Gemfile | ||
|---|---|---|
| 14 | 14 |
gem "mimemagic" |
| 15 | 15 |
gem "mail", "~> 2.6.4" |
| 16 | 16 |
gem "csv", "~> 1.0.2" if RUBY_VERSION >= "2.3" |
| 17 |
gem "rubyzip", "~> 1.2.1" |
|
| 17 | 18 | |
| 18 | 19 |
gem "nokogiri", "~> 1.8.0" |
| 19 | 20 |
gem "i18n", "~> 0.7.0" |
| app/controllers/attachments_controller.rb | ||
|---|---|---|
| 17 | 17 | |
| 18 | 18 |
class AttachmentsController < ApplicationController |
| 19 | 19 |
before_action :find_attachment, :only => [:show, :download, :thumbnail, :update, :destroy] |
| 20 |
before_action :find_container, :only => [:edit_all, :update_all, :download_all] |
|
| 20 | 21 |
before_action :find_editable_attachments, :only => [:edit_all, :update_all] |
| 21 | 22 |
before_action :file_readable, :read_authorize, :only => [:show, :download, :thumbnail] |
| 22 | 23 |
before_action :update_authorize, :only => :update |
| ... | ... | |
| 129 | 130 |
render :action => 'edit_all' |
| 130 | 131 |
end |
| 131 | 132 | |
| 133 |
def download_all |
|
| 134 |
begin |
|
| 135 |
temp_file = Attachment.attachments_to_zip(@container.attachments) |
|
| 136 |
render_404 and return if temp_file.nil? |
|
| 137 |
send_data(File.read(temp_file.path), :type => 'application/zip', :filename => "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip")
|
|
| 138 |
ensure |
|
| 139 |
# Close and delete the temp file |
|
| 140 |
unless temp_file.nil? |
|
| 141 |
temp_file.close |
|
| 142 |
temp_file.unlink |
|
| 143 |
end |
|
| 144 |
end |
|
| 145 |
end |
|
| 146 | ||
| 132 | 147 |
def update |
| 133 | 148 |
@attachment.safe_attributes = params[:attachment] |
| 134 | 149 |
saved = @attachment.save |
| ... | ... | |
| 190 | 205 |
end |
| 191 | 206 | |
| 192 | 207 |
def find_editable_attachments |
| 208 |
@attachments = @container.attachments.select(&:editable?) |
|
| 209 |
render_404 if @attachments.empty? |
|
| 210 |
end |
|
| 211 | ||
| 212 |
def find_container |
|
| 193 | 213 |
klass = params[:object_type].to_s.singularize.classify.constantize rescue nil |
| 194 | 214 |
unless klass && klass.reflect_on_association(:attachments) |
| 195 | 215 |
render_404 |
| ... | ... | |
| 201 | 221 |
render_403 |
| 202 | 222 |
return |
| 203 | 223 |
end |
| 204 |
@attachments = @container.attachments.select(&:editable?) |
|
| 205 | 224 |
if @container.respond_to?(:project) |
| 206 | 225 |
@project = @container.project |
| 207 | 226 |
end |
| 208 |
render_404 if @attachments.empty? |
|
| 209 | 227 |
rescue ActiveRecord::RecordNotFound |
| 210 | 228 |
render_404 |
| 211 | 229 |
end |
| app/helpers/attachments_helper.rb | ||
|---|---|---|
| 27 | 27 |
object_attachments_path container.class.name.underscore.pluralize, container.id |
| 28 | 28 |
end |
| 29 | 29 | |
| 30 |
def container_attachments_download_path(container) |
|
| 31 |
object_attachments_download_path container.class.name.underscore.pluralize, container.id |
|
| 32 |
end |
|
| 33 | ||
| 30 | 34 |
# Displays view/delete links to the attachments of the given object |
| 31 | 35 |
# Options: |
| 32 | 36 |
# :author -- author names are not displayed if set to false |
| app/models/attachment.rb | ||
|---|---|---|
| 17 | 17 | |
| 18 | 18 |
require "digest" |
| 19 | 19 |
require "fileutils" |
| 20 |
require "zip" |
|
| 20 | 21 | |
| 21 | 22 |
class Attachment < ActiveRecord::Base |
| 22 | 23 |
include Redmine::SafeAttributes |
| ... | ... | |
| 333 | 334 |
Attachment.where("created_on < ? AND (container_type IS NULL OR container_type = '')", Time.now - age).destroy_all
|
| 334 | 335 |
end |
| 335 | 336 | |
| 337 |
def self.attachments_to_zip(attachments) |
|
| 338 |
attachments = attachments.select{|attachment| File.readable?(attachment.diskfile) }
|
|
| 339 |
return nil if attachments.blank? |
|
| 340 |
temp_file = Tempfile.new('attachments_zip', Rails.root.join(Attachment.storage_path))
|
|
| 341 |
Zip.unicode_names = true |
|
| 342 |
existing_file_names = [] |
|
| 343 |
Zip::File.open(temp_file.path, Zip::File::CREATE) do |zip| |
|
| 344 |
attachments.each do |attachment| |
|
| 345 |
filename = attachment.filename |
|
| 346 |
existing_file_names << filename |
|
| 347 |
# If a file with the same name already exists, change the file name. |
|
| 348 |
if existing_file_names.count(filename) > 1 |
|
| 349 |
filename = "#{File.basename(filename, ".*")}(#{existing_file_names.count(filename)})#{File.extname(filename)}"
|
|
| 350 |
end |
|
| 351 |
zip.add(filename, attachment.diskfile) |
|
| 352 |
end |
|
| 353 |
end |
|
| 354 |
temp_file |
|
| 355 |
end |
|
| 356 | ||
| 336 | 357 |
# Moves an existing attachment to its target directory |
| 337 | 358 |
def move_to_target_directory! |
| 338 | 359 |
return unless !new_record? & readable? |
| app/views/attachments/_links.html.erb | ||
|---|---|---|
| 5 | 5 |
:title => l(:label_edit_attachments), |
| 6 | 6 |
:class => 'icon-only icon-edit' |
| 7 | 7 |
) if options[:editable] %> |
| 8 |
<%= link_to(l(:label_download_attachments), |
|
| 9 |
container_attachments_download_path(container), |
|
| 10 |
:title => l(:label_download_attachments), |
|
| 11 |
:class => 'icon-only icon-download' |
|
| 12 |
) %> |
|
| 8 | 13 |
</div> |
| 9 | 14 |
<table> |
| 10 | 15 |
<% for attachment in attachments %> |
| config/locales/en.yml | ||
|---|---|---|
| 988 | 988 |
label_users_visibility_all: All active users |
| 989 | 989 |
label_users_visibility_members_of_visible_projects: Members of visible projects |
| 990 | 990 |
label_edit_attachments: Edit attached files |
| 991 |
label_download_attachments: Download attached files |
|
| 991 | 992 |
label_link_copied_issue: Link copied issue |
| 992 | 993 |
label_ask: Ask |
| 993 | 994 |
label_search_attachments_yes: Search attachment filenames and descriptions |
| config/routes.rb | ||
|---|---|---|
| 275 | 275 |
resources :attachments, :only => [:show, :update, :destroy] |
| 276 | 276 |
get 'attachments/:object_type/:object_id/edit', :to => 'attachments#edit_all', :as => :object_attachments_edit |
| 277 | 277 |
patch 'attachments/:object_type/:object_id', :to => 'attachments#update_all', :as => :object_attachments |
| 278 |
get 'attachments/:object_type/:object_id/download', :to => 'attachments#download_all', :as => :object_attachments_download |
|
| 278 | 279 | |
| 279 | 280 |
resources :groups do |
| 280 | 281 |
resources :memberships, :controller => 'principal_memberships' |
| test/functional/attachments_controller_test.rb | ||
|---|---|---|
| 540 | 540 |
assert_equal 'This is a Ruby source file', attachment.description |
| 541 | 541 |
end |
| 542 | 542 | |
| 543 |
def test_download_all_with_valid_container |
|
| 544 |
@request.session[:user_id] = 2 |
|
| 545 |
# Check to delete tempfile |
|
| 546 |
Tempfile.any_instance.expects(:close) |
|
| 547 |
Tempfile.any_instance.expects(:unlink) |
|
| 548 | ||
| 549 |
get :download_all, :params => {
|
|
| 550 |
:object_type => 'issues', |
|
| 551 |
:object_id => '2' |
|
| 552 |
} |
|
| 553 |
assert_response 200 |
|
| 554 |
assert_equal response.headers['Content-Type'], 'application/zip' |
|
| 555 |
assert_match (/issue-2-attachments.zip/), response.headers['Content-Disposition'] |
|
| 556 |
end |
|
| 557 | ||
| 558 |
def test_download_all_with_invalid_container |
|
| 559 |
@request.session[:user_id] = 2 |
|
| 560 |
get :download_all, :params => {
|
|
| 561 |
:object_type => 'issues', |
|
| 562 |
:object_id => '999' |
|
| 563 |
} |
|
| 564 |
assert_response 404 |
|
| 565 |
end |
|
| 566 | ||
| 543 | 567 |
def test_destroy_issue_attachment |
| 544 | 568 |
set_tmp_attachments_directory |
| 545 | 569 |
issue = Issue.find(3) |
| test/unit/attachment_test.rb | ||
|---|---|---|
| 277 | 277 |
end |
| 278 | 278 |
end |
| 279 | 279 | |
| 280 |
def test_attachments_to_zip_with_attachments |
|
| 281 |
attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1)
|
|
| 282 |
tempfile = Attachment.attachments_to_zip([attachment]) |
|
| 283 |
assert_instance_of Tempfile, tempfile |
|
| 284 |
end |
|
| 285 | ||
| 286 |
def test_attachments_to_zip_without_attachments |
|
| 287 |
tempfile = Attachment.attachments_to_zip([]) |
|
| 288 |
assert_nil tempfile |
|
| 289 |
end |
|
| 290 | ||
| 291 |
def test_attachments_to_zip_should_not_duplicate_file_names |
|
| 292 |
attachment_1 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1)
|
|
| 293 |
attachment_2 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1)
|
|
| 294 |
zip_file = Attachment.attachments_to_zip([attachment_1, attachment_2]) |
|
| 295 |
zip_file_names = ['testfile.txt', 'testfile(2).txt'] |
|
| 296 | ||
| 297 |
Zip::File.open(zip_file.path) do |zip_file| |
|
| 298 |
zip_file.each_with_index do |entry, i| |
|
| 299 |
assert_includes zip_file_names[i], entry.name |
|
| 300 |
end |
|
| 301 |
end |
|
| 302 |
end |
|
| 303 | ||
| 280 | 304 |
def test_move_from_root_to_target_directory_should_move_root_files |
| 281 | 305 |
a = Attachment.find(20) |
| 282 | 306 |
assert a.disk_directory.blank? |
