Browse Source

Fixes #4857 - Function test when saving the S3 backend in the admin interface is missing.
Fixes #4873 - S3: Lack of error and status messages.

Co-authored-by: Dominik Klein <dk@zammad.com>
Co-authored-by: Florian Liebe <fl@zammad.com>
Co-authored-by: Tobias Schäfer <ts@zammad.com>

Tobias Schäfer 1 year ago
parent
commit
a5429e9926

+ 8 - 1
app/assets/javascripts/app/controllers/_settings/area_item.coffee

@@ -59,4 +59,11 @@ class App.SettingsAreaItem extends App.Controller
       value = params
     else
       value = directData
-    App.Setting.set(@setting['name'], value, doneLocal: => @formEnable(e))
+    App.Setting.set(
+      @setting['name'],
+      value,
+      {
+        doneLocal: => @formEnable(e),
+        failLocal: => @formEnable(e)
+      }
+    )

+ 1 - 1
app/assets/javascripts/app/controllers/_settings/area_switch.coffee

@@ -91,6 +91,6 @@ class App.SettingsAreaSwitch extends App.Controller
         App.Event.trigger 'notify', {
           type:    'error'
           msg:     App.i18n.translateContent(details.error_human || details.error || __('The object could not be updated.'))
-          timeout: 2000
+          timeout: 3000
         }
     )

+ 4 - 1
app/assets/javascripts/app/lib/app_post/html5_upload.coffee

@@ -76,6 +76,9 @@ class App.Html5Upload extends App.Controller
     @hideFileUploading()
     @inputField.val('')
 
