Merge pull request #1756 from Ducz0r/lm-sci-3426-fix-2

Refactor WOPI filename length validation onto the model [SCI-3426]

app/controllers/assets_controller.rb

@@ -204,25 +204,11 @@ class AssetsController < ApplicationController
     render_403 && return unless %w(docx xlsx pptx).include?(params[:file_type])
 
     # Asset validation
-    original_filename = "#{params[:file_name]}.#{params[:file_type]}"
     file = Paperclip.io_adapters.for(StringIO.new)
-    file.original_filename = original_filename
+    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)
 
-    # Filename length validation (this cannot be checked by Paperclip,
-    # as it depends on OS)
-    if original_filename.length > Constants::FILENAME_MAX_LENGTH
-      render json: {
-        message: {
-          file: I18n.t(
-            'assets.create_wopi_file.errors.file_name_too_long',
-            limit: Constants::FILENAME_MAX_LENGTH
-          )
-        }
-      }, status: 400 and return
-    end
-
     unless asset.valid?(:wopi_file_creation)
       render json: {
         message: asset.errors

app/models/asset.rb

@@ -49,7 +49,7 @@ class Asset < ApplicationRecord
   # This could cause some problems if you create empty asset and want to
   # assign it to result
   validate :step_or_result_or_repository_asset_value
-  validate :name_should_not_be_empty_without_extension,
+  validate :wopi_filename_valid,
            on: :wopi_file_creation
 
   belongs_to :created_by,
@@ -521,13 +521,24 @@ class Asset < ApplicationRecord
     end
   end
 
-  def name_should_not_be_empty_without_extension
+  def wopi_filename_valid
+    # Check that filename without extension is not blank
     unless file.original_filename[0..-6].present?
       errors.add(
         :file,
         I18n.t('general.text.not_blank')
       )
     end
+    # Check maximum filename length
+    if file.original_filename.length > Constants::FILENAME_MAX_LENGTH
+      errors.add(
+        :file,
+        I18n.t(
+          'general.file.file_name_too_long',
+          limit: Constants::FILENAME_MAX_LENGTH
+        )
+      )
+    end
   end
 
   def cache

config/locales/en.yml

@@ -1916,7 +1916,6 @@ en:
       errors:
         forbidden: 'You do not have permission to add files.'
         not_found: 'Element not found.'
-        file_name_too_long: 'is too long (maximum is %{limit} characters, with extension)'
   atwho:
     no_results: "No results found"
     users:
@@ -1983,6 +1982,7 @@ en:
       blank: "You didn't select any file"
       uploading: "If you leave this page, the file(s) that is/are currently uploading will not be saved! Are you sure you want to continue?"
       upload_failure: "Upload connection error. Try again or contact the administrator."
+      file_name_too_long: 'is too long (maximum is %{limit} characters, with extension)'
     text:
       not_blank: "can't be blank"
       length_too_long_general: "is too long"