Merge pull request #138 from mz3944/mz_result_edit_and_name_length_check_fix

Fixed result file deletion and result name updating [fixes SCI-409]. …
2025-03-10 06:37:32 +08:00 · 2016-09-07 11:07:23 +02:00 · 2016-09-07 11:07:23 +02:00 · d3a3dd4428
commit d3a3dd4428
parent 5f446ab1f0 b0ad6d0753
11 changed files with 55 additions and 37 deletions
--- a/app/assets/javascripts/direct-upload.js
+++ b/app/assets/javascripts/direct-upload.js
@ -189,14 +189,22 @@
      beforeUpload(ev);

      // Spoof checks files and, if OK, gets upload post requests
+      var anyFile = false;
      _.each($fileInputs, function (fileInput) {
        var file = fileInput.files[0];
        if (!_.isUndefined(file)) {
          signRequests.push(
            fetchUploadSignature(ev, fileInput, file, signUrl)
          );
+          anyFile = true;
        }
      });
+      if (!anyFile) {
+        // No files present
+        afterUpload();
+        $form.submit();
+        return;
+      }

      $.when.apply($, signRequests).then(function () {
        // After successful file spoof check and upload post requests fetching
--- a/app/assets/javascripts/my_modules/results.js
+++ b/app/assets/javascripts/my_modules/results.js
@ -320,7 +320,8 @@ var ResultTypeEnum = Object.freeze({
  COMMENT: 3
 });

-function processResult(ev, resultTypeEnum, forS3) {
+function processResult(ev, resultTypeEnum, editMode, forS3) {
+  forS3 = (typeof forS3 !== 'undefined') ? forS3 : false;
  var $form = $(ev.target.form);
  $form.clearFormErrors();

@ -329,7 +330,7 @@ function processResult(ev, resultTypeEnum, forS3) {
      var $nameInput = $form.find("#result_name");
      var nameValid = textValidator(ev, $nameInput, true);
      var $fileInput = $form.find("#result_asset_attributes_file");
-      var filesValid = filesValidator(ev, $fileInput, FileTypeEnum.FILE);
+      var filesValid = filesValidator(ev, $fileInput, FileTypeEnum.FILE, editMode);

      if(nameValid && filesValid) {
        if(forS3) {
--- a/app/assets/javascripts/sitewide/form_validators.js.erb
+++ b/app/assets/javascripts/sitewide/form_validators.js.erb
@ -18,7 +18,7 @@ $.fn.onSubmitValidator = function(validatorCb) {
  }
 };

-function textValidator(ev, textInput, canBeBlank) {
+function textValidator(ev, textInput, canBeBlank, limitLength) {
  canBeBlank = (typeof canBeBlank !== 'undefined') ? canBeBlank : false;
  limitLength = (typeof limitLength !== 'undefined') ? limitLength : true;
  var nameTooShort = $(textInput).val().length === 0;
@ -30,43 +30,49 @@ function textValidator(ev, textInput, canBeBlank) {
    errMsg = I18n.t("general.text.length_too_long", { max_length: 50 });
  }

-  var hasErrors = !_.isUndefined(errMsg);
-  if (hasErrors) {
+  var noErrors = _.isUndefined(errMsg);
+  if (!noErrors) {
    renderFormError(ev, $(textInput), errMsg);
  }
-  return !hasErrors;
+  return noErrors;
 }

 function checklistsValidator(ev, checklists, editMode) {
  var noErrors = true;
  // For every visible (i.e. not removed) checklist
-  checklists.each(function() {
-    $checklist = $(this);
+  $(checklists).each(function() {
+    var $checklist = $(this);
    if ($checklist.css('display') != 'none') {
-      var $checklistNameInput = $checklist.find(".checklist_name");
-      anyChecklistItemFilled = false;

-      // For every ckecklist item input
+      // For every visible (i.e. not removed) ckecklist item
+      anyChecklistItemFilled = false;
      $(" .checklist-item-text", $checklist).each(function() {
-        $checklistItemInput = $(this);
-        if ($checklistItemInput.val()) {
-          anyChecklistItemFilled = true;
-        } else {
-          // Remove empty checklist item input
-          $checklistItemInput.closest("fieldset").remove();
+        var $itemInput = $(this);
+        var $item = $itemInput.closest("fieldset");
+        if ($item.css('display') != 'none') {
+
+          if ($itemInput.val()) {
+            var itemNameValid = textValidator(ev, $itemInput);
+            if (!itemNameValid) {
+              noErrors = false;
+            }
+            anyChecklistItemFilled = true;
+          } else {
+            // Remove empty checklist item input
+            $item.remove();
+          }
        }
      })

-      if (!$checklistNameInput.val()) {
-        if (anyChecklistItemFilled || editMode) {
-            // In edit mode, checklist's name can't be blank
-            var errMsg = I18n.t("general.text.not_blank");
-            renderFormError(ev, $checklistNameInput, errMsg);
-            noErrors = false;
-        } else  {
-          // Hide empty checklist (remove would break logic)
-          $checklist.hide();
-        }
+      // In edit mode, checklist's name can't be blank if any items present
+      var allowBlankChklstName = !(anyChecklistItemFilled || editMode);
+      var $checklistNameInput = $checklist.find(".checklist_name");
+      var checklistNameValid = textValidator(ev, $checklistNameInput, allowBlankChklstName);
+      if (!checklistNameValid) {
+        noErrors = false;
+      } else if (allowBlankChklstName) {
+        // Hide empty checklist (remove would break server-side logic)
+        $checklist.hide();
      }
    }
  });
@ -79,10 +85,11 @@ var FileTypeEnum = Object.freeze({
  AVATAR: 1
 });

-function filesValidator(ev, fileInputs, fileTypeEnum) {
+function filesValidator(ev, fileInputs, fileTypeEnum, canBeEmpty) {
+  canBeEmpty = (typeof canBeEmpty !== 'undefined') ? canBeEmpty : false;
  var filesValid = true;
  if (fileInputs.length) {
-    var filesPresentValid = filesPresentValidator(ev, fileInputs);
+    var filesPresentValid = canBeEmpty || filesPresentValidator(ev, fileInputs);
    var filesSizeValid = filesSizeValidator(ev, fileInputs, fileTypeEnum);
    // File spoof check is done on server-side only
    filesValid = filesPresentValid && filesSizeValid;
--- a/app/controllers/steps_controller.rb
+++ b/app/controllers/steps_controller.rb
@ -553,7 +553,9 @@ class StepsController < ApplicationController

        for pos, attrs in params[key] do
          if attrs[:_destroy] == '1'
-            attr_params[pos] = { id: attrs[:id], _destroy: '1' }
+            if attrs[:id].present?
+              attr_params[pos] = { id: attrs[:id], _destroy: '1' }
+            end
            params[key].delete(pos)
          elsif has_destroy_params(params[key][pos])
            attr_params[pos] = { id: attrs[:id] }
--- a/app/views/result_assets/_edit.html.erb
+++ b/app/views/result_assets/_edit.html.erb
@ -7,7 +7,7 @@
      <%= ff.file_field :file %>
    <% end %>
    <hr>
-    <%= f.submit t("result_assets.edit.update"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.FILE, #{direct_upload});" %>
+    <%= f.submit t("result_assets.edit.update"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.FILE, true, #{direct_upload});" %>
    <button type="button" class="btn btn-default cancel-edit">
      <%= t("general.cancel")%>
    </button>
--- a/app/views/result_assets/_new.html.erb
+++ b/app/views/result_assets/_new.html.erb
@ -4,7 +4,7 @@
    <%= f.fields_for :asset do |ff| %>
      <%= ff.file_field :file %>
    <% end %>
-    <%= f.submit t("result_assets.new.create"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.FILE, #{direct_upload});" %>
+    <%= f.submit t("result_assets.new.create"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.FILE, false, #{direct_upload});" %>
    <button type="button" class="btn btn-default cancel-new">
      <%= t("general.cancel")%>
    </button>
--- a/app/views/result_comments/_index.html.erb
+++ b/app/views/result_comments/_index.html.erb
@ -19,7 +19,7 @@
    <li>
      <hr>
      <%= bootstrap_form_for :comment, url: { format: :json }, method: :post, remote: true do |f| %>
-        <%= f.text_field :message, hide_label: true, placeholder: t("general.comment_placeholder"), append: f.submit("+", onclick: "processResult(event, ResultTypeEnum.COMMENT);"), help: '.' %>
+        <%= f.text_field :message, hide_label: true, placeholder: t("general.comment_placeholder"), append: f.submit("+", onclick: "processResult(event, ResultTypeEnum.COMMENT, false);"), help: '.' %>
      <% end %>
    </li>
  </ul>
--- a/app/views/result_tables/_edit.html.erb
+++ b/app/views/result_tables/_edit.html.erb
@ -9,7 +9,7 @@
 			<% end %>
 		</div>
    <hr>
-		<%= f.submit t("result_tables.edit.update"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TABLE);" %>
+		<%= f.submit t("result_tables.edit.update"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TABLE, true);" %>
 		<button type="button" class="btn btn-default cancel-edit">
 			<%= t("general.cancel")%>
 		</button>
--- a/app/views/result_tables/_new.html.erb
+++ b/app/views/result_tables/_new.html.erb
@ -8,7 +8,7 @@
 				</div>
 			<% end %>
 		</div>
-		<%= f.submit t("result_tables.new.create"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TABLE);" %>
+		<%= f.submit t("result_tables.new.create"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TABLE, false);" %>
 		<button type="button" class="btn btn-default cancel-new">
 			<%= t("general.cancel")%>
 		</button>
--- a/app/views/result_texts/_edit.html.erb
+++ b/app/views/result_texts/_edit.html.erb
@ -5,7 +5,7 @@
      <%= ff.text_area :text, style: "margin-top: 10px;" %><br />
    <% end %>
    <hr>
-    <%= f.submit t("result_texts.edit.update"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TEXT);" %>
+    <%= f.submit t("result_texts.edit.update"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TEXT, true);" %>
    <button type="button" class="btn btn-default cancel-edit">
      <%= t("general.cancel")%>
    </button>
--- a/app/views/result_texts/_new.html.erb
+++ b/app/views/result_texts/_new.html.erb
@ -4,7 +4,7 @@
    <%= f.fields_for :result_text do |ff| %>
      <%= ff.text_area :text, style: "margin-top: 10px;" %><br />
    <% end %>
-    <%= f.submit t("result_texts.new.create"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TEXT);" %>
+    <%= f.submit t("result_texts.new.create"), class: 'btn btn-primary', onclick: "processResult(event, ResultTypeEnum.TEXT, false);" %>
    <button type="button" class="btn btn-default cancel-new">
      <%= t("general.cancel")%>
    </button>