+    if message
+      message = JSON.parse(message)
+
     @callbackFileUploadStop?()
 
     new App.ControllerModal(
@@ -83,7 +86,7 @@ class App.Html5Upload extends App.Controller
       buttonCancel: 'Cancel'
       buttonCancelClass: 'btn--danger'
       buttonSubmit: false
-      message: message || __('The file could not be uploaded.')
+      message: message['error_human'] || message['error'] || __('The file could not be uploaded.')
       shown: true
       small: true
       container: @inputField.closest('.content')

+ 1 - 1
app/assets/javascripts/app/lib/base/html5Upload.js

@@ -244,7 +244,7 @@
                 // call the error callback when the status is not ok
                 if (xhr.status !== 200){
                   console.log('Upload failed: ' + fileName);
-                  upload.events.onError(event.target.statusText);
+                  upload.events.onError(event.target.responseText);
                 } else {
                   console.log('Upload completed: ' + fileName);
                   upload.events.onCompleted(event.target.responseText);

+ 2 - 2
app/assets/javascripts/app/models/setting.coffee

@@ -29,7 +29,7 @@ class App.Setting extends App.Model
         App.Event.trigger 'notify', {
           type:    'error'
           msg:     App.i18n.translateContent(details?.error_human || details?.error || __('The setting could not be updated.'))
-          timeout: 2000
+          timeout: 3000
         }
         if options.failLocal
           options.failLocal(@)
@@ -66,7 +66,7 @@ class App.Setting extends App.Model
         App.Event.trigger 'notify', {
           type:    'error'
           msg:     App.i18n.translateContent(given_error || __('The setting could not be reset.'))
-          timeout: 2000
+          timeout: 3000
         }
     )
 

+ 2 - 2
app/assets/javascripts/app/views/settings/storage_provider.jst.eco

@@ -6,8 +6,8 @@
     <dd><%- @T('Attachments are stored in the filesystem.') %></dd>
     <dt><%- @T('Database') %><code>DB</code></dt>
     <dd><%- @T('Attachments are stored in the database.') %></dd>
-    <dt><%- @T('Simple Storage (S3)') %><code>S3</code></dt>
-    <dd><%- @T('Attachments are stored in a Simple Storage Service.') %></dd>
+    <dt><%- @T('Simple Storage Service (S3)') %><code>S3</code></dt>
+    <dd><%- @T('Attachments are stored in a Simple Storage Service.') %><br /><%- @T('For more details, please check out our online documentation %l.', 'https://admin-docs.zammad.org/en/latest/settings/system/storage.html') %></dd>
   </dl>
   <p class="help-text"><%- @T('If you want to move already stored attachments from one backend to another, you need to execute the following via console.') %></p>
   <code>

+ 3 - 4
app/controllers/application_controller/handles_errors.rb

@@ -15,6 +15,7 @@ module ApplicationController::HandlesErrors
     rescue_from Exceptions::NotAuthorized, with: :unauthorized
     rescue_from Exceptions::Forbidden, with: :forbidden
     rescue_from Pundit::NotAuthorizedError, with: :pundit_not_authorized_error
+    rescue_from Store::Provider::S3::Error, with: :unprocessable_entity
   end
 
   def not_found(e)
@@ -87,9 +88,7 @@ module ApplicationController::HandlesErrors
   end
 
   def humanize_error(e)
-    data = {
-      error: e.message
-    }
+    data = { error: e.message }
 
     if (base_error = e.try(:record)&.errors&.messages&.find { |key, _| key.match? %r{[\w+.]?base} }&.last&.last)
       data[:error_human] = base_error
@@ -105,7 +104,7 @@ module ApplicationController::HandlesErrors
     elsif e.message == 'Exceptions::NotAuthorized'
       data[:error]       = __('Authorization failed')
       data[:error_human] = data[:error]
-    elsif [ActionController::RoutingError, ActiveRecord::RecordNotFound, Exceptions::UnprocessableEntity, Exceptions::NotAuthorized, Exceptions::Forbidden].include?(e.class)
+    elsif [ActionController::RoutingError, ActiveRecord::RecordNotFound, Exceptions::UnprocessableEntity, Exceptions::NotAuthorized, Exceptions::Forbidden, Store::Provider::S3::Error].include?(e.class)
       data[:error_human] = data[:error]
     end
 

+ 8 - 4
app/frontend/apps/mobile/components/Form/fields/FieldFile/FieldFileInput.vue

@@ -66,10 +66,14 @@ const loadFiles = async (files: FileList | File[]) => {
 
   const uploads = await convertFileList(files)
 
-  const data = await addFileMutation.send({
-    formId: props.context.formId,
-    files: uploads,
-  })
+  const data = await addFileMutation
+    .send({
+      formId: props.context.formId,
+      files: uploads,
+    })
+    .catch(() => {
+      reset()
+    })
 
   const uploadedFiles = data?.formUploadCacheAdd?.uploadedFiles
 

+ 14 - 2
app/frontend/shared/server/apollo/handler/BaseHandler.ts

@@ -32,7 +32,7 @@ export default abstract class BaseHandler<
 
   protected baseHandlerOptions: BaseHandlerOptions = {
     errorShowNotification: true,
-    errorNotificationMessage: __('An error occured during the operation.'),
+    errorNotificationMessage: '',
     errorNotificationType: NotificationTypes.Error,
   }
 
@@ -113,7 +113,7 @@ export default abstract class BaseHandler<
       //   console.error(error)
       // }
       useNotifications().notify({
-        message: options.errorNotificationMessage,
+        message: this.errorNotificationMessage(errorHandler.message),
         type: options.errorNotificationType,
       })
     }
@@ -128,4 +128,16 @@ export default abstract class BaseHandler<
       handlerOptions,
     ) as CommonHandlerOptions<THandlerOptions>
   }
+
+  private errorNotificationMessage(errorMessage?: string): string {
+    const defaultErrorNotificationMessage = __(
+      'An error occured during the operation.',
+    )
+
+    return (
+      this.handlerOptions.errorNotificationMessage ||
+      errorMessage ||
+      defaultErrorNotificationMessage
+    )
+  }
 }

+ 27 - 0
app/models/setting/validation/storage_provider.rb

@@ -0,0 +1,27 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Setting::Validation::StorageProvider < Setting::Validation
+
+  def run
+    return result_success if value.blank?
+
+    msg = verify_configuration
+    return result_failed(msg) if !msg.nil?
+
+    result_success
+  end
+
+  private
+
+  def verify_configuration
+    return if !value.eql?('S3')
+
+    begin
+      Store::Provider::S3.ping!
+    rescue Store::Provider::S3::Error => e
+      return e.message
+    end
+
+    nil
+  end
+end

Some files were not shown because too many files changed in this diff