Merge pull request #654 from ZogStriP/consistent-behavior-regarding-file-uploads

consistent behavior regarding file uploads

app/assets/javascripts/discourse/components/utilities.js

@@ -9,17 +9,10 @@ Discourse.Utilities = {
 
   translateSize: function(size) {
     switch (size) {
-      case 'tiny':
-        size = 20;
-        break;
-      case 'small':
-        size = 25;
-        break;
-      case 'medium':
-        size = 32;
-        break;
-      case 'large':
-        size = 45;
+      case 'tiny': return 20;
+      case 'small': return 25;
+      case 'medium': return 32;
+      case 'large': return 45;
     }
     return size;
   },
@@ -163,6 +156,45 @@ Discourse.Utilities = {
       range.moveStart('character', pos);
       return range.select();
     }
+  },
+
+  /**
+  validateFilesForUpload
+
+  **/
+  /**
+    Validate a list of files to be uploaded
+
+    @method validateFilesForUpload
+    @param {Array} files The list of files we want to upload
+  **/
+  validateFilesForUpload: function(files) {
+    if (files) {
+      // can only upload one file at a time
+      if (files.length > 1) {
+        bootbox.alert(Em.String.i18n('post.errors.upload_too_many_images'));
+        return false;
+      } else if (files.length > 0) {
+        // check that the uploaded file is an image
+        // TODO: we should provide support for other types of file
+        if (files[0].type && files[0].type.indexOf('image/') !== 0) {
+          bootbox.alert(Em.String.i18n('post.errors.only_images_are_supported'));
+          return false;
+        }
+        // check file size
+        if (files[0].size && files[0].size > 0) {
+          var fileSizeInKB = files[0].size / 1024;
+          if (fileSizeInKB > Discourse.SiteSettings.max_upload_size_kb) {
+            bootbox.alert(Em.String.i18n('post.errors.upload_too_large', { max_size_kb: Discourse.SiteSettings.max_upload_size_kb }));
+            return false;
+          }
+          // everything is fine
+          return true;
+        }
+      }
+    }
+    // there has been an error
+    return false;
   }
 
 };

app/assets/javascripts/discourse/views/composer_view.js

@@ -276,43 +276,18 @@ Discourse.ComposerView = Discourse.View.extend({
         formData: { topic_id: 1234 }
     });
 
-    var addFiles = function (e, data) {
-      // can only upload one file at a time
-      if (data.files.length > 1) {
-        bootbox.alert(Em.String.i18n('post.errors.upload_too_many_images'));
-        return false;
-      } else if (data.files.length > 0) {
-        // check file size
-        var fileSizeInKB = data.files[0].size / 1024;
-        if (fileSizeInKB > Discourse.SiteSettings.max_upload_size_kb) {
-          bootbox.alert(Em.String.i18n('post.errors.upload_too_large', { max_size_kb: Discourse.SiteSettings.max_upload_size_kb }));
-          return false;
-        }
-        // check that the uploaded file is an image
-        // TODO: we should provide support for other types of file
-        if (data.files[0].type.indexOf('image/') !== 0) {
-          bootbox.alert(Em.String.i18n('post.errors.only_images_are_supported'));
-          return false; 
-        }
-        // everything is fine, reset upload status
-        _this.setProperties({
-          uploadProgress: 0,
-          loadingImage: true
-        });
-        return true;
-      }
-      // we need to return true here, otherwise it prevents the default paste behavior
-      return true;
-    };
+    // submit - this event is triggered for each upload
+    $uploadTarget.on('fileuploadsubmit', function (e, data) {
+      var result = Discourse.Utilities.validateFilesForUpload(data.files);
+      // reset upload status when everything is ok
+      if (result) _this.setProperties({ uploadProgress: 0, loadingImage: true });
+      return result;
+    });
 
