Redmine

In order to realize this feature, I thought of compressing and downloading Issue's attachment.
By applying this patch, you can download all of the attached files as a zip file. (using gem rubyzip)

Non-ascii file names are garbled in older versions (Windows 7, Windows Vista, Windows XP).
Windows 7 is scheduled to end support in 2020.
https://github.com/rubyzip/rubyzip/wiki/Files-with-non-ascii-filenames

Any feedback on this patch is welcome.

Go MAEDA wrote:

Thank you for posting the patch. Could you explain why attachments_to_zip method creates temporary files in Attachment.storage_path instead of tmp directory?

There was no particular reason. I changed the temporary file storage location to the tmp directory.

Go MAEDA wrote:

I think the patch should consider the free space of the disk. Suppose that the total size of attachments of the issue is 1GB. If the free space of the disk is less than 1GB, the creation of a zip archive will fail and the server may stop working due to disk full.

Here are some ideas to avoid the problem.

• Show an error message indicates no sufficient disk space if the total size of attachments exceeds the free disk space
• Create and send a zip archive without creating a temporary file

Thank you for your feedback.

Because I could not think of a good way to check disk space, I thought about limiting the maximum capacity of temporary files.

The maximum capacity of temporary file can be set in configration.yml.
By doing this setting, it is impossible to create a temporary file larger than the assumption of the server administrator.

I modified the patch to apply to the latest trunk.

I have changed the patch:

• Add a label to the bulk download button
• Move the button to below the attachments list (like usability plugin)
• Updated messages
• Raised the default value for bulk_download_max_size to 512MB

I have updated the patch. Moved bulk_download_max_size setting from configuration.yml to /settings GUI for consistency.

Setting the target version to 4.2.0.

Slightly updated the patch:

• Changed the method name Attachment.attachments_to_zip to Attachment.archive_attachments because attachments_to_zip is too specific. No one knows if the feature uses ZIP format forever, so I think the method name should not depend on a specific archive format
• Changed the text for label_download_all_attachments from "Download all attached files" to simpler "Download all files"

The patch may raise Zip::EntryExistsError when you try to download a ZIP file if a file that has "(2)" suffix in its basename is attached.

Started GET "/attachments/issues/32/download" for 127.0.0.1 at 2020-03-15 10:59:03 +0900
Processing by AttachmentsController#download_all as HTML
.
.
.
Completed 500 Internal Server Error in 28ms (ActiveRecord: 4.9ms)

Zip::EntryExistsError (add failed. Entry testfile(2).txt already exists):

app/models/attachment.rb:362:in `block (2 levels) in archive_attachments'
app/models/attachment.rb:355:in `each'
app/models/attachment.rb:355:in `block in archive_attachments'
app/models/attachment.rb:354:in `archive_attachments'
app/controllers/attachments_controller.rb:139:in `block in download_all'
app/controllers/attachments_controller.rb:138:in `download_all'
lib/redmine/sudo_mode.rb:64:in `sudo_mode'

I have fixed the above issue. Also, I have changed the "(n)" suffix for duplicate filenames to start from 1 instead of 2. It is the same behavior as Chrome and Firefox do when downloading files with the same name.

I like this feature.

What do you think if we show the link next to edit attached files icon (as Mizuki proposed first time) in order to keep the links grouped?

I don't find very UX friendly to have the Edit button to the right and the "Download all" button under attachments and thumbnails.

Thanks for contributions for releasing this feature.

• Add tests to test/integration/routing/attachments_test.rb
• Move icon position to original position

Committed the patch. Thank you for improving Redmine.

The attached patch human-readable-file-size.patch fixes the format of file size in the message error_bulk_download_size_too_big to human-readable.

Before:

After:

Go MAEDA wrote:

The attached patch human-readable-file-size.patch fixes the format of file size in the message error_bulk_download_size_too_big to human-readable.

Committed the patch in r19609.

unfortunately, this feature doesn't work on Windows.

Error:
AttachmentTest#test_archive_attachments:
Errno::EACCES: Permission denied @ rb_file_s_rename - (c:/redmine/tmp/attachments_zip20200324-12432-yknlax20200324-12432-1qjlddf, c:/redmine/tmp/attachments_zip20200324-12432-yknlax)
    app/models/attachment.rb:355:in `archive_attachments'
    test/unit/attachment_test.rb:284:in `block in test_archive_attachments'
    test/unit/attachment_test.rb:283:in `test_archive_attachments'

I've attached a quick fix.

Pavel Rosický wrote:

unfortunately, this feature doesn't work on Windows.

[...]

Thanks for reporting this problem.
I fixed it to create a zip file via stream. No temporary file is needed.

diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index b69dcf983..0c76490b2 100644
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -137,16 +137,16 @@ class AttachmentsController < ApplicationController
   end

   def download_all
-    Tempfile.create('attachments_zip-', Rails.root.join('tmp')) do |tempfile|
-      zip_file = Attachment.archive_attachments(tempfile, @attachments)
-      if zip_file
-        send_data(
-          File.read(zip_file.path),
-          :type => 'application/zip',
-          :filename => "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip")
-      else
-        render_404
-      end
+    zip_data = Attachment.archive_attachments(@attachments)
+    if zip_data
+      file_name = "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip" 
+      send_data(
+        zip_data,
+        :type => Redmine::MimeType.of(file_name),
+        :filename => file_name
+      )
+    else
+      render_404
     end
   end

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index b5a3332ac..734b8f7fa 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -346,28 +346,28 @@ class Attachment < ActiveRecord::Base
     Attachment.where("created_on < ? AND (container_type IS NULL OR container_type = '')", Time.now - age).destroy_all
   end

-  def self.archive_attachments(out_file, attachments)
+  def self.archive_attachments(attachments)
     attachments = attachments.select(&:readable?)
     return nil if attachments.blank?

     Zip.unicode_names = true
     archived_file_names = []
-    Zip::File.open(out_file.path, Zip::File::CREATE) do |zip|
+    Zip::OutputStream.write_buffer do |zos|
       attachments.each do |attachment|
         filename = attachment.filename
         # rename the file if a file with the same name already exists
         dup_count = 0
         while archived_file_names.include?(filename)
           dup_count += 1
-          basename = File.basename(attachment.filename, '.*')
           extname = File.extname(attachment.filename)
+          basename = File.basename(attachment.filename, extname)
           filename = "#{basename}(#{dup_count})#{extname}" 
         end
-        zip.add(filename, attachment.diskfile)
+        zos.put_next_entry(filename)
+        zos << IO.binread(attachment.diskfile)
         archived_file_names << filename
       end
-    end
-    out_file
+    end.string
   end

   # Moves an existing attachment to its target directory

Pavel Rosický wrote:

Yuichi HARADA thanks, it looks good!

but I'm getting a warning
[...]

the output StreamIO should be closed

Thanks for pointing it out.
I had forgotten to perform StringIO#close . Fixed a patch.

Pavel Rosický wrote:

unfortunately, this feature doesn't work on Windows.

[...]

I've attached a quick fix.

Committed a fix for the issue in r19688. Thank you.

Grubs _ wrote in #note-40:

v5.0.5.stable and the download all files icon has some issues where the mouse hover over "flashes" and the shadow box with the text "Download all files" does not always display. If the mouse cursor is hovered over the download icon the cursor alternates between an arrow and hand at a cycle of about 1 per second.

This issue is showing in "Documents", when there is more than one file attached to the document.

If the user clicks on the download all button it does work most times, but there is often no feedback to the user to indicate that their click on the download arrow has registered. (particularly if there is a large number of files attached to the one Document)

This may be a jquerey issue with the css that is attempting to display the shadow box rather than a redmine issue. I don't know.

The function of the "download all" does work - but the user feedback seems off and users are spamming the download all button because they don't know their click has registered.

Tested with Firefox 116.02 and Edge 115.0.1901.188 (on Windows 10 22H2)

P.S great job on this BTW. brilliant. Apologies for re-opening.

I was able to reproduce this problem.
When the cursor is focused on an icon that exists on the right-hand side and tries to display a tooltip, the tooltip content breaks lines because it is at the edge of the screen.
This causes the cursor to focus on the tooltip element that overlaps the icon and the tooltip, which is only visible when hovering over the icon, disappears. After the tooltip disappears, the tooltip appears again because of the focus on the icon, disappears, and so on.
This is the likely cause of the blinking.

This problem is not limited to download links, but can occur with any icon that exists at the edge of the screen.

Many thanks for the insight.

I fixed this by changing the CSS of my theme application.css to display the previously hidden link text "Download all files" which pushes the download icon away from the left edge of the screen and fixes the flashing.

The Download All Icon now takes up more width and the popup is now redundant but users can find it much easier. The small icon was not obvious on the right edge of the screen. I also changed the icon div container CSS so the file list begins below the download all link allowing the file names and descriptions to use the full width of the screen.

/* puts the edit and Download all icon on its own line */
div.attachments .contextual {
  float: unset;
  text-align: right;
}

/* this displays the "Download all files" link text */
div.contextual .icon-download {
  width: unset;
  font-size: 13px;
  }