[Pkg-owncloud-commits] [owncloud] 20/199: Fixed drag and drop from external files, added tests

[David Prévot]

This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch master
in repository owncloud.

commit 542b3958580ea77879d8c941a167af75972fa60c
Author: Vincent Petry <pvince81 at owncloud.com>
Date:   Wed May 21 15:55:29 2014 +0200

    Fixed drag and drop from external files, added tests
    
    - Fixed detection whether the drop zone is inside the currently visible
      table
    - Now dragging outside the table does nothing instead of uploading,
      because the user might drop on the sidebar
    - Added unit tests for the drop handler
---
 apps/files/js/filelist.js           |  19 ++++--
 apps/files/tests/js/filelistSpec.js | 131 ++++++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+), 5 deletions(-)

diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index 3dcd9dd..3d107c7 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -1517,13 +1517,19 @@
 			fileUploadStart.on('fileuploaddrop', function(e, data) {
 				OC.Upload.log('filelist handle fileuploaddrop', e, data);
 
-				var dropTarget = $(e.originalEvent.target).closest('tr, .crumb');
-				// check if dropped inside this list at all
-				if (dropTarget && !self.$el.has(dropTarget).length) {
+
+				var dropTarget = $(e.originalEvent.target);
+				// check if dropped inside this list and not another one
+				if (dropTarget.length && !self.$el.has(dropTarget).length) {
 					return false;
 				}
 
-				if (dropTarget && (dropTarget.data('type') === 'dir' || dropTarget.hasClass('crumb'))) { // drag&drop upload to folder
+				// find the closest tr or crumb to use as target
+				dropTarget = dropTarget.closest('tr, .crumb');
+
+				// if dropping on tr or crumb, drag&drop upload to folder
+				if (dropTarget && (dropTarget.data('type') === 'dir' ||
+					dropTarget.hasClass('crumb'))) {
 
 					// remember as context
 					data.context = dropTarget;
@@ -1543,7 +1549,7 @@
 					}
 
 					// update folder in form
-					data.formData = function(form) {
+					data.formData = function() {
 						return [
 							{name: 'dir', value: dir},
 							{name: 'requesttoken', value: oc_requesttoken},
@@ -1551,6 +1557,9 @@
 						];
 					};
 				} else {
+					// we are dropping somewhere inside the file list, which will
+					// upload the file to the current directory
+
 					// cancel uploads to current dir if no permission
 					var isCreatable = (self.getDirectoryPermissions() & OC.PERMISSION_CREATE) !== 0;
 					if (!isCreatable) {
diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js
index a3dc5b2..e5bbf0f 100644
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@@ -65,6 +65,8 @@ describe('OCA.Files.FileList tests', function() {
 			'   <div class="actions creatable"></div>' +
 			'   <div class="notCreatable"></div>' +
 			'</div>' +
+			// uploader
+			'<input type="file" id="file_upload_start" name="files[]" multiple="multiple">' +
 			// dummy table
 			// TODO: at some point this will be rendered by the fileList class itself!
 			'<table id="filestable">' +
@@ -1651,4 +1653,133 @@ describe('OCA.Files.FileList tests', function() {
 			expect(fileList.findFileEl('Three.pdf').index()).toEqual(0);
 		});
 	});
+	/**
+	 * Test upload mostly by testing the code inside the event handlers
+	 * that were registered on the magic upload object
+	 */
+	describe('file upload', function() {
+		var $uploader;
+
+		beforeEach(function() {
+			// note: this isn't the real blueimp file uploader from jquery.fileupload
+			// but it makes it possible to simulate the event triggering to
+			// test the response of the handlers
+			$uploader = $('#file_upload_start');
+			fileList.setupUploadEvents();
+			fileList.setFiles(testFiles);
+		});
+
+		afterEach(function() {
+			$uploader = null;
+		});
+
+		describe('dropping external files', function() {
+			var uploadData;
+
+			/**
+			 * Simulate drop event on the given target
+			 *
+			 * @param $target target element to drop on
+			 * @return event object including the result
+			 */
+			function dropOn($target, data) {
+				var eventData = {
+					originalEvent: {
+						target: $target
+					}
+				};
+				var ev = new $.Event('fileuploaddrop', eventData);
+				// using triggerHandler instead of trigger so we can pass
+				// extra data
+				$uploader.triggerHandler(ev, data || {});
+				return ev;
+			}
+
+			/**
+			 * Convert form data to a flat list
+			 * 
+			 * @param formData form data array as used by jquery.upload
+			 * @return map based on the array's key values
+			 */
+			function decodeFormData(data) {
+				var map = {};
+				_.each(data.formData(), function(entry) {
+					map[entry.name] = entry.value;
+				});
+				return map;
+			}
+
+			beforeEach(function() {
+				// simulate data structure from jquery.upload
+				uploadData = {
+					files: [{
+						relativePath: 'fileToUpload.txt'
+					}]
+				};
+			});
+			afterEach(function() {
+				uploadData = null;
+			});
+			it('drop on a tr or crumb outside file list does not trigger upload', function() {
+				var $anotherTable = $('<table><tbody><tr><td>outside<div class="crumb">crumb</div></td></tr></table>');
+				var ev;
+				$('#testArea').append($anotherTable);
+				ev = dropOn($anotherTable.find('tr'), uploadData);
+				expect(ev.result).toEqual(false);
+
+				ev = dropOn($anotherTable.find('.crumb'));
+				expect(ev.result).toEqual(false);
+			});
+			it('drop on an element outside file list does not trigger upload', function() {
+				var $anotherEl = $('<div>outside</div>');
+				var ev;
+				$('#testArea').append($anotherEl);
+				ev = dropOn($anotherEl);
+
+				expect(ev.result).toEqual(false);
+			});
+			it('drop on an element inside the table triggers upload', function() {
+				var ev;
+				ev = dropOn(fileList.$fileList.find('th:first'), uploadData);
+
+				expect(ev.result).not.toEqual(false);
+			});
+			it('drop on an element inside the table does not trigger upload if no upload permission', function() {
+				$('#permissions').val(0);
+				var ev;
+				ev = dropOn(fileList.$fileList.find('th:first'));
+
+				expect(ev.result).toEqual(false);
+			});
+			it('drop on a file row inside the table triggers upload to current folder', function() {
+				var ev;
+				ev = dropOn(fileList.findFileEl('One.txt').find('td:first'), uploadData);
+
+				expect(ev.result).not.toEqual(false);
+			});
+			it('drop on a folder row inside the table triggers upload to target folder', function() {
+				var ev, formData;
+				ev = dropOn(fileList.findFileEl('somedir').find('td:eq(2)'), uploadData);
+
+				expect(ev.result).not.toEqual(false);
+				expect(uploadData.formData).toBeDefined();
+				formData = decodeFormData(uploadData);
+				expect(formData.dir).toEqual('/subdir/somedir');
+				expect(formData.file_directory).toEqual('fileToUpload.txt');
+				expect(formData.requesttoken).toBeDefined();
+			});
+			it('drop on a breadcrumb inside the table triggers upload to target folder', function() {
+				var ev, formData;
+				fileList.changeDirectory('a/b/c/d');
+				ev = dropOn(fileList.$el.find('.crumb:eq(2)'), uploadData);
+
+				expect(ev.result).not.toEqual(false);
+				expect(uploadData.formData).toBeDefined();
+				formData = decodeFormData(uploadData);
+				expect(formData.dir).toEqual('/a/b');
+				expect(formData.file_directory).toEqual('fileToUpload.txt');
+				expect(formData.requesttoken).toBeDefined();
+			});
+		});
+	});
 });

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git