-    // paste
-    $uploadTarget.on('fileuploadpaste', addFiles);
-
-    // drop
-    $uploadTarget.on('fileuploaddrop', addFiles);
-
-    // send
+    // send - this event is triggered when the upload request is about to start
     $uploadTarget.on('fileuploadsend', function (e, data) {
+      // hide the "image selector" modal
+      $('#discourse-modal').modal('hide');
       // cf. https://github.com/blueimp/jQuery-File-Upload/wiki/API#how-to-cancel-an-upload
       var jqXHR = data.xhr();
       // need to wait for the link to show up in the DOM

app/assets/javascripts/discourse/views/modal/image_selector_view.js

@@ -13,30 +13,27 @@ Discourse.ImageSelectorView = Discourse.View.extend({
 
   init: function() {
     this._super();
-    return this.set('localSelected', true);
+    this.set('localSelected', true);
   },
 
   selectLocal: function() {
-    return this.set('localSelected', true);
+    this.set('localSelected', true);
   },
 
   selectRemote: function() {
-    return this.set('localSelected', false);
+    this.set('localSelected', false);
   },
 
-  remoteSelected: (function() {
+  remoteSelected: function() {
     return !this.get('localSelected');
-  }).property('localSelected'),
+  }.property('localSelected'),
 
   upload: function() {
-    this.get('uploadTarget').fileupload('send', { fileInput: $('#filename-input') });
-    return $('#discourse-modal').modal('hide');
+    this.get('uploadTarget').fileupload('add', { fileInput: $('#filename-input') });
   },
 
   add: function() {
-    this.get('composer').addMarkdown("![image](" + ($('#fileurl-input').val()) + ")");
-    return $('#discourse-modal').modal('hide');
+    this.get('composer').addMarkdown("![image](" + $('#fileurl-input').val() + ")");
+    $('#discourse-modal').modal('hide');
   }
 });
-
-

spec/javascripts/components/utilities_spec.js

@@ -1,4 +1,4 @@
-/*global waitsFor:true expect:true describe:true beforeEach:true it:true */
+/*global waitsFor:true expect:true describe:true beforeEach:true it:true spyOn:true */
 
 describe("Discourse.Utilities", function() {
 
@@ -30,4 +30,50 @@ describe("Discourse.Utilities", function() {
 
   });
 
+  describe("validateFilesForUpload", function() {
+
+    it("returns false when file is undefined", function() {
+      expect(Discourse.Utilities.validateFilesForUpload(null)).toBe(false);
+      expect(Discourse.Utilities.validateFilesForUpload(undefined)).toBe(false);
+    });
+
+    it("returns false when file there is no file", function() {
+      expect(Discourse.Utilities.validateFilesForUpload([])).toBe(false);
+    });
+
+    it("supports only one file", function() {
+      spyOn(bootbox, 'alert');
+      spyOn(Em.String, 'i18n');
+      expect(Discourse.Utilities.validateFilesForUpload([1, 2])).toBe(false);
+      expect(bootbox.alert).toHaveBeenCalled();
+      expect(Em.String.i18n).toHaveBeenCalledWith('post.errors.upload_too_many_images');
+    });
+
+    it("supports only an image", function() {
+      var html = { type: "text/html" };
+      spyOn(bootbox, 'alert');
+      spyOn(Em.String, 'i18n');
+      expect(Discourse.Utilities.validateFilesForUpload([html])).toBe(false);
+      expect(bootbox.alert).toHaveBeenCalled();
+      expect(Em.String.i18n).toHaveBeenCalledWith('post.errors.only_images_are_supported');
+    });
+
+    it("prevents the upload of a too large image", function() {
+      var image = { type: "image/png", size: 10 * 1024 };
+      Discourse.SiteSettings.max_upload_size_kb = 5;
+      spyOn(bootbox, 'alert');
+      spyOn(Em.String, 'i18n');
+      expect(Discourse.Utilities.validateFilesForUpload([image])).toBe(false);
+      expect(bootbox.alert).toHaveBeenCalled();
+      expect(Em.String.i18n).toHaveBeenCalledWith('post.errors.upload_too_large', { max_size_kb: 5 });
+    });
+
+    it("works", function() {
+      var image = { type: "image/png", size: 10 * 1024 };
+      Discourse.SiteSettings.max_upload_size_kb = 15;
+      expect(Discourse.Utilities.validateFilesForUpload([image])).toBe(true);
+    });
+
+  });
+
 });

spec/javascripts/spec.js

@@ -18,6 +18,7 @@
 // The rest of the externals
 //= require_tree ../../app/assets/javascripts/external
 
+//= require ../../app/assets/javascripts/locales/i18n
 //= require ../../app/assets/javascripts/discourse/helpers/i18n_helpers
 //= require ../../app/assets/javascripts/discourse