Fix error messages display during file upload in steps and results [SCI-1235]

app/assets/javascripts/results/result_assets.js

@@ -14,6 +14,9 @@ $("#new-result-asset").on("ajax:success", function(e, data) {
   toggleResultEditButtons(false);
 
   $("#result_name").focus();
+}).on('ajax:error', function(e, xhr) {
+  $(this).renderFormErrors('result', xhr.responseJSON, true, e);
+  animateSpinner(null, false);
 });
 
 $("#new-result-asset").on("ajax:error", function(e, xhr, status, error) {
@@ -66,15 +69,8 @@ function formAjaxResultAsset($form) {
     initPreviewModal();
     Comments.initialize();
   })
-  .on("ajax:error", function(e, data) {
+  .on('ajax:error', function(e, xhr) {
-    // This check is here only because of remotipart bug, which returns
+    $form.renderFormErrors('result', xhr.responseJSON, true, e);
-    // HTML instead of JSON, go figure
-    var errors = '';
-    if (data.errors)
-      errors = data.errors;
-    else
-      errors = data.responseJSON.errors;
-    $form.renderFormErrors("result", errors, true, e);
     animateSpinner(null, false);
   });
 }

app/assets/javascripts/sitewide/form_errors.js

@@ -2,18 +2,85 @@
  * Define AJAX methods for handling errors on forms.
  */
 
+/*
+* Render errors specified in array of strings format (or string if
+* just one error) for a single form element.
+*
+* Show error message/s and mark error input (if errMsgs is defined)
+* and, if present, mark and show the tab where the error occured and
+* focus/scroll to the error input, if it is the first one to be
+* specified or if errMsgs is undefined.
+*
+* @param {string} errAttributes Span element (error) attributes
+* @param {boolean} clearErr Set clearErr to true if this is the only
+* error that can happen/show.
+*/
+var renderFormError = function(ev, input, errMsgs, clearErr, errAttributes) {
+  clearErr = _.isUndefined(clearErr) ? false : clearErr;
+  errAttributes = _.isUndefined(errAttributes) ? '' : ' ' + errAttributes;
+  var $form = $(input).closest('form');
+
+  if (!_.isUndefined(errMsgs)) {
+    if (clearErr) {
+      $form.clearFormErrors();
+    }
+
+    // Mark error form group
+    var $formGroup = $(input).closest('.form-group');
+    if (!$formGroup.hasClass('has-error')) {
+      $formGroup.addClass('has-error');
+    }
+
+    // Add error message/s
+    var errorText = ($.makeArray(errMsgs).map(function(m) {
+      return m.strToErrorFormat();
+    })).join('<br />');
+    var $errSpan = "<span class='help-block'" +
+      errAttributes + '>' + errorText + '</span>';
+    $formGroup.append($errSpan);
+  }
+
+  var $parent;
+  var $tab = $(input).closest('.tab-pane');
+  if ($tab.length) {
+    // Mark error tab
+    tabsPropagateErrorClass($form);
+    $parent = $tab;
+  } else {
+    $parent = $form;
+  }
+
+  // Focus and scroll to the error if it is the first (most upper) one
+  if ($parent.find('.form-group.has-error').length === 1 ||
+      _.isUndefined(errMsgs)) {
+    goToFormElement(input);
+  }
+
+  if (!_.isUndefined(ev)) {
+    // Don't submit form
+    ev.preventDefault();
+    ev.stopPropagation();
+  }
+};
+
 /*
  * Render errors specified in JSON format for many form elements.
  */
-$.fn.renderFormErrors = function (modelName, errors, clear, ev) {
+$.fn.renderFormErrors = function(modelName, errors, clear, ev) {
-  clear = (typeof clear !== 'undefined') ? clear : true;
+  clear = ((typeof clear) === 'undefined') ? true : clear;
   if (clear || _.isUndefined(clear)) {
     this.clearFormErrors();
   }
-
   var form = $(this);
-  $.each(errors, function (field, messages) {
+  $.each(errors, function(field, messages) {
-    $input = $(_.filter(form.find('input, select, textarea'), function (el) {
+    // Special exception for file uploads in steps and results
+    if (field === 'assets.file') {
+      field = 'assets_attributes';
+    } else if (field === 'asset.file') {
+      field = 'asset_attribute';
+    }
+    var types = 'input, file, select, textarea';
+    var $input = $(_.filter(form.find(types), function(el) {
       var name = $(el).attr('name');
       if (name) {
         return name.match(new RegExp(modelName + '\\[' + field + '\\(?'));
@@ -25,80 +92,22 @@ $.fn.renderFormErrors = function (modelName, errors, clear, ev) {
   });
 };
 
-/*
- * Render errors specified in array of strings format (or string if
- * just one error) for a single form element.
- *
- * Show error message/s and mark error input (if errMsgs is defined)
- * and, if present, mark and show the tab where the error occured and
- * focus/scroll to the error input, if it is the first one to be
- * specified or if errMsgs is undefined.
- *
- * @param {string} errAttributes Span element (error) attributes
- * @param {boolean} clearErr Set clearErr to true if this is the only
- * error that can happen/show.
- */
-var renderFormError = function (ev, input, errMsgs, clearErr, errAttributes) {
-  clearErr = _.isUndefined(clearErr) ? false : clearErr;
-  errAttributes = _.isUndefined(errAttributes) ? "" : " " + errAttributes;
-  $form = $(input).closest("form");
-
-  if (!_.isUndefined(errMsgs)) {
-    if (clearErr) {
-      $form.clearFormErrors();
-    }
-
-    // Mark error form group
-    $formGroup = $(input).closest(".form-group");
-    if (!$formGroup.hasClass("has-error")) {
-      $formGroup.addClass("has-error");
-    }
-
-    // Add error message/s
-    error_text = ($.makeArray(errMsgs).map(function (m) {
-      return m.strToErrorFormat();
-    })).join("<br />");
-    $errSpan = "<span class='help-block'" + errAttributes + ">" + error_text + "</span>";
-    $formGroup.append($errSpan);
-  }
-
-  $tab = $(input).closest(".tab-pane");
-  if ($tab.length) {
-    // Mark error tab
-    tabsPropagateErrorClass($form);
-    $parent = $tab;
-  } else {
-    $parent = $form;
-  }
-
-  // Focus and scroll to the error if it is the first (most upper) one
-  if ($parent.find(".form-group.has-error").length === 1 || _.isUndefined(errMsgs)) {
-    goToFormElement(input);
-  }
-
-  if(!_.isUndefined(ev)) {
-    // Don't submit form
-    ev.preventDefault();
-    ev.stopPropagation();
-  }
-}
-
 /*
  * If any of form tabs (if exist) has errors, mark it and
  * and show the first erroneous tab.
  */
 function tabsPropagateErrorClass($form) {
-  var $contents = $form.find("div.tab-pane");
+  var $contents = $form.find('div.tab-pane');
-  _.each($contents, function (tab) {
+  _.each($contents, function(tab) {
     var $tab = $(tab);
-    var $errorFields = $tab.find(".has-error");
+    var $errorFields = $tab.find('.has-error');
     if ($errorFields.length) {
-      var id = $tab.attr("id");
+      var id = $tab.attr('id');
       var navLink = $form.find("a[href='#" + id + "'][data-toggle='tab']");
       if (navLink.parent().length) {
-        navLink.parent().addClass("has-error");
+        navLink.parent().addClass('has-error');
       }
     }
   });
-  $form.find(".nav-tabs .has-error:first > a", $form).tab("show");
+  $form.find('.nav-tabs .has-error:first > a', $form).tab('show');
 }

app/controllers/result_assets_controller.rb

@@ -72,11 +72,7 @@ class ResultAssetsController < ApplicationController
           }, status: :ok
         end
       else
-        # This response is sent as 200 OK due to IE security error when
+        format.json { render json: @result.errors, status: :bad_request }
-        # accessing iframe content.
-        format.json do
-          render json: { status: 'error', errors: @result.errors }
-        end
       end
     end
   end

app/controllers/steps_controller.rb

@@ -97,11 +97,6 @@ class StepsController < ApplicationController
           status: :ok
         end
       else
-        # On error, delete the newly added files from S3, as they were
-        # uploaded on client-side (in case of client-side hacking of
-        # asset's signature response)
-        Asset.destroy_all(new_assets)
-
         format.json {
           render json: {
             html: render_to_string(partial: 'new.html.erb')