From 4ea6b565825e70743563516b53d0782aa7149d72 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Mon, 1 Jul 2019 23:30:20 +0200 Subject: [PATCH 1/4] 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) From c0c8e0e1a3f845f06aba758d456f15d2f4f0ff95 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Thu, 4 Jul 2019 09:33:12 +0200 Subject: [PATCH 2/4] Refactor Experiment workflow images [SCI-3539] --- app/controllers/experiments_controller.rb | 7 +++---- app/models/asset.rb | 4 ++-- app/models/experiment.rb | 20 ++++++------------- .../generate_workflow_image_service.rb | 3 +-- .../experiments/move_to_project_service.rb | 2 -- .../projects/show/_workflow_img.html.erb | 5 +---- .../steps/attachments/_placeholder.html.erb | 2 +- config/routes.rb | 2 -- 8 files changed, 14 insertions(+), 31 deletions(-) diff --git a/app/controllers/experiments_controller.rb b/app/controllers/experiments_controller.rb index 7ff2856af..4b2e45bc4 100644 --- a/app/controllers/experiments_controller.rb +++ b/app/controllers/experiments_controller.rb @@ -238,14 +238,13 @@ class ExperimentsController < ApplicationController end def updated_img - if @experiment.workflowimg.present? && !@experiment.workflowimg.exists? - @experiment.workflowimg = nil - @experiment.save + if @experiment.workflowimg.attached? && !@experiment.workflowimg_exists? + @experiment.workflowimg.purge @experiment.generate_workflow_img end respond_to do |format| format.json do - if @experiment.workflowimg.present? + if @experiment.workflowimg.attached? render json: {}, status: 200 else render json: {}, status: 404 diff --git a/app/models/asset.rb b/app/models/asset.rb index b287ff556..44c0f0be8 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -212,14 +212,14 @@ class Asset < ApplicationRecord def medium_preview return file.variant(resize: Constants::MEDIUM_PIC_FORMAT) if previewable_image? - 'medium/processing.gif' + '/images/medium/processing.gif' # file.preview(resize: Constants::MEDIUM_PIC_FORMAT) end def large_preview return file.variant(resize: Constants::LARGE_PIC_FORMAT) if previewable_image? - 'large/processing.gif' + '/images/large/processing.gif' # file.preview(resize: Constants::LARGE_PIC_FORMAT) end diff --git a/app/models/experiment.rb b/app/models/experiment.rb index d730b06d7..5d5bbb40b 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -28,10 +28,7 @@ class Experiment < ApplicationRecord # Associations for old activity type has_many :activities, inverse_of: :experiment - has_attached_file :workflowimg - validates_attachment :workflowimg, - content_type: { content_type: ['image/png'] }, - if: :workflowimg_check + has_one_attached :workflowimg auto_strip_attributes :name, :description, nullify: false validates :name, @@ -220,13 +217,14 @@ class Experiment < ApplicationRecord end def generate_workflow_img - if workflowimg.present? - self.workflowimg = nil - save - end + workflowimg.purge if workflowimg.attached? Experiments::GenerateWorkflowImageService.delay.call(experiment_id: id) end + def workflowimg_exists? + workflowimg.service.exist?(workflowimg.blob.key) + end + # Get projects where user is either owner or user in the same team # as this experiment def projects_with_role_above_user(current_user) @@ -613,12 +611,6 @@ class Experiment < ApplicationRecord true end - def workflowimg_check - workflowimg_content_type - rescue - false - end - def log_activity(type_of, current_user, my_module) Activities::CreateActivityService .call(activity_type: type_of, diff --git a/app/services/experiments/generate_workflow_image_service.rb b/app/services/experiments/generate_workflow_image_service.rb index 725f67be8..8d0027796 100644 --- a/app/services/experiments/generate_workflow_image_service.rb +++ b/app/services/experiments/generate_workflow_image_service.rb @@ -76,9 +76,8 @@ module Experiments def save_file file = Tempfile.open(%w(wimg .png), Rails.root.join('tmp')) @graph.output(png: file.path) - @exp.workflowimg = file + @exp.workflowimg.attach(io: file, filename: File.basename(file.path)) file.close - @exp.save @exp.touch(:workflowimg_updated_at) end end diff --git a/app/services/experiments/move_to_project_service.rb b/app/services/experiments/move_to_project_service.rb index 0603a55ae..6a0837926 100644 --- a/app/services/experiments/move_to_project_service.rb +++ b/app/services/experiments/move_to_project_service.rb @@ -29,8 +29,6 @@ module Experiments end raise ActiveRecord::Rollback unless @exp.save - # To pass the ExperimentsController#updated_img check - @exp.update(workflowimg_updated_at: @exp.updated_at) end @errors.merge!(@exp.errors.to_hash) unless @exp.valid? diff --git a/app/views/projects/show/_workflow_img.html.erb b/app/views/projects/show/_workflow_img.html.erb index f75e948ea..55152afa6 100644 --- a/app/views/projects/show/_workflow_img.html.erb +++ b/app/views/projects/show/_workflow_img.html.erb @@ -1,4 +1 @@ -<%= image_tag(@experiment.workflowimg.expiring_url( - Constants::URL_SHORT_EXPIRE_TIME - ), - class: 'img-responsive center-block') %> +<%= image_tag url_for(@experiment.workflowimg), class: 'img-responsive center-block' %> diff --git a/app/views/steps/attachments/_placeholder.html.erb b/app/views/steps/attachments/_placeholder.html.erb index a07e47d10..56c7acf81 100644 --- a/app/views/steps/attachments/_placeholder.html.erb +++ b/app/views/steps/attachments/_placeholder.html.erb @@ -22,5 +22,5 @@ <% end %> <% end %>
-<% end %> + <% end %> diff --git a/config/routes.rb b/config/routes.rb index 523ab8e4a..c9109fe76 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -579,8 +579,6 @@ Rails.application.routes.draw do # We cannot use 'resources :assets' because assets is a reserved route # in Rails (assets pipeline) and causes funky behavior - get 'files/:id/present', to: 'assets#file_present', as: 'file_present_asset' - get 'files/:id/present_in_step', to: 'assets#step_file_present', as: 'step_file_present_asset' get 'files/:id/preview', to: 'assets#file_preview', as: 'asset_file_preview' From 9d926dc956dac57166cd503b5e215a99e96d066e Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Fri, 5 Jul 2019 16:56:05 +0200 Subject: [PATCH 3/4] Refactor TinyMce assets, user avatars, zip files [SCI-3539] --- .../javascripts/users/registrations/edit.js | 5 +- app/controllers/protocols_controller.rb | 2 +- app/controllers/tiny_mce_assets_controller.rb | 16 +-- .../users/registrations_controller.rb | 102 +++------------- app/controllers/zip_exports_controller.rb | 8 +- app/helpers/application_helper.rb | 6 +- app/helpers/reports_helper.rb | 14 +-- app/models/asset.rb | 55 --------- app/models/concerns/tiny_mce_images.rb | 49 ++++---- app/models/team.rb | 18 +-- app/models/temp_file.rb | 3 +- app/models/tiny_mce_asset.rb | 113 ++++++++---------- app/models/user.rb | 49 ++++++-- app/models/zip_export.rb | 78 ++++-------- .../v1/repository_asset_value_serializer.rb | 4 +- .../api/v1/result_asset_serializer.rb | 4 +- .../generate_workflow_image_service.rb | 1 + app/views/users/registrations/edit.html.erb | 1 - config/initializers/constants.rb | 6 +- .../initializers/filter_parameter_logging.rb | 2 +- config/routes.rb | 1 - ...0190613134100_convert_to_active_storage.rb | 4 +- docker-compose.yml | 2 + spec/services/templates_service_spec.rb | 1 - 24 files changed, 196 insertions(+), 348 deletions(-) diff --git a/app/assets/javascripts/users/registrations/edit.js b/app/assets/javascripts/users/registrations/edit.js index 2f59aeca6..55d363221 100644 --- a/app/assets/javascripts/users/registrations/edit.js +++ b/app/assets/javascripts/users/registrations/edit.js @@ -82,8 +82,8 @@ croppieContainer.croppie({ viewport: { type: 'circle' } }); $('.new-avatar-preview-container').off('update.croppie').on('update.croppie', function() { croppieContainer.croppie('result', { type: 'base64', format: 'jpeg', circle: false }) - .then(function(html) { - $('#user_avatar').val(html); + .then(function(image) { + $('#user_avatar').val(image); }); }); }; @@ -98,5 +98,6 @@ // Local file uploading animateSpinner(); } + $fileInput[0].value = ''; }); }()); diff --git a/app/controllers/protocols_controller.rb b/app/controllers/protocols_controller.rb index f70175b69..f810c7411 100644 --- a/app/controllers/protocols_controller.rb +++ b/app/controllers/protocols_controller.rb @@ -791,7 +791,7 @@ class ProtocolsController < ApplicationController asset_file_name = asset_guid.to_s + File.extname(asset.file_file_name).to_s ostream.put_next_entry("#{step_dir}/#{asset_file_name}") - input_file = asset.open + input_file = asset.file.download ostream.print(input_file.read) input_file.close end diff --git a/app/controllers/tiny_mce_assets_controller.rb b/app/controllers/tiny_mce_assets_controller.rb index bbc921bfa..2e64a4248 100644 --- a/app/controllers/tiny_mce_assets_controller.rb +++ b/app/controllers/tiny_mce_assets_controller.rb @@ -1,16 +1,19 @@ # frozen_string_literal: true class TinyMceAssetsController < ApplicationController - def create image = params.fetch(:file) { render_404 } - tiny_img = TinyMceAsset.new(image: image, - team_id: current_team.id, - saved: false) - if tiny_img.save + tiny_img = TinyMceAsset.new(team_id: current_team.id, saved: false) + + tiny_img.transaction do + tiny_img.save! + tiny_img.image.attach(io: image, filename: image.original_filename) + end + + if tiny_img.persisted? render json: { image: { - url: view_context.image_url(tiny_img.url(:large)), + url: url_for(tiny_img.image), token: Base62.encode(tiny_img.id) } }, content_type: 'text/html' @@ -20,5 +23,4 @@ class TinyMceAssetsController < ApplicationController }, status: :unprocessable_entity end end - end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 58ac7bb8f..d36d90945 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -8,40 +8,8 @@ class Users::RegistrationsController < Devise::RegistrationsController def avatar user = User.find_by_id(params[:id]) || current_user - style = params[:style] || "icon_small" - # TODO: Maybe avatar should be an Asset, so it's methods could be used, - # e.g. presigned_url in this case - redirect_to user.avatar.url(style.to_sym), status: 307 - end - - # Validates asset and then generates S3 upload posts, because - # otherwise untracked files could be uploaded to S3 - def signature - respond_to do |format| - format.json { - - # Changed avatar values are only used for pre-generating S3 key - # and user object is not persisted with this values. - current_user.empty_avatar avatar_params[:file].original_filename, - avatar_params[:file].size - - validation_asset = Asset.new(avatar_params) - if current_user.valid? && validation_asset.valid? - render json: { - posts: generate_upload_posts - } - else - if validation_asset.errors[:file].any? - # Add file content error - current_user.errors[:avatar] << validation_asset.errors[:file].first - end - render json: { - status: 'error', - errors: current_user.errors - }, status: :bad_request - end - } - end + style = params[:style] || :icon_small + redirect_to user.avatar_url(style) end def update_resource(resource, params) @@ -50,12 +18,10 @@ class Users::RegistrationsController < Devise::RegistrationsController if params.include? :change_password # Special handling if changing password params.delete(:change_password) - if ( - resource.valid_password?(params[:current_password]) and - params.include? :password and - params.include? :password_confirmation and - params[:password].blank? - ) then + if resource.valid_password?(params[:current_password]) && + params.include?(:password) && + params.include?(:password_confirmation) && + params[:password].blank? # If new password is blank and we're in process of changing # password, add error to the resource and return false resource.errors.add(:password, :blank) @@ -65,16 +31,26 @@ class Users::RegistrationsController < Devise::RegistrationsController end elsif params.include? :change_avatar params.delete(:change_avatar) - unless params.include? :avatar + if !params.include?(:avatar) resource.errors.add(:avatar, :blank) false else + temp_file = Tempfile.new('avatar', Rails.root.join('tmp')) + begin + temp_file.binmode + temp_file.write(Base64.decode64(params[:avatar].sub(%r{^data:image\/jpeg\;base64,}, ''))) + temp_file.rewind + resource.avatar.attach(io: temp_file, filename: 'avatar.jpg') + ensure + temp_file.close + temp_file.unlink + end + params.delete(:avatar) resource.update_without_password(params) end - elsif params.include? :email or params.include? :password + elsif params.include?(:email) || params.include?(:password) # For changing email or password, validate current_password resource.update_with_password(params) - else # For changing some attributes, no current_password validation # is required @@ -235,7 +211,6 @@ class Users::RegistrationsController < Devise::RegistrationsController :full_name, :initials, :avatar, - :avatar_file_name, :email, :password, :password_confirmation, @@ -251,45 +226,6 @@ class Users::RegistrationsController < Devise::RegistrationsController ) end - # Generates posts for uploading files (many sizes of same file) - # to S3 server - def generate_upload_posts - posts = [] - file_size = current_user.avatar_file_size - content_type = current_user.avatar_content_type - s3_post = S3_BUCKET.presigned_post( - key: current_user.avatar.path[1..-1], - success_action_status: '201', - acl: 'private', - storage_class: "STANDARD", - content_length_range: file_size..file_size, - content_type: content_type - ) - posts.push({ - url: s3_post.url, - fields: s3_post.fields - }) - - current_user.avatar.options[:styles].each do |style, option| - s3_post = S3_BUCKET.presigned_post( - key: current_user.avatar.path(style)[1..-1], - success_action_status: '201', - acl: 'public-read', - storage_class: "REDUCED_REDUNDANCY", - content_length_range: 1..Rails.configuration.x.file_max_size_mb.megabytes, - content_type: content_type - ) - posts.push({ - url: s3_post.url, - fields: s3_post.fields, - style_option: option, - mime_type: content_type - }) - end - - posts - end - private def layout diff --git a/app/controllers/zip_exports_controller.rb b/app/controllers/zip_exports_controller.rb index e30f1300a..5c8d5e669 100644 --- a/app/controllers/zip_exports_controller.rb +++ b/app/controllers/zip_exports_controller.rb @@ -5,12 +5,10 @@ class ZipExportsController < ApplicationController before_action :check_download_permissions, except: :file_expired def download - if @zip_export.stored_on_s3? - redirect_to @zip_export.presigned_url(download: true), status: 307 + if !@zip_export.zip_file.attached? + render_404 else - send_file @zip_export.zip_file.path, - filename: URI.unescape(@zip_export.zip_file_file_name), - type: 'application/zip' + redirect_to rails_blob_path(@zip_export.zip_file, disposition: 'attachment') end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 56ef2b58c..d7b26e533 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -197,11 +197,7 @@ module ApplicationHelper def user_avatar_absolute_url(user, style) begin unless missing_avatar(user, style) - image = if user.avatar.options[:storage].to_sym == :s3 - URI.parse(user.avatar.url(style)).open.to_a.join - else - File.open(user.avatar.path(style)).to_a.join - end + image = File.open(user.avatar_variant(style)) encoded_data = Base64.strict_encode64(image) avatar_base64 = "data:#{user.avatar_content_type};base64,#{encoded_data}" return avatar_base64 diff --git a/app/helpers/reports_helper.rb b/app/helpers/reports_helper.rb index f6fce8d87..c43ef03fe 100644 --- a/app/helpers/reports_helper.rb +++ b/app/helpers/reports_helper.rb @@ -98,19 +98,7 @@ module ReportsHelper # "Hack" to omit file preview URL because of WKHTML issues def report_image_asset_url(asset, type = :asset, klass = nil) - prefix = '' - if ENV['PAPERCLIP_STORAGE'].present? && - ENV['MAIL_SERVER_URL'].present? && - ENV['PAPERCLIP_STORAGE'] == 'filesystem' - prefix = ENV['MAIL_SERVER_URL'] - end - if !prefix.empty? && - !prefix.include?('http://') && - !prefix.include?('https://') - prefix = "http://#{prefix}" - end - size = type == :tiny_mce_asset ? :large : :medium - url = prefix + asset.url(size, timeout: Constants::URL_LONG_EXPIRE_TIME) + url = type == :tiny_mce_asset ? asset.preview : asset.large_preview image_tag(url, class: klass) end diff --git a/app/models/asset.rb b/app/models/asset.rb index 44c0f0be8..29bce0908 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -83,7 +83,6 @@ class Asset < ApplicationRecord after_validation :filter_paperclip_errors # Needed because Paperclip validatates on creation after_initialize :filter_paperclip_errors, if: :new_record? - before_destroy :paperclip_delete, prepend: true after_save { result&.touch; step&.touch } attr_accessor :file_content, :file_info, :preview_cached, :in_template @@ -330,23 +329,6 @@ class Asset < ApplicationRecord end end - # Workaround for making Paperclip work with asset deletion (might be because - # of how the asset models are implemented) - def paperclip_delete - report_elements.destroy_all - asset_text_datum.destroy if asset_text_datum.present? - # Nullify needed to force paperclip file deletion - file = nil - save && reload - end - - def destroy - super() - # Needed, otherwise the object isn't deleted, because of how the asset - # models are implemented - delete - end - # If team is provided, its space_taken # is updated as well def update_estimated_size(team = nil) @@ -369,43 +351,6 @@ class Asset < ApplicationRecord end end - def url(style = :original, timeout: Constants::URL_SHORT_EXPIRE_TIME) - if file.is_stored_on_s3? && !file.processing? - presigned_url(style, timeout: timeout) - else - file.url(style) - end - end - - # When using S3 file upload, we can limit file accessibility with url signing - def presigned_url(style = :original, - download: false, - timeout: Constants::URL_SHORT_EXPIRE_TIME) - if file.is_stored_on_s3? - if download - download_arg = 'attachment; filename=' + URI.escape(file_file_name) - else - download_arg = nil - end - - signer = Aws::S3::Presigner.new(client: S3_BUCKET.client) - signer.presigned_url(:get_object, - bucket: S3_BUCKET.name, - key: file.path(style)[1..-1], - expires_in: timeout, - # this response header forces object download - response_content_disposition: download_arg) - end - end - - def open - if file.is_stored_on_s3? - Kernel.open(presigned_url, "rb") - else - File.open(file.path, "rb") - end - end - # Preserving attachments (on client-side) between failed validations # (only usable for small/few files!). # Needs to be called before save method and view needs to have diff --git a/app/models/concerns/tiny_mce_images.rb b/app/models/concerns/tiny_mce_images.rb index a7965d4e8..84f5ab59b 100644 --- a/app/models/concerns/tiny_mce_images.rb +++ b/app/models/concerns/tiny_mce_images.rb @@ -18,23 +18,17 @@ module TinyMceImages description = TinyMceAsset.update_old_tinymce(description, self) tiny_mce_assets.each do |tm_asset| - tmp_f = Tempfile.open(tm_asset.image_file_name, Rails.root.join('tmp')) - begin - tm_asset.image.copy_to_local_file(:large, tmp_f.path) - encoded_tm_asset = Base64.strict_encode64(tmp_f.read) - new_tm_asset_src = "data:image/jpg;base64,#{encoded_tm_asset}" - html_description = Nokogiri::HTML(description) - tm_asset_to_update = html_description.css( - "img[data-mce-token=\"#{Base62.encode(tm_asset.id)}\"]" - )[0] - next unless tm_asset_to_update + tm_asset_key = tm_asset.image.preview.key + encoded_tm_asset = Base64.strict_encode64(tm_asset.image.service.download(tm_asset_key)) + new_tm_asset_src = "data:image/jpg;base64,#{encoded_tm_asset}" + html_description = Nokogiri::HTML(description) + tm_asset_to_update = html_description.css( + "img[data-mce-token=\"#{Base62.encode(tm_asset.id)}\"]" + )[0] + next unless tm_asset_to_update - tm_asset_to_update.attributes['src'].value = new_tm_asset_src - description = html_description.css('body').inner_html.to_s - ensure - tmp_f.close - tmp_f.unlink - end + tm_asset_to_update.attributes['src'].value = new_tm_asset_src + description = html_description.css('body').inner_html.to_s end description end @@ -72,12 +66,15 @@ module TinyMceImages cloned_img_ids = [] tiny_mce_assets.each do |tiny_img| tiny_img_clone = TinyMceAsset.new( - image: tiny_img.image, estimated_size: tiny_img.estimated_size, object: target, team: team ) - tiny_img_clone.save! + + tiny_img_clone.transaction do + tiny_img_clone.save! + tiny_img_clone.image.attach(io: tiny_img.image.open, filename: tiny_img.image.filename.to_s) + end target.tiny_mce_assets << tiny_img_clone cloned_img_ids << [tiny_img.id, tiny_img_clone.id] @@ -86,7 +83,7 @@ module TinyMceImages end def copy_unknown_tiny_mce_images - asset_team_id = Team.find_by_object(self) + asset_team_id = Team.find_by_object(self).id return unless asset_team_id object_field = Extends::RICH_TEXT_FIELD_MAPPINGS[self.class.name] @@ -97,19 +94,27 @@ module TinyMceImages if image['data-mce-token'] asset = TinyMceAsset.find_by_id(Base62.decode(image['data-mce-token'])) - next if asset && asset.object == self + next if asset && asset.object == self && asset.team_id != asset_team_id new_image = asset.image + new_image_filename = new_image.image.filename.to_s else - new_image = URI.parse(image['src']) + # We need implement size and type checks here + new_image = URI.parse(image['src']).open + new_image_filename = asset.class.generate_unique_secure_token + '.jpg' + # new_image_filename = File.basename(new_image.base_uri.path) end new_asset = TinyMceAsset.create( - image: new_image, object: self, team_id: asset_team_id ) + new_asset.transaction do + new_asset.save! + new_asset.image.attach(io: new_image, filename: new_image_filename) + end + image['src'] = '' image['class'] = 'img-responsive' image['data-mce-token'] = Base62.encode(new_asset.id) diff --git a/app/models/team.rb b/app/models/team.rb index 054ba05de..dbc88f7da 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -327,14 +327,16 @@ class Team < ApplicationRecord end def self.find_by_object(obj) - case obj.class.name - when 'Protocol' - obj.team_id - when 'MyModule', 'Step' - obj.protocol.team_id - when 'ResultText' - obj.result.my_module.protocol.team_id - end + find( + case obj.class.name + when 'Protocol' + obj.team_id + when 'MyModule', 'Step' + obj.protocol.team_id + when 'ResultText' + obj.result.my_module.protocol.team_id + end + ) end private diff --git a/app/models/temp_file.rb b/app/models/temp_file.rb index 134dfa830..8ed198c35 100644 --- a/app/models/temp_file.rb +++ b/app/models/temp_file.rb @@ -3,8 +3,7 @@ class TempFile < ApplicationRecord validates :session_id, presence: true - has_attached_file :file - do_not_validate_attachment_file_type :file + has_one_attached :file class << self def destroy_obsolete(temp_file_id) diff --git a/app/models/tiny_mce_asset.rb b/app/models/tiny_mce_asset.rb index 8e4c0b287..701f7e825 100644 --- a/app/models/tiny_mce_asset.rb +++ b/app/models/tiny_mce_asset.rb @@ -4,7 +4,7 @@ class TinyMceAsset < ApplicationRecord extend ProtocolsExporter attr_accessor :reference before_create :set_reference, optional: true - after_create :update_estimated_size, :self_destruct + after_create :calculate_estimated_size, :self_destruct after_destroy :release_team_space belongs_to :team, inverse_of: :tiny_mce_assets, optional: true @@ -17,20 +17,23 @@ class TinyMceAsset < ApplicationRecord belongs_to :object, polymorphic: true, optional: true, inverse_of: :tiny_mce_assets - has_attached_file :image, - styles: { large: [Constants::LARGE_PIC_FORMAT, :jpg] }, - convert_options: { large: '-quality 100 -strip' } - validates_attachment_content_type :image, - content_type: %r{^image/#{ Regexp.union( - Constants::WHITELISTED_IMAGE_TYPES - ) }} - validates_attachment :image, - presence: true, - size: { - less_than: Rails.configuration.x\ - .file_max_size_mb.megabytes - } + has_one_attached :image + + # has_attached_file :image, + # styles: { large: [Constants::LARGE_PIC_FORMAT, :jpg] }, + # convert_options: { large: '-quality 100 -strip' } + + # validates_attachment_content_type :image, + # content_type: %r{^image/#{ Regexp.union( + # Constants::WHITELISTED_IMAGE_TYPES + # ) }} + # validates_attachment :image, + # presence: true, + # size: { + # less_than: Rails.configuration.x\ + # .file_max_size_mb.megabytes + # } validates :estimated_size, presence: true def self.update_images(object, images) @@ -58,48 +61,17 @@ class TinyMceAsset < ApplicationRecord tm_assets = description.css('img[data-mce-token]') tm_assets.each do |tm_asset| asset_id = tm_asset.attr('data-mce-token') - new_asset_url = find_by_id(Base62.decode(asset_id)) - if new_asset_url - tm_asset.attributes['src'].value = new_asset_url.url + new_asset = obj.tiny_mce_assets.find_by_id(Base62.decode(asset_id)) + if new_asset + tm_asset.attributes['src'].value = Rails.application.routes.url_helpers.url_for(new_asset.image) tm_asset['class'] = 'img-responsive' end end description.css('body').inner_html.to_s end - def presigned_url(style = :large, - download: false, - timeout: Constants::URL_LONG_EXPIRE_TIME) - if stored_on_s3? - download_arg = ('attachment; filename=' + CGI.escape(image_file_name) if download) - - signer = Aws::S3::Presigner.new(client: S3_BUCKET.client) - signer.presigned_url(:get_object, - bucket: S3_BUCKET.name, - key: image.path(style)[1..-1], - expires_in: timeout, - response_content_disposition: download_arg) - end - end - - def stored_on_s3? - image.options[:storage].to_sym == :s3 - end - - def url(style = :large, timeout: Constants::URL_LONG_EXPIRE_TIME) - if image.is_stored_on_s3? - presigned_url(style, timeout: timeout) - else - image.url(style) - end - end - - def open - if image.is_stored_on_s3? - Kernel.open(presigned_url, 'rb') - else - File.open(image.path, 'rb') - end + def preview + image.variant(resize: Constants::LARGE_PIC_FORMAT) end def self.delete_unsaved_image(id) @@ -107,6 +79,21 @@ class TinyMceAsset < ApplicationRecord asset.destroy if asset && !asset.saved end + def self.update_estimated_size(id) + asset = find_by_id(id) + return unless asset&.image&.attached? + + size = asset.image.blob.byte_size + return if size.blank? + + e_size = size * Constants::ASSET_ESTIMATED_SIZE_FACTOR + asset.update(estimated_size: e_size) + Rails.logger.info "Asset #{id}: Estimated size successfully calculated" + # update team space taken + asset.team.take_space(e_size) + asset.team.save + end + def self.update_old_tinymce(description, obj = nil) return description unless description @@ -133,11 +120,9 @@ class TinyMceAsset < ApplicationRecord if exists? order(:id).each do |tiny_mce_asset| asset_guid = get_guid(tiny_mce_asset.id) - asset_file_name = - "rte-#{asset_guid.to_s + - File.extname(tiny_mce_asset.image_file_name).to_s}" + asset_file_name = "rte-#{asset_guid.to_s + File.extname(tiny_mce_asset.image.filename.to_s)}" ostream.put_next_entry("#{dir}/#{asset_file_name}") - input_file = tiny_mce_asset.open + input_file = tiny_mce_asset.image.download ostream.print(input_file.read) input_file.close end @@ -161,12 +146,17 @@ class TinyMceAsset < ApplicationRecord return false unless team_id tiny_img_clone = TinyMceAsset.new( - image: image, estimated_size: estimated_size, object: obj, team_id: team_id ) - tiny_img_clone.save! + + tiny_img_clone.transaction do + tiny_img_clone.save! + tiny_img_clone.image.attach(io: image.download, filename: image.filename.to_s) + end + + return false unless tiny_img_clone.persisted? obj.tiny_mce_assets << tiny_img_clone # Prepare array of image to update @@ -187,15 +177,8 @@ class TinyMceAsset < ApplicationRecord TinyMceAsset.delay(queue: :assets, run_at: 1.days.from_now).delete_unsaved_image(id) end - def update_estimated_size - return if image_file_size.blank? - - es = image_file_size * Constants::ASSET_ESTIMATED_SIZE_FACTOR - update(estimated_size: es) - Rails.logger.info "Asset #{id}: Estimated size successfully calculated" - # update team space taken - team.take_space(es) - team.save + def calculate_estimated_size + TinyMceAsset.delay(queue: :assets, run_at: 5.minutes.from_now).update_estimated_size(id) end def release_team_space diff --git a/app/models/user.rb b/app/models/user.rb index 8228f67e0..d7a6a215a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,14 +15,17 @@ class User < ApplicationRecord :timeoutable, :omniauthable, omniauth_providers: Extends::OMNIAUTH_PROVIDERS, stretches: Constants::PASSWORD_STRETCH_FACTOR - has_attached_file :avatar, - styles: { - medium: Constants::MEDIUM_PIC_FORMAT, - thumb: Constants::THUMB_PIC_FORMAT, - icon: Constants::ICON_PIC_FORMAT, - icon_small: Constants::ICON_SMALL_PIC_FORMAT - }, - default_url: Constants::DEFAULT_AVATAR_URL + + has_one_attached :avatar + + # has_attached_file :avatar, + # styles: { + # medium: Constants::MEDIUM_PIC_FORMAT, + # thumb: Constants::THUMB_PIC_FORMAT, + # icon: Constants::ICON_PIC_FORMAT, + # icon_small: Constants::ICON_SMALL_PIC_FORMAT + # }, + # default_url: Constants::DEFAULT_AVATAR_URL auto_strip_attributes :full_name, :initials, nullify: false validates :full_name, @@ -35,10 +38,10 @@ class User < ApplicationRecord presence: true, length: { maximum: Constants::EMAIL_MAX_LENGTH } - validates_attachment :avatar, - :content_type => { :content_type => ["image/jpeg", "image/png"] }, - size: { less_than: Constants::AVATAR_MAX_SIZE_MB.megabyte, - message: I18n.t('client_api.user.avatar_too_big') } + # validates_attachment :avatar, + # :content_type => { :content_type => ["image/jpeg", "image/png"] }, + # size: { less_than: Constants::AVATAR_MAX_SIZE_MB.megabyte, + # message: I18n.t('client_api.user.avatar_too_big') } validate :time_zone_check store_accessor :settings, :time_zone, :notifications_settings @@ -229,7 +232,7 @@ class User < ApplicationRecord # If other errors besides parameter "avatar" exist, # they will propagate to "avatar" also, so remove them # and put all other (more specific ones) in it - after_validation :filter_paperclip_errors + # after_validation :filter_paperclip_errors before_destroy :destroy_notifications @@ -249,6 +252,26 @@ class User < ApplicationRecord @avatar_remote_url = url_value end + def avatar_variant(style) + format = case style.to_sym + when :medium + Constants::MEDIUM_PIC_FORMAT + when :thumb + Constants::THUMB_PIC_FORMAT + when :icon + Constants::ICON_PIC_FORMAT + when :icon_small + Constants::ICON_SMALL_PIC_FORMAT + else + Constants::ICON_SMALL_PIC_FORMAT + end + avatar.variant(resize: format) + end + + def avatar_url(style) + Rails.application.routes.url_helpers.url_for(avatar_variant(style)) + end + def date_format settings[:date_format] || Constants::DEFAULT_DATE_FORMAT end diff --git a/app/models/zip_export.rb b/app/models/zip_export.rb index c44590437..27ede12ef 100644 --- a/app/models/zip_export.rb +++ b/app/models/zip_export.rb @@ -22,71 +22,45 @@ class ZipExport < ApplicationRecord belongs_to :user, optional: true # Override path only for S3 - if ENV['PAPERCLIP_STORAGE'] == 's3' - s3_path = - if ENV['S3_SUBFOLDER'] - "/#{ENV['S3_SUBFOLDER']}/zip_exports/:attachment/"\ - ":id_partition/:hash/:style/:filename" - else - '/zip_exports/:attachment/:id_partition/:hash/:style/:filename' - end + # if ENV['PAPERCLIP_STORAGE'] == 's3' + # s3_path = + # if ENV['S3_SUBFOLDER'] + # "/#{ENV['S3_SUBFOLDER']}/zip_exports/:attachment/"\ + # ":id_partition/:hash/:style/:filename" + # else + # '/zip_exports/:attachment/:id_partition/:hash/:style/:filename' + # end + # + # has_attached_file :zip_file, path: s3_path + # else + # has_attached_file :zip_file + # end - has_attached_file :zip_file, path: s3_path - else - has_attached_file :zip_file - end + has_one_attached :zip_file - validates_attachment :zip_file, - content_type: { content_type: 'application/zip' } + # validates_attachment :zip_file, + # content_type: { content_type: 'application/zip' } after_create :self_destruct - # When using S3 file upload, we can limit file accessibility with url signing - def presigned_url(style = :original, - download: false, - timeout: Constants::URL_SHORT_EXPIRE_TIME) - if stored_on_s3? - if download - download_arg = 'attachment; filename=' + URI.escape(zip_file_file_name) - else - download_arg = nil - end - - signer = Aws::S3::Presigner.new(client: S3_BUCKET.client) - signer.presigned_url(:get_object, - bucket: S3_BUCKET.name, - key: zip_file.path(style)[1..-1], - expires_in: timeout, - response_content_disposition: download_arg) - end - end - - def stored_on_s3? - zip_file.options[:storage].to_sym == :s3 - end - def self.delete_expired_export(id) export = find_by_id(id) - export.destroy if export + export&.destroy end def generate_exportable_zip(user, data, type, options = {}) - I18n.backend.date_format = - user.settings[:date_format] || Constants::DEFAULT_DATE_FORMAT - zip_input_dir = FileUtils.mkdir_p( - File.join(Rails.root, "tmp/temp_zip_#{Time.now.to_i}") - ).first - zip_dir = FileUtils.mkdir_p(File.join(Rails.root, 'tmp/zip-ready')).first - zip_file = File.new( - File.join(zip_dir, "export_#{Time.now.strftime('%F %H-%M-%S_UTC')}.zip"), - 'w+' - ) + I18n.backend.date_format = user.settings[:date_format] || Constants::DEFAULT_DATE_FORMAT + zip_input_dir = FileUtils.mkdir_p(File.join(Rails.root, "tmp/temp_zip_#{Time.now.to_i}")).first + tmp_zip_dir = FileUtils.mkdir_p(File.join(Rails.root, 'tmp/zip-ready')).first + tmp_zip_name = "export_#{Time.now.strftime('%F %H-%M-%S_UTC')}.zip" + tmp_zip_file = File.new(File.join(tmp_zip_dir, tmp_zip_name), 'w+') + fill_content(zip_input_dir, data, type, options) - zip!(zip_input_dir, zip_file) - self.zip_file = File.open(zip_file) + zip!(zip_input_dir, tmp_zip_file) + zip_file.attach(io: File.open(tmp_zip_file), filename: tmp_zip_name) generate_notification(user) if save ensure - FileUtils.rm_rf([zip_input_dir, zip_file], secure: true) + FileUtils.rm_rf([zip_input_dir, tmp_zip_file], secure: true) end handle_asynchronously :generate_exportable_zip diff --git a/app/serializers/api/v1/repository_asset_value_serializer.rb b/app/serializers/api/v1/repository_asset_value_serializer.rb index c6099a3fd..ad7eb2458 100644 --- a/app/serializers/api/v1/repository_asset_value_serializer.rb +++ b/app/serializers/api/v1/repository_asset_value_serializer.rb @@ -20,10 +20,8 @@ module Api def url if !object.asset&.file&.exists? nil - elsif object.asset&.file&.is_stored_on_s3? - object.asset.presigned_url(download: true) else - object.asset.file.url + rails_blob_path(object.asset.file, disposition: 'attachment') end end end diff --git a/app/serializers/api/v1/result_asset_serializer.rb b/app/serializers/api/v1/result_asset_serializer.rb index 0e31473a0..8e24e9a3b 100644 --- a/app/serializers/api/v1/result_asset_serializer.rb +++ b/app/serializers/api/v1/result_asset_serializer.rb @@ -21,10 +21,8 @@ module Api def url if !object.asset&.file&.exists? nil - elsif object.asset&.file&.is_stored_on_s3? - object.asset.presigned_url(download: true) else - object.asset.file.url + rails_blob_path(object.asset.file, disposition: 'attachment') end end end diff --git a/app/services/experiments/generate_workflow_image_service.rb b/app/services/experiments/generate_workflow_image_service.rb index 8d0027796..39a69324f 100644 --- a/app/services/experiments/generate_workflow_image_service.rb +++ b/app/services/experiments/generate_workflow_image_service.rb @@ -78,6 +78,7 @@ module Experiments @graph.output(png: file.path) @exp.workflowimg.attach(io: file, filename: File.basename(file.path)) file.close + file.unlink @exp.touch(:workflowimg_updated_at) end end diff --git a/app/views/users/registrations/edit.html.erb b/app/views/users/registrations/edit.html.erb index ef24e8ce8..9d9cc5464 100644 --- a/app/views/users/registrations/edit.html.erb +++ b/app/views/users/registrations/edit.html.erb @@ -239,4 +239,3 @@ <%= stylesheet_pack_tag 'custom/croppie_styles' %> <%= javascript_include_tag("canvas-to-blob.min") %> <%= javascript_include_tag "users/registrations/edit" %> - diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb index afdf18be1..1bd3a771b 100644 --- a/config/initializers/constants.rb +++ b/config/initializers/constants.rb @@ -90,9 +90,9 @@ class Constants # Picture size formats LARGE_PIC_FORMAT = '800x600>'.freeze MEDIUM_PIC_FORMAT = '300x300>'.freeze - THUMB_PIC_FORMAT = '100x100#'.freeze - ICON_PIC_FORMAT = '40x40#'.freeze - ICON_SMALL_PIC_FORMAT = '30x30#'.freeze + THUMB_PIC_FORMAT = '100x100'.freeze + ICON_PIC_FORMAT = '40x40'.freeze + ICON_SMALL_PIC_FORMAT = '30x30'.freeze # Hands-on-table number of starting columns and rows HANDSONTABLE_INIT_COLS_CNT = 5 diff --git a/config/initializers/filter_parameter_logging.rb b/config/initializers/filter_parameter_logging.rb index ec91e2ccf..156ba453d 100644 --- a/config/initializers/filter_parameter_logging.rb +++ b/config/initializers/filter_parameter_logging.rb @@ -1,4 +1,4 @@ # Be sure to restart your server when you modify this file. # Configure sensitive parameters which will be filtered from the log file. -Rails.application.config.filter_parameters += %i(password image) +Rails.application.config.filter_parameters += [:password, :image, /^avatar$/] diff --git a/config/routes.rb b/config/routes.rb index c9109fe76..a9a665a6a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -594,7 +594,6 @@ Rails.application.routes.draw do devise_scope :user do get 'avatar/:id/:style' => 'users/registrations#avatar', as: 'avatar' - post 'avatar_signature' => 'users/registrations#signature' get 'users/auth_token_sign_in' => 'users/sessions#auth_token_create' get 'users/sign_up_provider' => 'users/registrations#new_with_provider' post 'users/complete_sign_up_provider' => diff --git a/db/migrate/20190613134100_convert_to_active_storage.rb b/db/migrate/20190613134100_convert_to_active_storage.rb index 76593aa1b..0d651d7d4 100644 --- a/db/migrate/20190613134100_convert_to_active_storage.rb +++ b/db/migrate/20190613134100_convert_to_active_storage.rb @@ -99,8 +99,8 @@ class ConvertToActiveStorage < ActiveRecord::Migration[5.2] 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)) + instance.class.generate_unique_secure_token + # File.join('storage', key.first(2), key.first(4).last(2)) end end diff --git a/docker-compose.yml b/docker-compose.yml index bfbdcf1a6..a89a1f446 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,7 @@ services: - .:/usr/src/app - scinote_development_bundler:/usr/local/bundle/ - scinote_development_files:/usr/src/app/public/system + - scinote_development_storage:/usr/src/app/storage webpack: build: @@ -54,3 +55,4 @@ volumes: scinote_development_postgres: scinote_development_bundler: scinote_development_files: + scinote_development_storage: diff --git a/spec/services/templates_service_spec.rb b/spec/services/templates_service_spec.rb index 8274761c1..d7799b889 100644 --- a/spec/services/templates_service_spec.rb +++ b/spec/services/templates_service_spec.rb @@ -88,7 +88,6 @@ describe TemplatesService do .to eq(tmpl_step.step_comments.size) end end - Asset.all.each(&:paperclip_delete) FileUtils.remove_dir(tmplts_dir) end end From a1941a21dcabf81daaa175bde2a9e34428ceef10 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Tue, 9 Jul 2019 10:28:15 +0200 Subject: [PATCH 4/4] Futher refactoring of old Paperclip methods [SCI-3539] --- Gemfile | 2 +- Gemfile.lock | 6 +- .../forms/repository_item_edit.js | 6 +- .../repositories/repository_datatable.js.erb | 2 +- app/controllers/assets_controller.rb | 2 +- app/controllers/protocols_controller.rb | 9 +- app/controllers/teams_controller.rb | 178 +-------------- app/controllers/wopi_controller.rb | 6 +- app/helpers/file_icons_helper.rb | 4 +- app/helpers/my_modules_helper.rb | 6 +- app/helpers/reports_helper.rb | 2 +- app/models/asset.rb | 212 ++++++------------ app/models/concerns/tiny_mce_images.rb | 3 +- app/models/protocol.rb | 47 +--- app/models/repository_asset_value.rb | 7 +- app/models/team.rb | 118 ---------- app/models/team_zip_export.rb | 11 +- app/models/tiny_mce_asset.rb | 23 +- app/models/user.rb | 19 -- app/models/zip_export.rb | 8 +- .../v1/repository_asset_value_serializer.rb | 4 +- .../api/v1/result_asset_serializer.rb | 4 +- .../repository_actions/duplicate_cell.rb | 27 +-- app/services/repository_datatable_service.rb | 8 +- app/services/spreadsheet_parser.rb | 5 - app/services/team_importer.rb | 6 +- app/utilities/protocols_exporter.rb | 13 +- app/utilities/protocols_importer.rb | 13 +- app/utilities/wopi_util.rb | 4 +- app/views/assets/edit.erb | 2 +- app/views/assets/view.erb | 2 +- .../_my_module_result_asset_element.html.erb | 2 +- .../elements/_step_asset_element.html.erb | 4 +- app/views/result_assets/_edit.html.erb | 2 +- .../results/partials/_asset_text.html.erb | 2 +- config/environments/production.rb | 4 +- config/initializers/extends.rb | 4 +- config/locales/en.yml | 1 - config/routes.rb | 2 - docker-compose.production.yml | 2 + 40 files changed, 178 insertions(+), 604 deletions(-) diff --git a/Gemfile b/Gemfile index 4946767cd..b523846fa 100644 --- a/Gemfile +++ b/Gemfile @@ -12,7 +12,7 @@ gem 'bootstrap_form', '~> 2.7.0' gem 'devise', '~> 4.6.2' gem 'devise_invitable' gem 'figaro' -gem 'pg', '~> 0.18' +gem 'pg', '~> 1.1' gem 'pg_search' # PostgreSQL full text search gem 'rails', '~> 5.2.3' gem 'recaptcha', require: 'recaptcha/rails' diff --git a/Gemfile.lock b/Gemfile.lock index e2e5e6687..c2297e6d7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -364,7 +364,9 @@ GEM parallel (1.17.0) parser (2.6.3.0) ast (~> 2.4.0) - pg (0.21.0) + pg (1.1.4) + activerecord (>= 4.2) + activesupport (>= 4.2) pg_search (2.2.0) activerecord (>= 4.2) activesupport (>= 4.2) @@ -627,7 +629,7 @@ DEPENDENCIES omniauth-linkedin-oauth2 overcommit paperclip (~> 6.1) - pg (~> 0.18) + pg (~> 1.1) pg_search phantomjs poltergeist diff --git a/app/assets/javascripts/repositories/forms/repository_item_edit.js b/app/assets/javascripts/repositories/forms/repository_item_edit.js index 0a36e2d9e..b6b1d5a0d 100644 --- a/app/assets/javascripts/repositories/forms/repository_item_edit.js +++ b/app/assets/javascripts/repositories/forms/repository_item_edit.js @@ -71,7 +71,7 @@ $.each(itemData.repository_row.repository_cells, function(i, cell) { var tableCellId = 'colId-' + cell.cell_column_id; if(cell.type === 'RepositoryAssetValue') { - formBindingsData[tableCellId] = new File([""], cell.value.file_file_name); + formBindingsData[tableCellId] = new File([""], cell.value.file_name); } else { formBindingsData[tableCellId] = cell.value; } @@ -193,7 +193,7 @@ * @returns (String) */ function changeToInputFileField(object, name, value, id) { - var fileName = (value.file_file_name) ? value.file_file_name : I18n.t('general.file.no_file_chosen'); + var fileName = (value.file_name) ? value.file_name : I18n.t('general.file.no_file_chosen'); var buttonLabel = I18n.t('general.file.choose'); var html = "
" + "
" + buttonLabel + "

