From c87ba5b45c878dd91c35a11c7679d0264e690754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Zrim=C5=A1ek?= Date: Tue, 9 Aug 2016 14:39:24 +0200 Subject: [PATCH] Multiple files upload handling and user experience improved. Spinner now also prevents user from accidentally leaving page. --- app/assets/javascripts/application.js | 21 ++- app/assets/javascripts/direct-upload.js | 178 +++++++++++------- app/assets/javascripts/my_modules/results.js | 2 +- app/assets/javascripts/protocols/steps.js | 13 +- .../sitewide/form_validators.js.erb | 34 ++-- .../javascripts/users/registrations/edit.js | 2 +- app/controllers/assets_controller.rb | 4 +- app/controllers/steps_controller.rb | 9 +- .../users/registrations_controller.rb | 2 + config/locales/en.yml | 49 +++-- 10 files changed, 188 insertions(+), 126 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 913a4182f..ec039d7ff 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -81,7 +81,7 @@ function animateLoading(start){ * Shows spinner if start is true or not given, hides it if false. * Optional parameter options for spin.js options. */ -function animateSpinner(el, start, options) { +function animateSpinner(el, start, options, msg) { // If overlaying the whole page, // put the spinner in the middle of the page var overlayPage = false; @@ -114,6 +114,10 @@ function animateSpinner(el, start, options) { $(".loading-overlay").remove(); } + busy = start; + if (busy && !_.isUndefined(msg)) { + busyMsg = msg; + } } /* @@ -163,3 +167,18 @@ $(document).ready(function(){ truncateLongString( $(this), 30); }); }); + +/* + * Prevents user from accidentally leaving page when + * server is busy and notifies him with a message. + */ +var busy = false; +var busyMsg = I18n.t("general.busy"); +window.onbeforeunload = function () { + if (busy) { + var currentMsg = busyMsg; + // Reset to default message + busyMsg = I18n.t("general.busy"); + return currentMsg; + } +} diff --git a/app/assets/javascripts/direct-upload.js b/app/assets/javascripts/direct-upload.js index 86ece445d..723c13e6f 100644 --- a/app/assets/javascripts/direct-upload.js +++ b/app/assets/javascripts/direct-upload.js @@ -1,5 +1,16 @@ (function (exports) { + var styleOptionRe = /(\d+)x(\d+)/i; + + function parseStyleOption(option) { + var m = option.match(styleOptionRe); + + return { + width: m && m[1] || 150, + height: m && m[2] || 150 + }; + } + // Edits (size, quality, parameters) image file for S3 server uploading function generateThumbnail(origFile, type, max_width, max_height, cb) { var img = new Image; @@ -21,7 +32,7 @@ size = this.width; offsetY = (this.height - this.width) / 2; } - if(type === "image/jpeg") { + if (type === "image/jpeg") { type = "image/jpg"; } @@ -35,10 +46,8 @@ } // This server checks if files are OK (correct file type, presence, - // size and spoofing) and generates posts for S3 server file uploading - // (each post for different size of the same file) - // We do this synchronically, because we need to verify all files - // before uploading them + // size and spoofing) and only then generates posts for S3 server + // file uploading (each post for different size of the same file) function fetchUploadSignature(file, signUrl, cb) { var csrfParam = $("meta[name=csrf-param]").attr("content"); var csrfToken = $("meta[name=csrf-token]").attr("content"); @@ -50,7 +59,6 @@ url : signUrl, type : 'POST', data : formData, - async : false, processData: false, contentType: false, complete : function(xhr) { @@ -65,7 +73,7 @@ } // Upload file to S3 server - function uploadData(postData, cb) { + function uploadFile(postData, cb) { var xhr = new XMLHttpRequest; var fd = new FormData(); var fields = postData.fields; @@ -80,94 +88,126 @@ if (xhr.readyState === 4) { // complete cb(); } else if (xhr.readyState == 0) { // connection error - cb(I18n.t("errors.upload")); + cb(I18n.t("general.file.upload_failure")); } } xhr.open("POST", url); xhr.send(fd); } - var styleOptionRe = /(\d+)x(\d+)/i; + // For each file proccesses its posts and uploads them + // If one post fails, the user is allowed to leave page, + // but other files are still being uplaoded because of + // asynchronous behaviour, so errors for other files may + // can still show afterwards + // TODO On S3 server uplaod error the other files that + // were already asynchronously uploaded remain, but + // should be deleted - this should generally not happen, + // because all files should fail to upload in such cases + // (connection error) + function uploadFiles(ev, fileInputs, datas, cb) { + var noErrors = true; + $.each(datas, function(fileIndex, data) { + var fileInput = fileInputs.get(fileIndex); + var file = fileInput.files[0]; - function parseStyleOption(option) { - var m = option.match(styleOptionRe); + function processPost(error) { + // File upload error handling + if (error) { + renderFormError(ev, fileInput, error); + noErrors = false; + animateSpinner(null, false); + return; + } - return { - width: m && m[1] || 150, - height: m && m[2] || 150 - }; + var postData = posts[postIndex]; + if (!postData) { + if (fileIndex === datas.length-1 && noErrors) { + // After successful file processing and uploading + $.each(datas, function(fileIndex, data) { + // Use file input to pass file info on submit + var fileInput = fileInputs.get(fileIndex); + cb(fileInput, data.asset_id); + }); + animateSpinner(null, false); + $(ev.target.form).submit(); + } + return; + } + postData.fileName = file.name; + postIndex++; + + if (postData.style_option) { + // Picture file + var styleSize = parseStyleOption(postData.style_option); + generateThumbnail(file, postData.mime_type, styleSize.width, + styleSize.height, function (blob) { + + postData.file = blob; + uploadFile(postData, processPost); + }); + } else { + // Other file + postData.file = file; + uploadFile(postData, processPost); + } + } + + var posts = data.posts; + var postIndex = 0; + processPost(); + }); } - // Validates files on this server and uploads them to S3 server + // Validates files on server and uploads them to S3 server + // + // First we validate files on server and generate post requests (fetchUploadSignature), + // if OK the post requests are used to uplaod files asyncronously to S3 (uploadFiles), + // and if successful the form is submitted, otherwise no file is saved exports.directUpload = function (ev, fileInputs, signUrl, cb) { var noErrors = true; - var inputPointer = 0; - animateSpinner(); + var inputsPosts = [] - function processFile () { - var fileInput = fileInputs.get(inputPointer); + function processFile(fileIndex) { + var fileInput = fileInputs.get(fileIndex); if (!fileInput || !fileInput.files[0]) { + // After file processing + if(fileIndex !== 0) { + if(noErrors) { + uploadFiles(ev, fileInputs, inputsPosts, cb); + } else { + animateSpinner(null, false); + } + } return; } + + if (fileIndex === 0) { + // Before file processing and uploading + animateSpinner(null, true, undefined, I18n.t("general.file.uploading")); + ev.preventDefault(); + ev.stopPropagation(); + } + var file = fileInput.files[0]; - inputPointer++; - fetchUploadSignature(file, signUrl, function (data) { - - function processError(errMsgs) { - renderFormError(ev, fileInput, errMsgs); - noErrors = false; - } - - function processPost(error) { - // File post error handling - if (error) { - processError(error); - } - - var postData = posts[postPosition]; - if (!postData) { - animateSpinner(null, false); - return; - } - postData.fileName = file.name; - postPosition += 1; - - if (postData.style_option) { - var styleSize = parseStyleOption(postData.style_option); - generateThumbnail(file, postData.mime_type, styleSize.width, - styleSize.height, function (blob) { - - postData.file = blob; - uploadData(postData, processPost); - }); - } else { - postData.file = file; - uploadData(postData, processPost); - } - } - // File signature error handling if (_.isUndefined(data)) { - processError(I18n.t("errors.upload")); + renderFormError(ev, fileInput, I18n.t("general.file.upload_failure")); + noErrors = false; } - if (data.status === "error") { - processError(jsonToValuesArray(data.errors)); + else if (data.status === "error") { + renderFormError(ev, fileInput, jsonToValuesArray(data.errors)); + noErrors = false; + } else { + inputsPosts.push(data); } - processFile(); - if(noErrors) { - // Use file input to pass file info on submit - cb(fileInput, data.asset_id); - - var posts = data.posts; - var postPosition = 0; - processPost(); - } + processFile(fileIndex+1); }); } - processFile(); + processFile(0); }; }(this)); diff --git a/app/assets/javascripts/my_modules/results.js b/app/assets/javascripts/my_modules/results.js index 9a1551639..5eaf30671 100644 --- a/app/assets/javascripts/my_modules/results.js +++ b/app/assets/javascripts/my_modules/results.js @@ -326,7 +326,7 @@ function processResult(ev, resultTypeEnum, forS3) { startFileUpload(ev, ev.target); } else { // Local file uploading - animateSpinner(); + animateSpinner(null, true, undefined, I18n.t("general.file.uploading")); } } break; diff --git a/app/assets/javascripts/protocols/steps.js b/app/assets/javascripts/protocols/steps.js index c6efbe7fe..dbc69226e 100644 --- a/app/assets/javascripts/protocols/steps.js +++ b/app/assets/javascripts/protocols/steps.js @@ -360,13 +360,16 @@ function initEditableHandsOnTable(root) { } // Initialize comment form. -function initStepCommentForm($el) { - +function initStepCommentForm(ev, $el) { var $form = $el.find("ul form"); + var $commentInput = $form.find("#comment_message"); + $form.onSubmitValidator(textValidator, $commentInput); + $(".help-block", $form).addClass("hide"); - $form.on("ajax:send", function (data) { + $form + .on("ajax:send", function (data) { $("#comment_message", $form).attr("readonly", true); }) .on("ajax:success", function (e, data) { @@ -450,7 +453,7 @@ function initStepCommentTabAjax() { var parentNode = $this.parents("ul").parent(); target.html(data.html); - initStepCommentForm(parentNode); + initStepCommentForm(e, parentNode); initStepCommentsLink(parentNode); parentNode.find(".active").removeClass("active"); @@ -598,7 +601,7 @@ function processStep(ev, editMode, forS3) { startFileUpload(ev, ev.target); } else { // Local file uploading - animateSpinner(); + animateSpinner(null, true, undefined, I18n.t("general.file.uploading")); } } } diff --git a/app/assets/javascripts/sitewide/form_validators.js.erb b/app/assets/javascripts/sitewide/form_validators.js.erb index 04032badb..1e108764a 100644 --- a/app/assets/javascripts/sitewide/form_validators.js.erb +++ b/app/assets/javascripts/sitewide/form_validators.js.erb @@ -1,6 +1,19 @@ // Form validators. They'll find, render and focus error/s and // prevent form submission. +// Calls specified validator along with the arguments on form submition +$.fn.onSubmitValidator = function(validatorCb) { + var params = Array.prototype.slice.call(arguments, 1); + var $form = $(this); + if ($form.length) { + $form.submit(function (ev) { + $form.clear_form_errors(); + params.unshift(ev); + validatorCb.apply(this, params); + }); + } +}; + function textValidator(ev, textInput, canBeBlank = false, limitLength = true) { var nameTooShort = $(textInput).val().length === 0; var nameTooLong = $(textInput).val().length > 50; @@ -55,16 +68,10 @@ function checklistsValidator(ev, checklists, editMode) { return noErrors; } -$.fn.files_validator = function(fileTypeEnum) { - var $form = $(this); - if ($form.length) { - $form.submit(function (ev) { - $form.clear_form_errors(); - var $fileInputs = $form.find("input[type=file]"); - filesValidator(ev, $fileInputs, fileTypeEnum); - }); - } -}; +var FileTypeEnum = Object.freeze({ + FILE: 0, + AVATAR: 1 +}); function filesValidator(ev, fileInputs, fileTypeEnum) { var filesValid = true; @@ -89,11 +96,6 @@ function filesPresentValidator(ev, fileInputs) { return filesPresentValid; } -var FileTypeEnum = Object.freeze({ - FILE: 0, - AVATAR: 1 -}); - function filesSizeValidator(ev, fileInputs, fileTypeEnum) { function getFileTooBigError(file) { @@ -132,4 +134,4 @@ function filesSizeValidator(ev, fileInputs, fileTypeEnum) { // whether enough organization space is free function enaughSpaceValidator(ev, fileInputs) { return true; -} \ No newline at end of file +} diff --git a/app/assets/javascripts/users/registrations/edit.js b/app/assets/javascripts/users/registrations/edit.js index c90cd2de8..aca3d2494 100644 --- a/app/assets/javascripts/users/registrations/edit.js +++ b/app/assets/javascripts/users/registrations/edit.js @@ -74,7 +74,7 @@ function processFile(ev, forS3) { startFileUpload(ev, ev.target); } else { // Local file uploading - animateSpinner(); + animateSpinner(null, true, undefined, I18n.t("general.file.uploading")); } } } diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 2f3d16e20..afd5742b9 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -2,6 +2,8 @@ class AssetsController < ApplicationController before_action :load_vars, except: [:signature] before_action :check_read_permission, except: [:signature, :file_present] + # 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 { @@ -19,7 +21,7 @@ class AssetsController < ApplicationController validationAsset = Asset.new(asset_params) end - if not validationAsset.valid? + if validationAsset.errors.any? render json: { status: 'error', errors: validationAsset.errors diff --git a/app/controllers/steps_controller.rb b/app/controllers/steps_controller.rb index 1af1eb456..ec1e8b744 100644 --- a/app/controllers/steps_controller.rb +++ b/app/controllers/steps_controller.rb @@ -37,13 +37,8 @@ class StepsController < ApplicationController if step_assets.size > 0 step_assets[:assets_attributes].each do |i, data| # Ignore destroy requests on create - if data[:_destroy].nil? - if data[:id].present? - asset = Asset.find_by_id(data[:id]) - else - # For validation purposses if no JS - asset = Asset.new(data) - end + if data[:_destroy].nil? and data[:id].present? + asset = Asset.find_by_id(data[:id]) asset.created_by = current_user asset.last_modified_by = current_user @step.assets << asset diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 67a8a26d4..0ef8a0b12 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -7,6 +7,8 @@ class Users::RegistrationsController < Devise::RegistrationsController 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 { diff --git a/config/locales/en.yml b/config/locales/en.yml index 8dfdd40f3..20ea89168 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1397,29 +1397,6 @@ en: head_title: "Edit protocol" no_keywords: "No keywords" - general: - update: "Update" - edit: "Edit" - cancel: "Cancel" - close: "Close" - check_all: "Check all" - no_comments: "No comments!" - more_comments: "More comments" - comment_placeholder: "Your Message" - module: - one: "task" - other: "tasks" - public: "public" - private: "private" - search: "Search" - file: - loading: "%{fileName} is loading..." - size_exceeded: "File size must be less than %{file_size} MB." - blank: "You didn't select any file" - text: - not_blank: "can't be blank" - length_too_long: "is too long (maximum is %{max_length} characters)" - mailer: invitation_to_organization: subject: "You have been invited to team" @@ -1469,8 +1446,30 @@ en: # This section contains general words that can be used in any parts of # application. - errors: - upload: "Upload error. Try again or contact the administrator." + general: + update: "Update" + edit: "Edit" + cancel: "Cancel" + close: "Close" + check_all: "Check all" + no_comments: "No comments!" + more_comments: "More comments" + comment_placeholder: "Your Message" + module: + one: "task" + other: "tasks" + public: "public" + private: "private" + search: "Search" + file: + size_exceeded: "File size must be less than %{file_size} MB." + blank: "You didn't select any file" + uploading: "If you leave this page, the file(s) that 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." + text: + not_blank: "can't be blank" + length_too_long: "is too long (maximum is %{max_length} characters)" + busy: "The server is still processing your request. If you leave this page, the changes will be lost! Are you sure you want to continue?" Add: "Add" Asset: "File"