diff --git a/.rubocop.yml b/.rubocop.yml index b94e63629..af6fc7333 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,7 +5,7 @@ AllCops: - "vendor/**/*" - "db/schema.rb" UseCache: false - TargetRubyVersion: 2.5 + TargetRubyVersion: 2.6 ##################### Style #################################### diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 00c5e918a..60caa59a6 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -63,7 +63,7 @@ class AssetsController < ApplicationController def file_preview response_json = { 'id' => @asset.id, - 'type' => (@asset.is_image? ? 'image' : 'file'), + 'type' => (@asset.image? ? 'image' : 'file'), 'filename' => truncate(escape_input(@asset.file_file_name), length: Constants::FILENAME_TRUNCATION_LENGTH), @@ -78,7 +78,7 @@ class AssetsController < ApplicationController can_manage_repository_rows?(@repository.team) end - if @asset.is_image? + if @asset.image? if ['image/jpeg', 'image/pjpeg'].include? @asset.file.content_type response_json['quality'] = @asset.file_image_quality || 90 end @@ -322,7 +322,7 @@ class AssetsController < ApplicationController def asset_data_type(asset) return 'wopi' if wopi_file?(asset) - return 'image' if asset.is_image? + return 'image' if asset.image? 'file' end diff --git a/app/models/asset.rb b/app/models/asset.rb index ff78b324e..77470189a 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -203,16 +203,30 @@ class Asset < ApplicationRecord end end + def previewable? + previewable_document? || previewable_image? + end + def medium_preview - file.variant(resize: Constants::MEDIUM_PIC_FORMAT) + return file.variant(resize: Constants::MEDIUM_PIC_FORMAT) if previewable_image? + + 'medium/processing.gif' + # file.preview(resize: Constants::MEDIUM_PIC_FORMAT) end def large_preview - file.variant(resize: Constants::LARGE_PIC_FORMAT) + return file.variant(resize: Constants::LARGE_PIC_FORMAT) if previewable_image? + + 'large/processing.gif' + # file.preview(resize: Constants::LARGE_PIC_FORMAT) + end + + def file_name + file.blob&.filename&.to_s end def file_size - file.blob.byte_size + file.blob&.byte_size end def extract_image_quality @@ -227,13 +241,8 @@ class Asset < ApplicationRecord Rails.logger.info "There was an error extracting image quality - #{e}" end - def previewable? - file.previewable_image? || file.previewable_document? - end - - def is_image? - %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES)}} === - file.content_type + def image? + file.blob.content_type == %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES)}} end def text? @@ -520,6 +529,29 @@ class Asset < ApplicationRecord private + def previewable_document? + previewable = Constants::PREVIEWABLE_FILE_TYPES.include?(file.blob&.content_type) + + filename = file.blob&.filename + content_type = file.blob&.content_type + + extensions = %w(.xlsx .docx .pptx .xls .doc .ppt) + # Mimetype sometimes recognizes Office files as zip files + # In this case we also check the extension of the given file + # Otherwise the conversion should fail if the file is being something else + previewable ||= (content_type == 'application/zip' && extensions.include?(File.extname(filename))) + + # Mimetype also sometimes recognizes '.xls' and '.ppt' files as + # application/x-ole-storage (https://github.com/minad/mimemagic/issues/50) + previewable ||= (content_type == 'application/x-ole-storage' && %w(.xls .ppt).include?(File.extname(filename))) + + previewable + end + + def previewable_image? + file.blob&.content_type =~ %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES)}} + end + def filter_paperclip_errors if errors.size > 1 temp_errors = errors[:file] diff --git a/app/models/protocol.rb b/app/models/protocol.rb index 2e857e14a..add6f26d4 100644 --- a/app/models/protocol.rb +++ b/app/models/protocol.rb @@ -236,7 +236,7 @@ class Protocol < ApplicationRecord dest.file = src.file dest.save! - if dest.is_image? + if dest.image? dest.file.reprocess!(:large) dest.file.reprocess!(:medium) end @@ -325,7 +325,7 @@ class Protocol < ApplicationRecord asset2.created_by = current_user asset2.team = dest.team asset2.last_modified_by = current_user - asset2.file_processing = true if asset.is_image? + asset2.file_processing = true if asset.image? asset2.save! step2.assets << asset2 diff --git a/app/services/repository_actions/duplicate_cell.rb b/app/services/repository_actions/duplicate_cell.rb index c6a8478dc..554638226 100644 --- a/app/services/repository_actions/duplicate_cell.rb +++ b/app/services/repository_actions/duplicate_cell.rb @@ -77,13 +77,13 @@ module RepositoryActions new_asset.created_by = old_asset.created_by new_asset.team = @team new_asset.last_modified_by = @user - new_asset.file_processing = true if old_asset.is_image? + new_asset.file_processing = true if old_asset.image? new_asset.file = old_asset.file new_asset.save return unless new_asset.valid? - if new_asset.is_image? + if new_asset.image? new_asset.file.reprocess!(:large) new_asset.file.reprocess!(:medium) end diff --git a/app/views/my_modules/archive/_result.html.erb b/app/views/my_modules/archive/_result.html.erb index 51b70b1da..64cc1d1aa 100644 --- a/app/views/my_modules/archive/_result.html.erb +++ b/app/views/my_modules/archive/_result.html.erb @@ -36,7 +36,7 @@

<% if result.is_asset %> - <% if result.asset.is_image? %> + <% if result.asset.image? %> <% else %> diff --git a/app/views/reports/elements/_my_module_result_asset_element.html.erb b/app/views/reports/elements/_my_module_result_asset_element.html.erb index ca24d4f9a..3cbb13b6e 100644 --- a/app/views/reports/elements/_my_module_result_asset_element.html.erb +++ b/app/views/reports/elements/_my_module_result_asset_element.html.erb @@ -1,6 +1,6 @@ <% result ||= @result %> <% asset = result.asset %> -<% is_image = result.asset.is_image? %> +<% is_image = result.asset.image? %> <% comments = result.result_comments %> <% timestamp = asset.created_at %> <% icon_class = 'fas ' + (is_image ? 'fa-image' : 'fa-file') %> diff --git a/app/views/reports/elements/_step_asset_element.html.erb b/app/views/reports/elements/_step_asset_element.html.erb index f9bbcde53..b84d9a61b 100644 --- a/app/views/reports/elements/_step_asset_element.html.erb +++ b/app/views/reports/elements/_step_asset_element.html.erb @@ -1,5 +1,5 @@ <% asset ||= @asset %> -<% is_image = asset.is_image? %> +<% is_image = asset.image? %> <% timestamp = asset.created_at %> <% icon_class = 'fas ' + (is_image ? 'fa-image' : 'fa-file') %>
" data-icon-class="<%= icon_class %>"> diff --git a/app/views/search/results/_assets.html.erb b/app/views/search/results/_assets.html.erb index 73a884c4e..68b42a216 100644 --- a/app/views/search/results/_assets.html.erb +++ b/app/views/search/results/_assets.html.erb @@ -1,6 +1,6 @@ <% @asset_results.each do |asset| %>
- <% if asset.is_image? %> + <% if asset.image? %> <% else %> <% if wopi_file?(asset) %> diff --git a/app/views/search/results/_results.html.erb b/app/views/search/results/_results.html.erb index 4f2849096..a062ba182 100644 --- a/app/views/search/results/_results.html.erb +++ b/app/views/search/results/_results.html.erb @@ -5,7 +5,7 @@ <% elsif result.is_table %> <% else %> - <% if result.asset.is_image? %> + <% if result.asset.image? %> <% else %> diff --git a/app/views/shared/_asset_link.html.erb b/app/views/shared/_asset_link.html.erb index 7dbd2482f..f214ed94f 100644 --- a/app/views/shared/_asset_link.html.erb +++ b/app/views/shared/_asset_link.html.erb @@ -1,37 +1,19 @@ -<% if asset.file_present %> - <% if asset.file.processing? && display_image_tag && asset.is_image? %> - - <%= image_tag 'medium/processing.gif' %> - <%= link_to download_asset_path(asset), - class: 'file-preview-link', - id: "modal_link#{asset.id}", - data: { no_turbolink: true, id: true, status: 'asset-present', 'preview-url': asset_file_preview_path(asset) } do %> - <%= truncate(asset.file_file_name, - length: Constants::FILENAME_TRUNCATION_LENGTH) %> - <% end %> - - <% else %> - <%= link_to download_asset_path(asset), - class: 'file-preview-link', - id: "modal_link#{asset.id}", - data: { no_turbolink: true, id: true, status: 'asset-present', 'preview-url': asset_file_preview_path(asset) } do %> - <% if asset.is_image? && display_image_tag %> - <%= image_tag asset.url(:medium) %> - <% end %> - <% if display_image_tag %> -