" + truncateLongString(fileName, 20) + "

"; - if(value.file_file_name) { + if(value.file_name) { html += "
e Rails.logger.fatal( - "Asset #{asset.id}: Error extracting contents from asset "\ - "file #{asset.file.path}: #{e.message}" + "Asset #{id}: Error extracting contents from asset "\ + "file #{file.blob.key}: #{e.message}" ) - ensure - File.delete file_path if fa end end # If team is provided, its space_taken # is updated as well def update_estimated_size(team = nil) - return if file_file_size.blank? || in_template + return if file_size.blank? || in_template - es = file_file_size + es = file_size if asset_text_datum.present? && asset_text_datum.persisted? asset_text_datum.reload es += get_octet_length_record(asset_text_datum, :data) @@ -351,29 +343,18 @@ class Asset < ApplicationRecord end end - # Preserving attachments (on client-side) between failed validations - # (only usable for small/few files!). - # Needs to be called before save method and view needs to have - # :file_content and :file_info hidden field. - # If file is an image, it can be viewed on front-end - # using @preview_cached with image_tag tag. - def preserve(file_data) - if file_data[:file_content].present? - restore_cached(file_data[:file_content], file_data[:file_info]) - end - cache - end - def can_perform_action(action) if ENV['WOPI_ENABLED'] == 'true' - file_ext = file_file_name.split('.').last + file_ext = file_name.split('.').last if file_ext == 'wopitest' && (!ENV['WOPI_TEST_ENABLED'] || ENV['WOPI_TEST_ENABLED'] == 'false') return false end + action = get_action(file_ext, action) return false if action.nil? + true else false @@ -381,7 +362,7 @@ class Asset < ApplicationRecord end def get_action_url(user, action, with_tokens = true) - file_ext = file_file_name.split('.').last + file_ext = file_name.split('.').last action = get_action(file_ext, action) if !action.nil? action_url = action.urlsrc @@ -404,7 +385,7 @@ class Asset < ApplicationRecord ) action_url += "WOPISrc=#{rest_url}" if with_tokens - token = user.get_wopi_token + token = user.get_wopi_token action_url + "&access_token=#{token.token}"\ "&access_token_ttl=#{(token.ttl * 1000)}" else @@ -416,7 +397,7 @@ class Asset < ApplicationRecord end def favicon_url(action) - file_ext = file_file_name.split('.').last + file_ext = file_name.split('.').last action = get_action(file_ext, action) action.wopi_app.icon if action.try(:wopi_app) end @@ -458,8 +439,8 @@ class Asset < ApplicationRecord def update_contents(new_file) new_file.class.class_eval { attr_accessor :original_filename } - new_file.original_filename = file_file_name - self.file = new_file + new_file.original_filename = file_name + file.attach(io: new_file, filename: original_filename) self.version = version.nil? ? 1 : version + 1 save end @@ -468,18 +449,12 @@ class Asset < ApplicationRecord !locked? && %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES_EDITABLE)}} =~ file.content_type end - protected - - # Checks if attachments is an image (in post processing imagemagick will - # generate styles) - def allow_styles_on_images - if !(file.content_type =~ %r{^(image|(x-)?application)/(x-png|pjpeg|jpeg|jpg|png|gif)$}) - return false - end - end - private + def tempdir + Rails.root.join('tmp') + end + def previewable_document? previewable = Constants::PREVIEWABLE_FILE_TYPES.include?(file.content_type) @@ -503,29 +478,6 @@ class Asset < ApplicationRecord 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] - errors.clear - errors.add(:file, temp_errors) - end - end - - def file_changed? - previous_changes.present? and - ( - ( - previous_changes.key? "file_file_name" and - previous_changes["file_file_name"].first != - previous_changes["file_file_name"].last - ) or ( - previous_changes.key? "file_file_size" and - previous_changes["file_file_size"].first != - previous_changes["file_file_size"].last - ) - ) - end - def step_or_result_or_repository_asset_value # We must allow both step and result to be blank because of GUI # (even though it's not really a "valid" asset) @@ -558,30 +510,4 @@ class Asset < ApplicationRecord ) end end - - def cache - fetched_file = file.fetch - file_content = fetched_file.read - @file_content = encrypt(file_content) - @file_info = %Q[{"content_type" : "#{file.content_type}", "original_filename" : "#{file.original_filename}"}] - @file_info = encrypt(@file_info) - if !(file.content_type =~ /^image/).nil? - @preview_cached = "data:image/png;base64," + Base64.encode64(file_content) - end - end - - def restore_cached(file_content, file_info) - decoded_data = decrypt(file_content) - decoded_data_info = decrypt(file_info) - decoded_data_info = JSON.parse decoded_data_info - - data = StringIO.new(decoded_data) - data.class_eval do - attr_accessor :content_type, :original_filename - end - data.content_type = decoded_data_info['content_type'] - data.original_filename = File.basename(decoded_data_info['original_filename']) - - self.file = data - end end diff --git a/app/models/concerns/tiny_mce_images.rb b/app/models/concerns/tiny_mce_images.rb index 84f5ab59b..b5cc990f2 100644 --- a/app/models/concerns/tiny_mce_images.rb +++ b/app/models/concerns/tiny_mce_images.rb @@ -97,12 +97,11 @@ module TinyMceImages next if asset && asset.object == self && asset.team_id != asset_team_id new_image = asset.image - new_image_filename = new_image.image.filename.to_s + new_image_filename = new_image.file_name else # We need implement size and type checks here new_image = URI.parse(image['src']).open new_image_filename = asset.class.generate_unique_secure_token + '.jpg' - # new_image_filename = File.basename(new_image.base_uri.path) end new_asset = TinyMceAsset.create( diff --git a/app/models/protocol.rb b/app/models/protocol.rb index add6f26d4..942386d98 100644 --- a/app/models/protocol.rb +++ b/app/models/protocol.rb @@ -226,41 +226,15 @@ class Protocol < ApplicationRecord end # Deep-clone given array of assets - def self.deep_clone_assets(assets_to_clone, team) + def self.deep_clone_assets(assets_to_clone) assets_to_clone.each do |src_id, dest_id| src = Asset.find_by_id(src_id) dest = Asset.find_by_id(dest_id) dest.destroy! if src.blank? && dest.present? next unless src.present? && dest.present? + # Clone file - dest.file = src.file - dest.save! - - if dest.image? - dest.file.reprocess!(:large) - dest.file.reprocess!(:medium) - end - - # Clone extracted text data if it exists - if (atd = src.asset_text_datum).present? - atd2 = AssetTextDatum.new( - data: atd.data, - asset: dest - ) - atd2.save! - end - - # Update estimated size of cloned asset - # (& file_present flag) - dest.update( - estimated_size: src.estimated_size, - file_present: true - ) - - # Update team's space taken - team.reload - team.take_space(dest.estimated_size) - team.save! + src.duplicate_file(dst) end end @@ -318,16 +292,8 @@ class Protocol < ApplicationRecord # "Shallow" Copy assets step.assets.each do |asset| - asset2 = Asset.new_empty( - asset.file_file_name, - asset.file_file_size - ) - asset2.created_by = current_user - asset2.team = dest.team - asset2.last_modified_by = current_user - asset2.file_processing = true if asset.image? + asset2 = asset.dup asset2.save! - step2.assets << asset2 assets_to_clone << [asset.id, asset2.id] end @@ -348,10 +314,7 @@ class Protocol < ApplicationRecord step.clone_tinymce_assets(step2, dest.team) end # Call clone helper - Protocol.delay(queue: :assets).deep_clone_assets( - assets_to_clone, - dest.team - ) + Protocol.delay(queue: :assets).deep_clone_assets(assets_to_clone) end def in_repository_active? diff --git a/app/models/repository_asset_value.rb b/app/models/repository_asset_value.rb index a8bb65de2..e1eb6e4c3 100644 --- a/app/models/repository_asset_value.rb +++ b/app/models/repository_asset_value.rb @@ -16,11 +16,11 @@ class RepositoryAssetValue < ApplicationRecord validates :asset, :repository_cell, presence: true def formatted - asset.file_file_name + asset.file_name end def data - asset.file_file_name + asset.file_name end def data_changed?(_new_data) @@ -28,9 +28,8 @@ class RepositoryAssetValue < ApplicationRecord end def update_data!(new_data, user) - file = Paperclip.io_adapters.for(new_data[:file_data]) file.original_filename = new_data[:file_name] - asset.file = file + asset.file.attach(io: new_data[:file_data], filename: new_data[:file_name]) asset.last_modified_by = user self.last_modified_by = user asset.save! && save! diff --git a/app/models/team.rb b/app/models/team.rb index dbc88f7da..ccd350add 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -63,124 +63,6 @@ class Team < ApplicationRecord .where('full_name ILIKE ? OR email ILIKE ?', a_query, a_query) end - # Imports samples into db - # -1 == sample_name, - # -2 == sample_type, - # -3 == sample_group - # TODO: use constants - def import_samples(sheet, mappings, user) - errors = false - nr_of_added = 0 - total_nr = 0 - header_skipped = false - - # First let's query for all custom_fields we're refering to - custom_fields = [] - sname_index = -1 - stype_index = -1 - sgroup_index = -1 - mappings.each.with_index do |(_, v), i| - if v == '-1' - # Fill blank space, so our indices stay the same - custom_fields << nil - sname_index = i - elsif v == '-2' - custom_fields << nil - stype_index = i - elsif v == '-3' - custom_fields << nil - sgroup_index = i - else - cf = CustomField.find_by_id(v) - - # Even if doesn't exist we add nil value in order not to destroy our - # indices - custom_fields << cf - end - end - - rows = SpreadsheetParser.spreadsheet_enumerator(sheet) - - # Now we can iterate through sample data and save stuff into db - rows.each do |row| - # Skip empty rows - next if row.empty? - unless header_skipped - header_skipped = true - next - end - total_nr += 1 - row = SpreadsheetParser.parse_row(row, sheet) - - sample = Sample.new(name: row[sname_index], - team: self, - user: user) - - sample.transaction do - unless sample.valid? - errors = true - raise ActiveRecord::Rollback - end - - row.each.with_index do |value, index| - next unless value.present? - if index == stype_index - stype = SampleType.where(team: self) - .where('name ILIKE ?', value.strip) - .take - - unless stype - stype = SampleType.new(name: value.strip, team: self) - unless stype.save - errors = true - raise ActiveRecord::Rollback - end - end - sample.sample_type = stype - elsif index == sgroup_index - sgroup = SampleGroup.where(team: self) - .where('name ILIKE ?', value.strip) - .take - - unless sgroup - sgroup = SampleGroup.new(name: value.strip, team: self) - unless sgroup.save - errors = true - raise ActiveRecord::Rollback - end - end - sample.sample_group = sgroup - elsif value && custom_fields[index] - # we're working with CustomField - scf = SampleCustomField.new( - sample: sample, - custom_field: custom_fields[index], - value: value - ) - unless scf.valid? - errors = true - raise ActiveRecord::Rollback - end - sample.sample_custom_fields << scf - end - end - if Sample.import([sample], - recursive: true, - validate: false).failed_instances.any? - errors = true - raise ActiveRecord::Rollback - end - nr_of_added += 1 - end - end - - if errors - return { status: :error, nr_of_added: nr_of_added, total_nr: total_nr } - else - return { status: :ok, nr_of_added: nr_of_added, total_nr: total_nr } - end - end - def to_csv(samples, headers) require "csv" diff --git a/app/models/team_zip_export.rb b/app/models/team_zip_export.rb index 742ab5294..b741365a7 100644 --- a/app/models/team_zip_export.rb +++ b/app/models/team_zip_export.rb @@ -147,7 +147,7 @@ class TeamZipExport < ZipExport .routes .url_helpers .zip_exports_download_export_all_path(self)}'>" \ - "#{zip_file_file_name}" + "#{zip_file_name}" ) UserNotification.create(notification: notification, user: user) end @@ -197,11 +197,9 @@ class TeamZipExport < ZipExport if type == :step name = "#{directory}/" \ - "#{append_file_suffix(asset.file_file_name, - "_#{i}_Step#{element.step.position_plus_one}")}" + "#{append_file_suffix(asset.file_name, "_#{i}_Step#{element.step.position_plus_one}")}" elsif type == :result - name = "#{directory}/#{append_file_suffix(asset.file_file_name, - "_#{i}")}" + name = "#{directory}/#{append_file_suffix(asset.file_name, "_#{i}")}" end asset.file.copy_to_local_file(:original, name) if asset.file.exists? asset_indexes[asset.id] = name @@ -254,8 +252,7 @@ class TeamZipExport < ZipExport assets = {} asset_counter = 0 handle_name_func = lambda do |asset| - file_name = append_file_suffix(asset.file_file_name, - "_#{asset_counter}").to_s + file_name = append_file_suffix(asset.file_name, "_#{asset_counter}").to_s # Save pair for downloading it later assets[asset] = "#{attach_path}/#{file_name}" diff --git a/app/models/tiny_mce_asset.rb b/app/models/tiny_mce_asset.rb index 701f7e825..6f56d307a 100644 --- a/app/models/tiny_mce_asset.rb +++ b/app/models/tiny_mce_asset.rb @@ -70,6 +70,26 @@ class TinyMceAsset < ApplicationRecord description.css('body').inner_html.to_s end + def file_name + return '' unless image.attached? + + image.blob&.filename&.to_s + end + + def file_size + return 0 unless image.attached? + + image.blob&.byte_size + end + + def content_type + image&.blob&.content_type + end + + def file_size + image&.blob&.byte_size + end + def preview image.variant(resize: Constants::LARGE_PIC_FORMAT) end @@ -122,8 +142,7 @@ class TinyMceAsset < ApplicationRecord asset_guid = get_guid(tiny_mce_asset.id) asset_file_name = "rte-#{asset_guid.to_s + File.extname(tiny_mce_asset.image.filename.to_s)}" ostream.put_next_entry("#{dir}/#{asset_file_name}") - input_file = tiny_mce_asset.image.download - ostream.print(input_file.read) + ostream.print(tiny_mce_asset.image.download) input_file.close end end diff --git a/app/models/user.rb b/app/models/user.rb index d7a6a215a..9698b474a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -229,11 +229,6 @@ class User < ApplicationRecord foreign_key: :resource_owner_id, dependent: :delete_all - # If other errors besides parameter "avatar" exist, - # they will propagate to "avatar" also, so remove them - # and put all other (more specific ones) in it - # after_validation :filter_paperclip_errors - before_destroy :destroy_notifications def name @@ -334,20 +329,6 @@ class User < ApplicationRecord self.avatar_file_size = size.to_i end - def filter_paperclip_errors - if errors.key? :avatar - errors.delete(:avatar) - messages = [] - errors.each do |attribute| - errors.full_messages_for(attribute).each do |message| - messages << message.split(' ').drop(1).join(' ') - end - end - errors.clear - errors.add(:avatar, messages.join(',')) - end - end - # Whether user is active (= confirmed) or not def active? confirmed_at.present? diff --git a/app/models/zip_export.rb b/app/models/zip_export.rb index 27ede12ef..6706662a8 100644 --- a/app/models/zip_export.rb +++ b/app/models/zip_export.rb @@ -48,6 +48,12 @@ class ZipExport < ApplicationRecord export&.destroy end + def zip_file_name + return '' unless file.attached? + + zip_file.blob&.filename&.to_s + end + def generate_exportable_zip(user, data, type, options = {}) I18n.backend.date_format = user.settings[:date_format] || Constants::DEFAULT_DATE_FORMAT zip_input_dir = FileUtils.mkdir_p(File.join(Rails.root, "tmp/temp_zip_#{Time.now.to_i}")).first @@ -96,7 +102,7 @@ class ZipExport < ApplicationRecord .routes .url_helpers .zip_exports_download_path(self)}'>" \ - "#{zip_file_file_name}" + "#{zip_file_name}" ) UserNotification.create(notification: notification, user: user) end diff --git a/app/serializers/api/v1/repository_asset_value_serializer.rb b/app/serializers/api/v1/repository_asset_value_serializer.rb index ad7eb2458..a64c49532 100644 --- a/app/serializers/api/v1/repository_asset_value_serializer.rb +++ b/app/serializers/api/v1/repository_asset_value_serializer.rb @@ -10,11 +10,11 @@ module Api end def file_name - object.asset&.file_file_name + object.asset&.file_name end def file_size - object.asset&.file_file_size + object.asset&.file_size end def url diff --git a/app/serializers/api/v1/result_asset_serializer.rb b/app/serializers/api/v1/result_asset_serializer.rb index 8e24e9a3b..d86e999f2 100644 --- a/app/serializers/api/v1/result_asset_serializer.rb +++ b/app/serializers/api/v1/result_asset_serializer.rb @@ -11,11 +11,11 @@ module Api end def file_name - object.asset&.file_file_name + object.asset&.file_name end def file_size - object.asset&.file_file_size + object.asset&.file_size end def url diff --git a/app/services/repository_actions/duplicate_cell.rb b/app/services/repository_actions/duplicate_cell.rb index 554638226..e3f717fa1 100644 --- a/app/services/repository_actions/duplicate_cell.rb +++ b/app/services/repository_actions/duplicate_cell.rb @@ -43,7 +43,7 @@ module RepositoryActions def duplicate_repository_asset_value old_value = @cell.value - new_asset = create_new_asset(old_value.asset) + new_asset = old_value.asset.duplicate RepositoryAssetValue.create( old_value.attributes.merge( id: nil, asset: new_asset, created_by: @user, last_modified_by: @user, @@ -67,30 +67,5 @@ module RepositoryActions ) ) end - - # reuses the same code we have in copy protocols action - def create_new_asset(old_asset) - new_asset = Asset.new_empty( - old_asset.file_file_name, - old_asset.file_file_size - ) - 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.image? - new_asset.file = old_asset.file - new_asset.save - - return unless new_asset.valid? - - if new_asset.image? - new_asset.file.reprocess!(:large) - new_asset.file.reprocess!(:medium) - end - - new_asset.post_process_file(new_asset.team) - - new_asset - end end end diff --git a/app/services/repository_datatable_service.rb b/app/services/repository_datatable_service.rb index 2e2887230..9a5e55160 100644 --- a/app/services/repository_datatable_service.rb +++ b/app/services/repository_datatable_service.rb @@ -171,12 +171,18 @@ class RepositoryDatatableService def filter_by_asset_value(records, id, dir) records.joins( "LEFT OUTER JOIN (SELECT repository_cells.repository_row_id, - assets.file_file_name AS value + active_storage_blobs.filename AS value FROM repository_cells INNER JOIN repository_asset_values ON repository_asset_values.id = repository_cells.value_id INNER JOIN assets ON repository_asset_values.asset_id = assets.id + INNER JOIN active_storage_attachments + ON active_storage_attachments.record_id = assets.id + AND active_storage_attachments.record_type = 'Asset' + AND active_storage_attachments.name = 'file' + INNER JOIN active_storage_blobs + ON active_storage_blobs.id = active_storage_attachments.blob_id WHERE repository_cells.repository_column_id = #{id}) AS values ON values.repository_row_id = repository_rows.id" ).order("values.value #{dir}") diff --git a/app/services/spreadsheet_parser.rb b/app/services/spreadsheet_parser.rb index ab2c54235..ef36175f4 100644 --- a/app/services/spreadsheet_parser.rb +++ b/app/services/spreadsheet_parser.rb @@ -4,11 +4,6 @@ class SpreadsheetParser filename = file.original_filename file_path = file.path - if file.class == Paperclip::Attachment && file.is_stored_on_s3? - fa = file.fetch - file_path = fa.path - end - case File.extname(filename) when '.csv' Roo::CSV.new(file_path, extension: :csv) diff --git a/app/services/team_importer.rb b/app/services/team_importer.rb index 74ed1dfcb..ecb3c7dc1 100644 --- a/app/services/team_importer.rb +++ b/app/services/team_importer.rb @@ -340,8 +340,8 @@ class TeamImporter tiny_mce_asset.object_id = mappings[tiny_mce_asset.object_id] end tiny_mce_asset.team = team - tiny_mce_asset.image = tiny_mce_file tiny_mce_asset.save! + tiny_mce_asset.image.attach(io: tiny_mce_file, filename: tiny_mce_file.basename) @mce_asset_counter += 1 if tiny_mce_asset.object_id.present? object = tiny_mce_asset.object @@ -794,7 +794,7 @@ class TeamImporter def create_asset(asset_json, team, user_id = nil) asset = Asset.new(asset_json) File.open( - "#{@import_dir}/assets/#{asset.id}/#{asset.file_file_name}" + "#{@import_dir}/assets/#{asset.id}/#{asset.file_name}" ) do |file| orig_asset_id = asset.id asset.id = nil @@ -802,9 +802,9 @@ class TeamImporter asset.last_modified_by_id = user_id || find_user(asset.last_modified_by_id) asset.team = team - asset.file = file asset.in_template = true if @is_template asset.save! + asset.file.attach(io: file, filename: file.basename) asset.post_process_file(team) @asset_mappings[orig_asset_id] = asset.id @asset_counter += 1 diff --git a/app/utilities/protocols_exporter.rb b/app/utilities/protocols_exporter.rb index cb402e2dc..f63d31246 100644 --- a/app/utilities/protocols_exporter.rb +++ b/app/utilities/protocols_exporter.rb @@ -51,12 +51,11 @@ module ProtocolsExporter next unless img img_guid = get_guid(img.id) - asset_file_name = "rte-#{img_guid}" \ - "#{File.extname(img.image_file_name)}" + asset_file_name = "rte-#{img_guid}#{File.extname(img.file_name)}" asset_xml = "\n" - asset_xml << "#{img.image_file_name}\n" - asset_xml << "#{img.image_content_type}\n" + asset_xml << "#{img.file_name}\n" + asset_xml << "#{img.content_type}\n" asset_xml << "\n" tiny_assets_xml << asset_xml end @@ -100,11 +99,11 @@ module ProtocolsExporter step.assets.order(:id).each do |asset| asset_guid = get_guid(asset.id) asset_file_name = "#{asset_guid}" \ - "#{File.extname(asset.file_file_name)}" + "#{File.extname(asset.file_name)}" asset_xml = "\n" - asset_xml << "#{asset.file_file_name}\n" - asset_xml << "#{asset.file_content_type}\n" + asset_xml << "#{asset.file_name}\n" + asset_xml << "#{asset.content_type}\n" asset_xml << "\n" step_xml << asset_xml end diff --git a/app/utilities/protocols_importer.rb b/app/utilities/protocols_importer.rb index d095a12d7..00726582d 100644 --- a/app/utilities/protocols_importer.rb +++ b/app/utilities/protocols_importer.rb @@ -111,9 +111,7 @@ module ProtocolsImporter ) # Decode the file bytes - asset.file = StringIO.new(Base64.decode64(asset_json['bytes'])) - asset.file_file_name = asset_json['fileName'] - asset.file_content_type = asset_json['fileType'] + asset.file.attach(io: StringIO.new(Base64.decode64(asset_json['bytes'])), filename: asset_json['fileName']) asset.save! asset_ids << asset.id @@ -152,12 +150,11 @@ module ProtocolsImporter team_id: team.id, saved: true ) - # Decode the file bytes - tiny_mce_img.image = StringIO.new( - Base64.decode64(tiny_mce_img_json['bytes']) - ) - tiny_mce_img.image_content_type = tiny_mce_img_json['fileType'] tiny_mce_img.save! + + # Decode the file bytes + tiny_mce_img.image.attach(io: StringIO.new(Base64.decode64(tiny_mce_img_json['bytes'])), + filename: tiny_mce_img_json['fileName']) if description.gsub!("data-mce-token=\"#{tiny_mce_img_json['tokenId']}\"", "data-mce-token=\"#{Base62.encode(tiny_mce_img.id)}\"") else diff --git a/app/utilities/wopi_util.rb b/app/utilities/wopi_util.rb index eabeaa693..2ed5ebb9e 100644 --- a/app/utilities/wopi_util.rb +++ b/app/utilities/wopi_util.rb @@ -83,7 +83,7 @@ module WopiUtil default_step_items = { step: @asset.step.id, step_position: { id: @asset.step.id, value_for: 'position_plus_one' }, - asset_name: { id: @asset.id, value_for: 'file_file_name' }, + asset_name: { id: @asset.id, value_for: 'file_name' }, action: action } if @protocol.in_module? project = @protocol.my_module.experiment.project @@ -113,7 +113,7 @@ module WopiUtil project: @my_module.experiment.project, message_items: { result: @asset.result.id, - asset_name: { id: @asset.id, value_for: 'file_file_name' }, + asset_name: { id: @asset.id, value_for: 'file_name' }, action: action }) end diff --git a/app/views/assets/edit.erb b/app/views/assets/edit.erb index 17e71dc25..f4617301f 100644 --- a/app/views/assets/edit.erb +++ b/app/views/assets/edit.erb @@ -6,7 +6,7 @@ - <%= t('assets.head_title.edit', file_name: @asset.file_file_name) %> + <%= t('assets.head_title.edit', file_name: @asset.file_name) %> diff --git a/app/views/assets/view.erb b/app/views/assets/view.erb index e20aa03ff..1bd84e2a2 100644 --- a/app/views/assets/view.erb +++ b/app/views/assets/view.erb @@ -6,7 +6,7 @@ - <%= t('assets.head_title.view', file_name: @asset.file_file_name) %> + <%= t('assets.head_title.view', file_name: @asset.file_name) %> 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 3cbb13b6e..e934d8adf 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 @@ -24,7 +24,7 @@ <% else %> <%=t "projects.reports.elements.result_asset.file_name", - file: truncate(asset.file_file_name, + file: truncate(asset.file_name, length: Constants::FILENAME_TRUNCATION_LENGTH) %> <% end %>
diff --git a/app/views/reports/elements/_step_asset_element.html.erb b/app/views/reports/elements/_step_asset_element.html.erb index b84d9a61b..ef768927b 100644 --- a/app/views/reports/elements/_step_asset_element.html.erb +++ b/app/views/reports/elements/_step_asset_element.html.erb @@ -2,7 +2,7 @@ <% is_image = asset.image? %> <% timestamp = asset.created_at %> <% icon_class = 'fas ' + (is_image ? 'fa-image' : 'fa-file') %> -
" data-icon-class="<%= icon_class %>"> +
" data-icon-class="<%= icon_class %>">
@@ -16,7 +16,7 @@ <% else %> <%=t 'projects.reports.elements.step_asset.file_name', - file: truncate(asset.file_file_name, + file: truncate(asset.file_name, length: Constants::FILENAME_TRUNCATION_LENGTH) %> <% end %>
diff --git a/app/views/result_assets/_edit.html.erb b/app/views/result_assets/_edit.html.erb index e35b55239..7d1bc574d 100644 --- a/app/views/result_assets/_edit.html.erb +++ b/app/views/result_assets/_edit.html.erb @@ -5,7 +5,7 @@ <%=t "result_assets.edit.uploaded_asset" %>

<%= file_extension_icon(ff.object) %> - <%= ff.object.file_file_name %> + <%= ff.object.file_name %>

<%= ff.file_field :file %> <% end %> diff --git a/app/views/search/results/partials/_asset_text.html.erb b/app/views/search/results/partials/_asset_text.html.erb index f5a483393..04de9d822 100644 --- a/app/views/search/results/partials/_asset_text.html.erb +++ b/app/views/search/results/partials/_asset_text.html.erb @@ -1,6 +1,6 @@ <% query ||= nil %> <% asset_read_allowed = false %> -<% text = query.present? ? highlight(asset.file_file_name, query.strip.split(/\s+/)) : asset.file_file_name %> +<% text = query.present? ? highlight(asset.file_name, query.strip.split(/\s+/)) : asset.file_name %> <% if asset.step %> <% protocol = asset.step.protocol %> diff --git a/config/environments/production.rb b/config/environments/production.rb index 5df50d3fc..676fca7c2 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -48,7 +48,7 @@ Rails.application.configure do # Compress JavaScripts and CSS. - config.assets.js_compressor = :uglifier + config.assets.js_compressor = Uglifier.new(harmony: true) # config.assets.css_compressor = :sass # Do not fallback to assets pipeline if a precompiled asset is missed. @@ -71,6 +71,8 @@ Rails.application.configure do # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies. config.force_ssl = ENV['RAILS_FORCE_SSL'].present? + config.ssl_options = { redirect: { exclude: ->(request) { request.path =~ %r{api\/health} } } } + # Use the lowest log level to ensure availability of diagnostic information # when problems arise. config.log_level = :debug diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index 2e79014d1..4aac4bfff 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -55,12 +55,12 @@ class Extends # are only supported REPOSITORY_EXTRA_SEARCH_ATTR = ['repository_text_values.data', 'repository_list_items.data', - 'assets.file_file_name'] + 'active_storage_blobs.filename'] # Array of includes used in search query for repository rows REPOSITORY_SEARCH_INCLUDES = [:repository_text_value, repository_list_value: :repository_list_item, - repository_asset_value: :asset] + repository_asset_value: { asset: { file_attachment: :blob } }] # List of implemented core API versions API_VERSIONS = ['v1'] diff --git a/config/locales/en.yml b/config/locales/en.yml index 536da4a75..14407e7b5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1262,7 +1262,6 @@ en: scinote_columns_html: "SciNote columns:" file_columns: "Imported columns:" example_value: "Imported file content:" - import_samples: "Import samples" do_not_include_column: "Do not include this column" errors: invalid_file: "The file you provided is invalid. Make sure the file is encoded using %{encoding}." diff --git a/config/routes.rb b/config/routes.rb index a9a665a6a..6f5bf3cdb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -192,8 +192,6 @@ Rails.application.routes.draw do # end member do post 'parse_sheet', defaults: { format: 'json' } - # post 'import_samples' - # post 'export_samples' post 'export_repository', to: 'repositories#export_repository' post 'export_projects' get 'export_projects_modal' diff --git a/docker-compose.production.yml b/docker-compose.production.yml index 0877637cb..f8dbfa157 100644 --- a/docker-compose.production.yml +++ b/docker-compose.production.yml @@ -37,7 +37,9 @@ services: - RAILS_ENV=production volumes: - scinote_production_files:/usr/src/app/public/system + - scinote_production_storage:/usr/src/app/storage volumes: scinote_production_postgres: scinote_production_files: + scinote_production_storage: