From 553ac4c36011827d9f9ce639d15d2dabe0527629 Mon Sep 17 00:00:00 2001 From: Jure Grabnar Date: Sun, 30 Sep 2018 16:37:18 +0200 Subject: [PATCH] Refactor code according to PR review PR review: https://github.com/biosistemika/scinote-web/pull/1304 Closes: [SCI-2640], [SCI-2642], [SCI-2643], [SCI-2644]. --- app/models/team_zip_export.rb | 106 ++++++++------------ app/models/zip_export.rb | 1 - app/services/team_zip_exporter.rb | 2 +- app/views/user_notifications/_list.html.erb | 2 +- 4 files changed, 46 insertions(+), 65 deletions(-) diff --git a/app/models/team_zip_export.rb b/app/models/team_zip_export.rb index dad6903cd..c702b4b11 100644 --- a/app/models/team_zip_export.rb +++ b/app/models/team_zip_export.rb @@ -20,7 +20,7 @@ class TeamZipExport < ZipExport ).first output_file = File.new( File.join(Rails.root, - "tmp/zip-ready/projects_export-timestamp-#{Time.now.to_i}.zip"), + "tmp/zip-ready/projects-export-#{Time.now.to_i}.zip"), 'w+' ) fill_content(dir_to_zip, data, type, options) @@ -56,7 +56,7 @@ class TeamZipExport < ZipExport root += "/#{project_name}" FileUtils.mkdir_p(root) - FileUtils.touch("#{root}/#{project_name}_REPORT.pdf").first + FileUtils.touch("#{root}/#{project_name}_Report.pdf").first inventories = "#{root}/Inventories" FileUtils.mkdir_p(inventories) @@ -68,9 +68,9 @@ class TeamZipExport < ZipExport .distinct # Iterate through every inventory repo and save it to CSV - repo_rows.map(&:repository).uniq.each_with_index do |repo, repo_ind| + repo_rows.map(&:repository).uniq.each_with_index do |repo, repo_idx| curr_repo_rows = repo_rows.select { |x| x.repository_id == repo.id } - save_inventories_to_csv(inventories, repo, curr_repo_rows, repo_ind) + save_inventories_to_csv(inventories, repo, curr_repo_rows, repo_idx) end # Include all experiments @@ -92,14 +92,14 @@ class TeamZipExport < ZipExport # Export protocols steps = my_module.protocols.map(&:steps).flatten - export_step_assets(StepAsset.where(step: steps), protocol_path) - export_step_tables(StepTable.where(step: steps), protocol_path) + export_assets(StepAsset.where(step: steps), :step, protocol_path) + export_tables(StepTable.where(step: steps), :step, protocol_path) # Export results - export_result_assets(ResultAsset.where(result: my_module.results) - .map(&:asset), result_path) - export_result_tables(ResultTable.where(result: my_module.results) - .map(&:table), result_path) + export_assets(ResultAsset.where(result: my_module.results), + :result, result_path) + export_tables(ResultTable.where(result: my_module.results), + :result, result_path) end end end @@ -124,7 +124,7 @@ class TeamZipExport < ZipExport if name == '..' return '__' elsif name == '.' - return '.' + return '_' end # Truncate and replace reserved characters @@ -139,54 +139,42 @@ class TeamZipExport < ZipExport end # Appends given suffix to file_name and then adds original extension - def append_suffix(file_name, suffix) + def append_file_suffix(file_name, suffix) ext = File.extname(file_name) File.basename(file_name, ext) + suffix + ext end # Helper method to extract given assets to the directory - def export_result_assets(assets, directory) - assets.each_with_index do |asset, i| - file = FileUtils.touch("#{directory}/#{append_suffix(asset.file_file_name, - "_#{i}")}").first - File.open(file, 'wb') { |f| f.write(asset.open.read) } - end - end - - # Helper method to extract given step assets to the directory - def export_step_assets(assets, directory) - assets.each_with_index do |step_asset, i| - asset = step_asset.asset - file = FileUtils.touch( - "#{directory}/" \ - "#{append_suffix(asset.file_file_name, - "_#{i}_Step#{step_asset.step.position + 1}")}" - ).first + def export_assets(elements, type, directory) + elements.each_with_index do |element, i| + asset = element.asset + if type == :step + name = "#{directory}/" \ + "#{append_file_suffix(asset.file_file_name, + "_#{i}_Step#{element.step.position + 1}")}" + elsif type == :result + name = "#{directory}/#{append_file_suffix(asset.file_file_name, + "_#{i}")}" + end + file = FileUtils.touch(name).first File.open(file, 'wb') { |f| f.write(asset.open.read) } end end # Helper method to extract given tables to the directory - def export_step_tables(step_tables, directory) - step_tables.each_with_index do |step_table, i| - table = step_table.table + def export_tables(elements, type, directory) + elements.each_with_index do |element, i| + table = element.table table_name = table.name.presence || 'Table' table_name += i.to_s - file = FileUtils.touch( - "#{directory}/#{handle_name(table_name)}" \ - "_#{i}_Step#{step_table.step.position + 1}.csv" - ).first - File.open(file, 'wb') { |f| f.write(table.to_csv) } - end - end - # Helper method to extract given tables to the directory - def export_result_tables(tables, directory) - tables.each_with_index do |table, i| - table_name = table.name.presence || 'Table' - table_name += i.to_s - file = FileUtils.touch("#{directory}/#{handle_name(table_name)}.csv") - .first + if type == :step + name = "#{directory}/#{handle_name(table_name)}" \ + "_#{i}_Step#{element.step.position + 1}.csv" + elsif type == :result + name = "#{directory}/#{handle_name(table_name)}.csv" + end + file = FileUtils.touch(name).first File.open(file, 'wb') { |f| f.write(table.to_csv) } end end @@ -197,7 +185,7 @@ class TeamZipExport < ZipExport file = FileUtils.touch("#{path}/#{repo_name}.csv").first # Attachment folder - rel_attach_path = "#{repo_name}_ATTACHMENTS" + rel_attach_path = "#{repo_name}_attachments" attach_path = "#{path}/#{rel_attach_path}" FileUtils.mkdir_p(attach_path) @@ -208,7 +196,8 @@ class TeamZipExport < ZipExport assets = {} asset_counter = 0 handle_name_func = lambda do |asset| - file_name = append_suffix(asset.file_file_name, "_#{asset_counter}").to_s + file_name = append_file_suffix(asset.file_file_name, + "_#{asset_counter}").to_s # Save pair for downloading it later assets[asset] = "#{attach_path}/#{file_name}" @@ -233,23 +222,15 @@ class TeamZipExport < ZipExport # Recursive zipping def zip!(input_dir, output_file) files = Dir.entries(input_dir) - files.delete_if { |el| el == '..' || el == '.' } + + # Don't zip current/above directory + files.delete_if { |el| ['.', '..'].include?(el) } + Zip::File.open(output_file, Zip::File::CREATE) do |zipfile| write_entries(input_dir, files, '', zipfile) end end - # Zip the input directory. - def write - entries = Dir.entries(@inputDir) - entries.delete('.') - entries.delete('..') - - io = Zip::File.open(@outputFile, Zip::File::CREATE) - write_entries(entries, '', io) - io.close - end - # A helper method to make the recursion work. def write_entries(input_dir, entries, path, io) entries.each do |e| @@ -259,8 +240,9 @@ class TeamZipExport < ZipExport if File.directory?(disk_file_path) io.mkdir(zip_file_path) subdir = Dir.entries(disk_file_path) - subdir.delete('.') - subdir.delete('..') + + # Remove current/above directory to prevent infinite recursion + subdir.delete_if { |el| ['.', '..'].include?(el) } write_entries(input_dir, subdir, zip_file_path, io) else diff --git a/app/models/zip_export.rb b/app/models/zip_export.rb index 6e8fbb84a..eddba2617 100644 --- a/app/models/zip_export.rb +++ b/app/models/zip_export.rb @@ -63,7 +63,6 @@ class ZipExport < ApplicationRecord handle_asynchronously :generate_exportable_zip - # A helper method to make the recursion work. private def method_missing(m, *args, &block) diff --git a/app/services/team_zip_exporter.rb b/app/services/team_zip_exporter.rb index 17cec0143..009a70727 100644 --- a/app/services/team_zip_exporter.rb +++ b/app/services/team_zip_exporter.rb @@ -1,6 +1,6 @@ module TeamZipExporter def self.generate_zip(params, team, current_user) - Rails.logger.warn('Exporting team zip') + Rails.logger.info('Exporting team zip') project_ids = params[:project_ids] ids = Project.where(id: project_ids, diff --git a/app/views/user_notifications/_list.html.erb b/app/views/user_notifications/_list.html.erb index a9e6a40e2..2a0ade8c0 100644 --- a/app/views/user_notifications/_list.html.erb +++ b/app/views/user_notifications/_list.html.erb @@ -10,7 +10,7 @@
<%= sanitize_input(notification.title) %>
<% if notification.deliver? %> - Click the link to download the file.
+ Click the link to download the file.
<% end %> <%= l(notification.created_at, format: :full) %> | <%= sanitize_input(notification.message) %>