- <%= truncate(asset.file_file_name, - length: Constants::FILENAME_TRUNCATION_LENGTH) %> -

- <% else %> - - <%= truncate(asset.file_file_name, - length: Constants::FILENAME_TRUNCATION_LENGTH) %> - - <% end %> - <% end %> +<%= link_to download_asset_path(asset), + class: 'file-preview-link', + id: "modal_link#{asset.id}", + data: { no_turbolink: true, id: true, status: 'asset-present', 'preview-url': asset_file_preview_path(asset) } do %> + <% if asset.image? && display_image_tag %> + <%= image_tag asset.medium_preview %> + <% end %> + <% if display_image_tag %> +

+ <%= truncate(asset.file_file_name, + length: Constants::FILENAME_TRUNCATION_LENGTH) %> +

+ <% else %> + + <%= truncate(asset.file_file_name, + length: Constants::FILENAME_TRUNCATION_LENGTH) %> + <% end %> -<% else %> - <%= image_tag 'medium/processing.gif' %> <% end %> diff --git a/app/views/shared/_file_preview_icon.html.erb b/app/views/shared/_file_preview_icon.html.erb index a37b5e60e..45c362022 100644 --- a/app/views/shared/_file_preview_icon.html.erb +++ b/app/views/shared/_file_preview_icon.html.erb @@ -1,6 +1,6 @@
<% if asset.file.previewable_document? %> - <%= image_tag(asset.url(:large)) %> + <%= image_tag(asset.large_preview) %> <% else %> <% end %> diff --git a/app/views/steps/attachments/_item.html.erb b/app/views/steps/attachments/_item.html.erb index 08c1be4b3..679f4dafa 100644 --- a/app/views/steps/attachments/_item.html.erb +++ b/app/views/steps/attachments/_item.html.erb @@ -1,19 +1,9 @@ -<% if asset.file.processing? && (asset.is_image? || asset.file.previewable_document?) %> - <% asset_status = 'asset-loading' %> - <% present_url = step_file_present_asset_path(asset.id) %> -<% else %> - <% asset_status = 'asset-present' %> - <% present_url = '' %> -<% end %> -
<%= link_to download_asset_path(asset), class: 'file-preview-link', id: "modal_link#{asset.id}", data: { no_turbolink: true, id: true, - status: asset_status, - 'present-url': present_url, 'preview-url': asset_file_preview_path(asset), 'order-atoz': az_ordered_assets_index(step, asset.id), 'order-ztoa': assets_count - az_ordered_assets_index(step, asset.id), diff --git a/app/views/steps/attachments/_placeholder.html.erb b/app/views/steps/attachments/_placeholder.html.erb index c5c079c52..a07e47d10 100644 --- a/app/views/steps/attachments/_placeholder.html.erb +++ b/app/views/steps/attachments/_placeholder.html.erb @@ -1,23 +1,22 @@ -<% image_preview = (asset.is_image? || asset.file.previewable_document?) ? asset.url(:medium) : nil %> -
-
- <% if image_preview %> - <%= image_tag image_preview %> +
+ <% if asset.previewable? %> + <%= image_tag asset.medium_preview %> <% else %> - + <% end %>
-
<%= truncate(asset.file_file_name, - length: Constants::FILENAME_TRUNCATION_LENGTH) %>
+
+ <%= truncate(asset.file_name, length: Constants::FILENAME_TRUNCATION_LENGTH) %> +
<%= t('protocols.steps.attachments.modified_label') %> <%= l(asset.updated_at, format: :full_date) if asset.updated_at %>
- <%= number_to_human_size(asset.file.size) %> + <%= number_to_human_size(asset.file_size) %>
<% if edit_page %>
- <% unless ff.object.file.exists? && ff.object.locked? %> + <% unless ff.object.file.attached? && ff.object.locked? %> <%= ff.remove_nested_fields_link do %> <% end %> diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index 7ac23bbe9..f032d32ca 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -61,27 +61,6 @@ Paperclip::Attachment.class_eval do def fetch Paperclip.io_adapters.for self end - - def previewable_document? - previewable = Constants::PREVIEWABLE_FILE_TYPES.include?(content_type) - - extensions = %w(.xlsx .docx .pptx .xls .doc .ppt) - # Mimetype sometimes recognizes Office files as zip files - # In this case we also check the extension of the given file - # Otherwise the conversion should fail if the file is being something else - previewable ||= (content_type == 'application/zip' && extensions.include?(File.extname(original_filename))) - - # Mimetype also sometimes recognizes '.xls' and '.ppt' files as - # application/x-ole-storage (https://github.com/minad/mimemagic/issues/50) - previewable ||= - (content_type == 'application/x-ole-storage' && %w(.xls .ppt).include?(File.extname(original_filename))) - - previewable - end - - def previewable_image? - content_type =~ %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES)}} - end end module Paperclip diff --git a/db/migrate/20190613134100_convert_to_active_storage.rb b/db/migrate/20190613134100_convert_to_active_storage.rb index ea9de35b3..76593aa1b 100644 --- a/db/migrate/20190613134100_convert_to_active_storage.rb +++ b/db/migrate/20190613134100_convert_to_active_storage.rb @@ -41,7 +41,7 @@ class ConvertToActiveStorage < ActiveRecord::Migration[5.2] instance.__send__("#{attachment}_file_name"), instance.__send__("#{attachment}_content_type"), instance.__send__("#{attachment}_file_size") || 0, - checksum(instance.__send__(attachment)), + checksum(attachment), instance.updated_at.iso8601 ] ) @@ -96,12 +96,12 @@ class ConvertToActiveStorage < ActiveRecord::Migration[5.2] def key(instance, attachment) # SecureRandom.uuid # Alternatively: - pattern = if ENV['PAPERCLIP_STORAGE'] == 's3' - ':class/:attachment/:id_partition/:hash/original/:filename' - else - "#{Rails.root}/public/system/:class/:attachment/:id_partition/:hash/original/:filename" - end - interpolate(pattern, instance, attachment) + if ENV['PAPERCLIP_STORAGE'] == 's3' + interpolate(':class/:attachment/:id_partition/:hash/original/:filename', instance, attachment) + else + key = SecureRandom.uuid + File.join('storage', key.first(2), key.first(4).last(2)) + end end def checksum(_attachment) diff --git a/lib/tasks/paperclip.rake b/lib/tasks/paperclip.rake index eea765620..21c79a6e3 100644 --- a/lib/tasks/paperclip.rake +++ b/lib/tasks/paperclip.rake @@ -36,7 +36,7 @@ namespace :paperclip do assets = assets.where('updated_at < ?', eval(args[:before])) end assets.find_each(batch_size: 100) do |asset| - next unless asset.is_image? + next unless asset.image? begin asset.file.reprocess! :medium, :large rescue