Fixed drag and drop into folder and onto breadcrumb

Fixed drag and drop code to use FileList.getSelectedFiles() instead of
the visible DOM elements.

3 files changed, 211 insertions, 82 deletions

diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index f9916b647b2..5667d07ede7 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -78,7 +78,7 @@ window.FileList = {
 
 		this.breadcrumb = new BreadCrumb({
 			onClick: this._onClickBreadCrumb,
-			onDrop: this._onDropOnBreadCrumb,
+			onDrop: _.bind(this._onDropOnBreadCrumb, this),
 			getCrumbUrl: function(part, index) {
 				return self.linkTo(part.dir);
 			}
@@ -259,44 +259,31 @@ window.FileList = {
 	 * Event handler when dropping on a breadcrumb
 	 */
 	_onDropOnBreadCrumb: function( event, ui ) {
-		var target=$(this).data('dir');
-		var dir = FileList.getCurrentDirectory();
-		while(dir.substr(0,1) === '/') {//remove extra leading /'s
-			dir=dir.substr(1);
+		var $target = $(event.target);
+		if (!$target.is('.crumb')) {
+			$target = $target.closest('.crumb');
+		}
+		var targetPath = $(event.target).data('dir');
+		var dir = this.getCurrentDirectory();
+		while (dir.substr(0,1) === '/') {//remove extra leading /'s
+			dir = dir.substr(1);
 		}
 		dir = '/' + dir;
 		if (dir.substr(-1,1) !== '/') {
 			dir = dir + '/';
 		}
-		if (target === dir || target+'/' === dir) {
+		// do nothing if dragged on current dir
+		if (targetPath === dir || targetPath + '/' === dir) {
 			return;
 		}
-		var files = ui.helper.find('tr');
-		$(files).each(function(i,row) {
-			var dir = $(row).data('dir');
-			var file = $(row).data('filename');
-			//slapdash selector, tracking down our original element that the clone budded off of.
-			var origin = $('tr[data-id=' + $(row).data('origin') + ']');
-			var td = origin.children('td.filename');
-			var oldBackgroundImage = td.css('background-image');
-			td.css('background-image', 'url('+ OC.imagePath('core', 'loading.gif') + ')');
-			$.post(OC.filePath('files', 'ajax', 'move.php'), { dir: dir, file: file, target: target }, function(result) {
-				if (result) {
-					if (result.status === 'success') {
-						FileList.remove(file);
-						FileList.updateSelectionSummary();
-						$('#notification').hide();
-					} else {
-						$('#notification').hide();
-						$('#notification').text(result.data.message);
-						$('#notification').fadeIn();
-					}
-				} else {
-					OC.dialogs.alert(t('files', 'Error moving file'), t('files', 'Error'));
-				}
-				td.css('background-image', oldBackgroundImage);
-			});
-		});
+
+		var files = this.getSelectedFiles();
+		if (files.length === 0) {
+			// single one selected without checkbox?
+			files = _.map(ui.helper.find('tr'), FileList.elementToFile);
+		}
+
+		FileList.move(_.pluck(files, 'name'), targetPath);
 	},
 
 	/**
@@ -329,6 +316,7 @@ window.FileList = {
 	 * @return file data
 	 */
 	elementToFile: function($el){
+		$el = $($el);
 		return {
 			id: parseInt($el.attr('data-id'), 10),
 			name: $el.attr('data-file'),
@@ -877,6 +865,77 @@ window.FileList = {
 		}
 		return index;
 	},
+	/**
+	 * Moves a file to a given target folder.
+	 *
+	 * @param fileNames array of file names to move
+	 * @param targetPath absolute target path
+	 */
+	move: function(fileNames, targetPath) {
+		var self = this;
+		var dir = this.getCurrentDirectory();
+		var target = OC.basename(targetPath);
+		if (!_.isArray(fileNames)) {
+			fileNames = [fileNames];
+		}
+		_.each(fileNames, function(fileName) {
+			var $tr = self.findFileEl(fileName);
+			var $td = $tr.children('td.filename');
+			var oldBackgroundImage = $td.css('background-image');
+			$td.css('background-image', 'url('+ OC.imagePath('core', 'loading.gif') + ')');
+			// TODO: improve performance by sending all file names in a single call
+			$.post(
+				OC.filePath('files', 'ajax', 'move.php'),
+				{
+					dir: dir,
+					file: fileName,
+					target: targetPath
+				},
+				function(result) {
+					if (result) {
+						if (result.status === 'success') {
+							// if still viewing the same directory
+							if (self.getCurrentDirectory() === dir) {
+								// recalculate folder size
+								var oldFile = self.findFileEl(target);
+								var newFile = self.findFileEl(fileName);
+								var oldSize = oldFile.data('size');
+								var newSize = oldSize + newFile.data('size');
+								oldFile.data('size', newSize);
+								oldFile.find('td.filesize').text(OC.Util.humanFileSize(newSize));
+
+								// TODO: also update entry in FileList.files
+
+								self.remove(fileName);
+							}
+						} else {
+							OC.Notification.hide();
+							if (result.status === 'error' && result.data.message) {
+								OC.Notification.show(result.data.message);
+							}
+							else {
+								OC.Notification.show(t('files', 'Error moving file.'));
+							}
+							// hide notification after 10 sec
+							setTimeout(function() {
+								OC.Notification.hide();
+							}, 10000);
+						}
+					} else {
+						OC.dialogs.alert(t('files', 'Error moving file'), t('files', 'Error'));
+					}
+					$td.css('background-image', oldBackgroundImage);
+			});
+		});
+
+	},
+
+	/**
+	 * Triggers file rename input field for the given file name.
+	 * If the user enters a new name, the file will be renamed.
+	 *
+	 * @param oldname file name of the file to rename
+	 */
 	rename: function(oldname) {
 		var tr, td, input, form;
 		tr = FileList.findFileEl(oldname);
diff --git a/apps/files/js/files.js b/apps/files/js/files.js
index f4c99d1128c..6857450602c 100644
--- a/apps/files/js/files.js
+++ b/apps/files/js/files.js
@@ -142,6 +142,7 @@ var Files = {
 		}
 	},
 
+	// TODO: move to FileList class
 	setupDragAndDrop: function() {
 		var $fileList = $('#fileList');
 
@@ -308,6 +309,7 @@ function boolOperationFinished(data, callback) {
 	}
 }
 
+// TODO: move to FileList
 var createDragShadow = function(event) {
 	//select dragged file
 	var isDragSelected = $(event.target).parents('tr').find('td input:first').prop('checked');
@@ -333,9 +335,12 @@ var createDragShadow = function(event) {
 	var dir=$('#dir').val();
 
 	$(selectedFiles).each(function(i,elem) {
-		var newtr = $('<tr/>').attr('data-dir', dir).attr('data-filename', elem.name).attr('data-origin', elem.origin);
+		var newtr = $('<tr/>')
+			.attr('data-dir', dir)
+			.attr('data-file', elem.name)
+			.attr('data-origin', elem.origin);
 		newtr.append($('<td/>').addClass('filename').text(elem.name));
-		newtr.append($('<td/>').addClass('size').text(humanFileSize(elem.size)));
+		newtr.append($('<td/>').addClass('size').text(OC.Util.humanFileSize(elem.size)));
 		tbody.append(newtr);
 		if (elem.type === 'dir') {
 			newtr.find('td.filename').attr('style','background-image:url('+OC.imagePath('core', 'filetypes/folder.png')+')');
@@ -352,6 +357,7 @@ var createDragShadow = function(event) {
 
 //options for file drag/drop
 //start&stop handlers needs some cleaning up
+// TODO: move to FileList class
 var dragOptions={
 	revert: 'invalid', revertDuration: 300,
 	opacity: 0.7, zIndex: 100, appendTo: 'body', cursorAt: { left: 24, top: 18 },
@@ -381,50 +387,24 @@ if ( $('html.ie').length === 0) {
 	dragOptions['distance'] = 20;
 }
 
-var folderDropOptions={
+// TODO: move to FileList class
+var folderDropOptions = {
 	hoverClass: "canDrop",
 	drop: function( event, ui ) {
-		//don't allow moving a file into a selected folder
+		// don't allow moving a file into a selected folder
 		if ($(event.target).parents('tr').find('td input:first').prop('checked') === true) {
 			return false;
 		}
 
-		var target = $(this).closest('tr').data('file');
-
-		var files = ui.helper.find('tr');
-		$(files).each(function(i,row) {
-			var dir = $(row).data('dir');
-			var file = $(row).data('filename');
-							//slapdash selector, tracking down our original element that the clone budded off of.
-				var origin = $('tr[data-id=' + $(row).data('origin') + ']');
-				var td = origin.children('td.filename');
-				var oldBackgroundImage = td.css('background-image');
-				td.css('background-image', 'url('+ OC.imagePath('core', 'loading.gif') + ')');
-			$.post(OC.filePath('files', 'ajax', 'move.php'), { dir: dir, file: file, target: dir+'/'+target }, function(result) {
-				if (result) {
-					if (result.status === 'success') {
-						//recalculate folder size
-						var oldFile = FileList.findFileEl(target);
-						var newFile = FileList.findFileEl(file);
-						var oldSize = oldFile.data('size');
-						var newSize = oldSize + newFile.data('size');
-						oldFile.data('size', newSize);
-						oldFile.find('td.filesize').text(humanFileSize(newSize));
-
-						FileList.remove(file);
-						FileList.updateSelectionSummary();
-						$('#notification').hide();
-					} else {
-						$('#notification').hide();
-						$('#notification').text(result.data.message);
-						$('#notification').fadeIn();
-					}
-				} else {
-					OC.dialogs.alert(t('files', 'Error moving file'), t('files', 'Error'));
-				}
-				td.css('background-image', oldBackgroundImage);
-			});
-		});
+		var targetPath = FileList.getCurrentDirectory() + '/' + $(this).closest('tr').data('file');
+
+		var files = FileList.getSelectedFiles();
+		if (files.length === 0) {
+			// single one selected without checkbox?
+			files = _.map(ui.helper.find('tr'), FileList.elementToFile);
+		}
+
+		FileList.move(_.pluck(files, 'name'), targetPath);
 	},
 	tolerance: 'pointer'
 };
diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js
index da209220cca..eab364644cd 100644
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@@ -541,6 +541,100 @@ describe('FileList tests', function() {
 			expect($tr.find('a.name').attr('href')).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2Fsubdir&files=One.txt');
 		});
 	});
+	describe('Moving files', function() {
+		beforeEach(function() {
+			FileList.setFiles(testFiles);
+		});
+		it('Moves single file to target folder', function() {
+			var request;
+			FileList.move('One.txt', '/somedir');
+
+			expect(fakeServer.requests.length).toEqual(1);
+			request = fakeServer.requests[0];
+			expect(request.url).toEqual(OC.webroot + '/index.php/apps/files/ajax/move.php');
+			expect(OC.parseQueryString(request.requestBody)).toEqual({dir: '/subdir', file: 'One.txt', target: '/somedir'});
+
+			fakeServer.requests[0].respond(200, {'Content-Type': 'application/json'}, JSON.stringify({
+				status: 'success',
+				data: {
+					name: 'One.txt',
+					type: 'file'
+				}
+			}));
+
+			expect(FileList.findFileEl('One.txt').length).toEqual(0);
+
+			// folder size has increased
+			expect(FileList.findFileEl('somedir').data('size')).toEqual(262);
+			expect(FileList.findFileEl('somedir').find('.filesize').text()).toEqual('262 B');
+
+			expect(notificationStub.notCalled).toEqual(true);
+		});
+		it('Moves list of files to target folder', function() {
+			var request;
+			FileList.move(['One.txt', 'Two.jpg'], '/somedir');
+
+			expect(fakeServer.requests.length).toEqual(2);
+			request = fakeServer.requests[0];
+			expect(request.url).toEqual(OC.webroot + '/index.php/apps/files/ajax/move.php');
+			expect(OC.parseQueryString(request.requestBody)).toEqual({dir: '/subdir', file: 'One.txt', target: '/somedir'});
+
+			request = fakeServer.requests[1];
+			expect(request.url).toEqual(OC.webroot + '/index.php/apps/files/ajax/move.php');
+			expect(OC.parseQueryString(request.requestBody)).toEqual({dir: '/subdir', file: 'Two.jpg', target: '/somedir'});
+
+			fakeServer.requests[0].respond(200, {'Content-Type': 'application/json'}, JSON.stringify({
+				status: 'success',
+				data: {
+					name: 'One.txt',
+					type: 'file'
+				}
+			}));
+
+			expect(FileList.findFileEl('One.txt').length).toEqual(0);
+
+			// folder size has increased
+			expect(FileList.findFileEl('somedir').data('size')).toEqual(262);
+			expect(FileList.findFileEl('somedir').find('.filesize').text()).toEqual('262 B');
+
+			fakeServer.requests[1].respond(200, {'Content-Type': 'application/json'}, JSON.stringify({
+				status: 'success',
+				data: {
+					name: 'Two.jpg',
+					type: 'file'
+				}
+			}));
+
+			expect(FileList.findFileEl('Two.jpg').length).toEqual(0);
+
+			// folder size has increased
+			expect(FileList.findFileEl('somedir').data('size')).toEqual(12311);
+			expect(FileList.findFileEl('somedir').find('.filesize').text()).toEqual('12 kB');
+
+			expect(notificationStub.notCalled).toEqual(true);
+		});
+		it('Shows notification if a file could not be moved', function() {
+			var request;
+			FileList.move('One.txt', '/somedir');
+
+			expect(fakeServer.requests.length).toEqual(1);
+			request = fakeServer.requests[0];
+			expect(request.url).toEqual(OC.webroot + '/index.php/apps/files/ajax/move.php');
+			expect(OC.parseQueryString(request.requestBody)).toEqual({dir: '/subdir', file: 'One.txt', target: '/somedir'});
+
+			fakeServer.requests[0].respond(200, {'Content-Type': 'application/json'}, JSON.stringify({
+				status: 'error',
+				data: {
+					message: 'Error while moving file',
+				}
+			}));
+
+			expect(FileList.findFileEl('One.txt').length).toEqual(1);
+
+			expect(notificationStub.calledOnce).toEqual(true);
+			expect(notificationStub.getCall(0).args[0]).toEqual('Error while moving file');
+		});
+	});
 	describe('List rendering', function() {
 		it('renders a list of files using add()', function() {
 			expect(FileList.files.length).toEqual(0);
@@ -932,14 +1026,12 @@ describe('FileList tests', function() {
 				}
 			};
 			// returns a list of tr that were dragged
-			// FIXME: why are their attributes different than the
-			// regular file trs ?
 			ui.helper.find.returns([
-				$('<tr data-filename="One.txt" data-dir="' + testDir + '"></tr>'),
-				$('<tr data-filename="Two.jpg" data-dir="' + testDir + '"></tr>')
+				$('<tr data-file="One.txt" data-dir="' + testDir + '"></tr>'),
+				$('<tr data-file="Two.jpg" data-dir="' + testDir + '"></tr>')
 			]);
 			// simulate drop event
-			FileList._onDropOnBreadCrumb.call($crumb, new $.Event('drop'), ui);
+			FileList._onDropOnBreadCrumb(new $.Event('drop', {target: $crumb}), ui);
 
 			// will trigger two calls to move.php (first one was previous list.php)
 			expect(fakeServer.requests.length).toEqual(3);
@@ -976,14 +1068,12 @@ describe('FileList tests', function() {
 				}
 			};
 			// returns a list of tr that were dragged
-			// FIXME: why are their attributes different than the
-			// regular file trs ?
 			ui.helper.find.returns([
-				$('<tr data-filename="One.txt" data-dir="' + testDir + '"></tr>'),
-				$('<tr data-filename="Two.jpg" data-dir="' + testDir + '"></tr>')
+				$('<tr data-file="One.txt" data-dir="' + testDir + '"></tr>'),
+				$('<tr data-file="Two.jpg" data-dir="' + testDir + '"></tr>')
 			]);
 			// simulate drop event
-			FileList._onDropOnBreadCrumb.call($crumb, new $.Event('drop'), ui);
+			FileList._onDropOnBreadCrumb(new $.Event('drop', {target: $crumb}), ui);
 
 			// no extra server request
 			expect(fakeServer.requests.length).toEqual(1);