From 4ea6b565825e70743563516b53d0782aa7149d72 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Mon, 1 Jul 2019 23:30:20 +0200 Subject: [PATCH] Fix Step and Result file previews [SCI-3539] --- app/assets/javascripts/assets.js | 102 ------------------ .../javascripts/my_modules/results.js.erb | 3 - app/assets/javascripts/protocols/steps.js.erb | 4 - .../javascripts/sitewide/file_preview.js | 4 +- app/controllers/assets_controller.rb | 102 ++++-------------- app/controllers/result_assets_controller.rb | 4 +- app/helpers/file_icons_helper.rb | 13 ++- app/models/asset.rb | 14 ++- app/views/shared/_asset_link.html.erb | 16 ++- app/views/shared/_file_preview_icon.html.erb | 2 +- config/initializers/assets.rb | 1 - 11 files changed, 46 insertions(+), 219 deletions(-) delete mode 100644 app/assets/javascripts/assets.js diff --git a/app/assets/javascripts/assets.js b/app/assets/javascripts/assets.js deleted file mode 100644 index 412cf7f60..000000000 --- a/app/assets/javascripts/assets.js +++ /dev/null @@ -1,102 +0,0 @@ -/* global animateSpinner FilePreviewModal */ - -function setupAssetsLoading() { - var DELAY = 2500; - var REPETITIONS = 60; - var cntr = 0; - var intervalId; - - function refreshAssets() { - var elements = $("[data-status='asset-loading']"); - - if (!elements.length) { - return false; - } - - $.each(elements, function(_, el) { - var $el = $(el); - - // Perform an AJAX call to present URL - // to check if file already exists - $.ajax({ - url: $el.data('present-url'), - type: 'GET', - dataType: 'json', - success: function(data) { - if (data.processing === true) { - return; - } - - if (data.processing === false) { - $el.html(data.placeholder_html); - $el.attr('data-status', 'asset-loaded'); - return; - } - - $el.attr('data-status', 'asset-loaded'); - $el.find('img').hide(); - $el.next().hide(); - $el.html(''); - - if (data.type === 'image') { - $el.html( - "" - + "

" + data.filename + '

' - ); - } else { - $el.html( - "

" + data.filename + '

' - ); - } - animateSpinner(null, false); - FilePreviewModal.init(); - }, - error: function(data) { - if (data.status === 403) { - $el.find('img').hide(); - $el.next().hide(); - // Image/file exists, but user doesn't have - // rights to download it - if (data.type === 'image') { - $el.html( - "

" + data.filename + '

' - ); - } else { - $el.html('

' + data.filename + '

'); - } - } else { - // Do nothing, file is not yet present - animateSpinner(null, false); - } - } - }); - }); - - return true; - } - - function finalizeAssets() { - var elements = $("[data-status='asset-loading']"); - - $.each(elements, function(_, el) { - var $el = $(el); - $el.attr('data-status', 'asset-failed'); - if ($el.data('filename')) { - $el.html($el.data('filename')); - } - }); - } - - intervalId = window.setInterval(function() { - cntr += 1; - if (cntr >= REPETITIONS || !refreshAssets()) { - finalizeAssets(); - window.clearInterval(intervalId); - } - }, DELAY); -} diff --git a/app/assets/javascripts/my_modules/results.js.erb b/app/assets/javascripts/my_modules/results.js.erb index 12d2c359e..890e4565f 100644 --- a/app/assets/javascripts/my_modules/results.js.erb +++ b/app/assets/javascripts/my_modules/results.js.erb @@ -1,5 +1,3 @@ -//= require assets - (function(global) { 'use strict'; @@ -111,7 +109,6 @@ }); _renderTable($(result).find('div.step-result-hot-table')); animateSpinner(null, false); - setupAssetsLoading(); } function _renderTable(table) { diff --git a/app/assets/javascripts/protocols/steps.js.erb b/app/assets/javascripts/protocols/steps.js.erb index 0336ac4a2..c5f23ca44 100644 --- a/app/assets/javascripts/protocols/steps.js.erb +++ b/app/assets/javascripts/protocols/steps.js.erb @@ -1,6 +1,5 @@ //= require Sortable.min //= require canvas-to-blob.min -//= require assets (function(global) { 'use strict'; @@ -246,7 +245,6 @@ $new_step.find("[data-role='step-hot-table']").each(function() { renderTable($(this)); }); - setupAssetsLoading(); }) .on("ajax:error", function(e, xhr, status, error) { $form.renderFormErrors('step', xhr.responseJSON, true, e); @@ -609,7 +607,6 @@ $(this).handsontable("render"); }); animateSpinner(null, false); - setupAssetsLoading(); DragNDropSteps.clearFiles(); FilePreviewModal.init(); $.initTooltips(); @@ -680,7 +677,6 @@ initCallBacks(); initHandsOnTable($(document)); expandAllSteps(); - setupAssetsLoading(); FilePreviewModal.init(); TinyMCE.highlight(); SmartAnnotation.preventPropagation('.atwho-user-popover'); diff --git a/app/assets/javascripts/sitewide/file_preview.js b/app/assets/javascripts/sitewide/file_preview.js index d65f68e09..da16d0c5b 100644 --- a/app/assets/javascripts/sitewide/file_preview.js +++ b/app/assets/javascripts/sitewide/file_preview.js @@ -1,8 +1,7 @@ /* eslint no-underscore-dangle: ["error", { "allowAfterThis": true }]*/ /* eslint no-use-before-define: ["error", { "functions": false }]*/ /* eslint-disable no-underscore-dangle */ -/* global Uint8Array fabric tui animateSpinner setupAssetsLoading I18n PerfectScrollbar*/ -//= require assets +/* global Uint8Array fabric tui animateSpinner I18n PerfectScrollbar*/ var FilePreviewModal = (function() { 'use strict'; @@ -396,7 +395,6 @@ var FilePreviewModal = (function() { processData: false, success: function(res) { $('#modal_link' + data.id).parent().html(res.html); - setupAssetsLoading(); } }).done(function() { animateSpinner(null, false); diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 60caa59a6..4f5aafab5 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -10,62 +10,15 @@ class AssetsController < ApplicationController include FileIconsHelper before_action :load_vars, except: :create_wopi_file - before_action :check_read_permission, except: :file_present + before_action :check_read_permission before_action :check_edit_permission, only: :edit - def file_present - respond_to do |format| - format.json do - if @asset.file.processing? - render json: {}, status: 404 - else - # Only if file is present, - # check_read_permission - check_read_permission - - # If check_read_permission already rendered error, - # stop execution - return if performed? - - # If check permission passes, return :ok - render json: { - 'asset-id' => @asset.id, - 'image-tag-url' => @asset.url(:medium), - 'preview-url' => asset_file_preview_path(@asset), - 'filename' => truncate(escape_input(@asset.file_file_name), - length: Constants::FILENAME_TRUNCATION_LENGTH), - 'download-url' => download_asset_path(@asset), - 'type' => asset_data_type(@asset) - }, status: 200 - end - end - end - end - - def step_file_present - respond_to do |format| - format.json do - if @asset.file.processing? - render json: { processing: true } - else - render json: { - placeholder_html: render_to_string( - partial: 'steps/attachments/placeholder.html.erb', - locals: { asset: @asset, edit_page: false } - ), - processing: false - } - end - end - end - end - def file_preview response_json = { 'id' => @asset.id, 'type' => (@asset.image? ? 'image' : 'file'), - 'filename' => truncate(escape_input(@asset.file_file_name), + 'filename' => truncate(escape_input(@asset.file_name), length: Constants::FILENAME_TRUNCATION_LENGTH), 'download-url' => download_asset_path(@asset, timestamp: Time.now.to_i) } @@ -85,18 +38,11 @@ class AssetsController < ApplicationController response_json.merge!( 'editable' => @asset.editable_image? && can_edit, 'mime-type' => @asset.file.content_type, - 'processing' => @asset.file.processing?, - 'large-preview-url' => @asset.url(:large), - 'processing-img' => image_tag('medium/processing.gif') + 'large-preview-url' => @asset.large_preview ) else - response_json.merge!( - 'processing' => @asset.file.processing?, - 'preview-icon' => render_to_string( - partial: 'shared/file_preview_icon.html.erb', - locals: { asset: @asset } - ) - ) + response_json['preview-icon'] = render_to_string(partial: 'shared/file_preview_icon.html.erb', + locals: { asset: @asset }) end if wopi_enabled? && wopi_file?(@asset) @@ -120,7 +66,7 @@ class AssetsController < ApplicationController # Check whether the wopi file can be edited and return appropriate response def wopi_file_edit_button_status - file_ext = @asset.file_file_name.split('.').last + file_ext = @asset.file_name.split('.').last if Constants::WOPI_EDITABLE_FORMATS.include?(file_ext) edit_supported = true title = '' @@ -138,20 +84,16 @@ class AssetsController < ApplicationController end def download - if !@asset.file_present - render_404 and return - elsif @asset.file.is_stored_on_s3? - redirect_to @asset.presigned_url(download: true), status: 307 + if !@asset.file.attached? + render_404 else - send_file @asset.file.path, filename: URI.unescape(@asset.file_file_name), - type: @asset.file_content_type + redirect_to rails_blob_path(@asset.file, disposition: 'attachment') end end def edit - action = @asset.file_file_size.zero? && !@asset.locked? ? 'editnew' : 'edit' - @action_url = append_wd_params(@asset - .get_action_url(current_user, action, false)) + action = @asset.file_size.zero? && !@asset.locked? ? 'editnew' : 'edit' + @action_url = append_wd_params(@asset.get_action_url(current_user, action, false)) @favicon_url = @asset.favicon_url('edit') tkn = current_user.get_wopi_token @token = tkn.token @@ -164,8 +106,7 @@ class AssetsController < ApplicationController end def view - @action_url = append_wd_params(@asset - .get_action_url(current_user, 'view', false)) + @action_url = append_wd_params(@asset.get_action_url(current_user, 'view', false)) @favicon_url = @asset.favicon_url('view') tkn = current_user.get_wopi_token @token = tkn.token @@ -176,12 +117,11 @@ class AssetsController < ApplicationController def update_image @asset = Asset.find(params[:id]) - orig_file_size = @asset.file_file_size - orig_file_name = @asset.file_file_name + orig_file_size = @asset.file_size + orig_file_name = @asset.file_name return render_403 unless can_read_team?(@asset.team) - @asset.file = params[:image] - @asset.file_file_name = orig_file_name + @asset.file.attach(io: params.require(:image), filename: orig_file_name) @asset.save! # release previous image space @asset.team.release_space(orig_file_size) @@ -225,10 +165,10 @@ class AssetsController < ApplicationController render_403 && return unless %w(docx xlsx pptx).include?(params[:file_type]) # Asset validation - file = Paperclip.io_adapters.for(StringIO.new) - file.original_filename = "#{params[:file_name]}.#{params[:file_type]}" - file.content_type = wopi_content_type(params[:file_type]) - asset = Asset.new(file: file, created_by: current_user, file_present: true) + asset = Asset.new(created_by: current_user) + asset.file.attach(io: StringIO.new, + filename: "#{params[:file_name]}.#{params[:file_type]}", + content_type: wopi_content_type(params[:file_type])) unless asset.valid?(:wopi_file_creation) render json: { @@ -315,9 +255,7 @@ class AssetsController < ApplicationController end def asset_params - params.permit( - :file - ) + params.permit(:file) end def asset_data_type(asset) diff --git a/app/controllers/result_assets_controller.rb b/app/controllers/result_assets_controller.rb index 3520626b2..092d32084 100644 --- a/app/controllers/result_assets_controller.rb +++ b/app/controllers/result_assets_controller.rb @@ -194,10 +194,10 @@ class ResultAssetsController < ApplicationController success = true results = [] params[:results_files].values.each_with_index do |file, index| - asset = Asset.new(file: file, - created_by: current_user, + asset = Asset.new(created_by: current_user, last_modified_by: current_user, team: current_team) + asset.file.attach(file) result = Result.new(user: current_user, my_module: @my_module, name: params[:results_names][index.to_s], diff --git a/app/helpers/file_icons_helper.rb b/app/helpers/file_icons_helper.rb index 021303b06..c26116f21 100644 --- a/app/helpers/file_icons_helper.rb +++ b/app/helpers/file_icons_helper.rb @@ -1,11 +1,12 @@ module FileIconsHelper def wopi_file?(asset) - file_ext = asset.file_file_name.split('.').last - %w(csv ods xls xlsb xlsm xlsx odp pot potm potx pps ppsm ppsx ppt pptm pptx doc docm docx dot dotm dotx odt rtf).include?(file_ext) + file_ext = asset.file_name.split('.').last + %w(csv ods xls xlsb xlsm xlsx odp pot potm potx pps ppsm + ppsx ppt pptm pptx doc docm docx dot dotm dotx odt rtf).include?(file_ext) end def file_fa_icon_class(asset) - file_ext = asset.file_file_name.split('.').last + file_ext = asset.file_name.split('.').last if Extends::FILE_FA_ICON_MAPPINGS[file_ext] # Check for custom mappings or possible overrides return Extends::FILE_FA_ICON_MAPPINGS[file_ext] @@ -28,7 +29,7 @@ module FileIconsHelper # For showing next to file def file_extension_icon(asset) - file_ext = asset.file_file_name.split('.').last + file_ext = asset.file_name.split('.').last if Constants::FILE_TEXT_FORMATS.include?(file_ext) image_link = 'office/Word-docx_20x20x32.png' elsif Constants::FILE_TABLE_FORMATS.include?(file_ext) @@ -38,9 +39,7 @@ module FileIconsHelper end # Now check for custom mappings or possible overrides - if Extends::FILE_ICON_MAPPINGS[file_ext] - image_link = Extends::FILE_ICON_MAPPINGS[file_ext] - end + image_link = Extends::FILE_ICON_MAPPINGS[file_ext] if Extends::FILE_ICON_MAPPINGS[file_ext] if image_link image_tag image_link diff --git a/app/models/asset.rb b/app/models/asset.rb index 77470189a..b287ff556 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -204,6 +204,8 @@ class Asset < ApplicationRecord end def previewable? + return false unless file.attached? + previewable_document? || previewable_image? end @@ -222,10 +224,14 @@ class Asset < ApplicationRecord end def file_name + return '' unless file.attached? + file.blob&.filename&.to_s end def file_size + return 0 unless file.attached? + file.blob&.byte_size end @@ -247,7 +253,7 @@ class Asset < ApplicationRecord def text? Constants::TEXT_EXTRACT_FILE_TYPES.any? do |v| - file_content_type.start_with? v + file.content_type.start_with? v end end @@ -530,10 +536,10 @@ class Asset < ApplicationRecord private def previewable_document? - previewable = Constants::PREVIEWABLE_FILE_TYPES.include?(file.blob&.content_type) + previewable = Constants::PREVIEWABLE_FILE_TYPES.include?(file.content_type) - filename = file.blob&.filename - content_type = file.blob&.content_type + filename = file.filename.to_s + content_type = file.content_type extensions = %w(.xlsx .docx .pptx .xls .doc .ppt) # Mimetype sometimes recognizes Office files as zip files diff --git a/app/views/shared/_asset_link.html.erb b/app/views/shared/_asset_link.html.erb index f214ed94f..fb762f4c5 100644 --- a/app/views/shared/_asset_link.html.erb +++ b/app/views/shared/_asset_link.html.erb @@ -2,18 +2,14 @@ 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 %> + <% if display_image_tag && asset.previewable? %> <%= image_tag asset.medium_preview %> - <% end %> - <% if display_image_tag %>

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

- <% else %> - - <%= truncate(asset.file_file_name, - length: Constants::FILENAME_TRUNCATION_LENGTH) %> - + <% else %> + + <%= truncate(asset.file_name, length: Constants::FILENAME_TRUNCATION_LENGTH) %> + <% end %> <% end %> diff --git a/app/views/shared/_file_preview_icon.html.erb b/app/views/shared/_file_preview_icon.html.erb index 45c362022..14b2ef46c 100644 --- a/app/views/shared/_file_preview_icon.html.erb +++ b/app/views/shared/_file_preview_icon.html.erb @@ -1,5 +1,5 @@
- <% if asset.file.previewable_document? %> + <% if asset.previewable? %> <%= image_tag(asset.large_preview) %> <% else %> diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index c88708404..f926d84e1 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -76,7 +76,6 @@ Rails.application.config.assets.precompile += %w(canvas-to-blob.min.js) Rails.application.config.assets.precompile += %w(Sortable.min.js) Rails.application.config.assets.precompile += %w(reports_pdf.css) Rails.application.config.assets.precompile += %w(jszip.min.js) -Rails.application.config.assets.precompile += %w(assets.js) Rails.application.config.assets.precompile += %w(comments.js) Rails.application.config.assets.precompile += %w(projects/show.js) Rails.application.config.assets.precompile += %w(notifications.js)