Merge pull request #6929 from owncloud/sharing-fixfiledownloadlink

Sharing fixfiledownloadlink

apps/files/js/fileactions.js

@@ -173,7 +173,10 @@ $(document).ready(function () {
 		FileActions.register(downloadScope, 'Download', OC.PERMISSION_READ, function () {
 			return OC.imagePath('core', 'actions/download');
 		}, function (filename) {
-			window.location = OC.filePath('files', 'ajax', 'download.php') + '?files=' + encodeURIComponent(filename) + '&dir=' + encodeURIComponent($('#dir').val());
+			var url = FileList.getDownloadUrl(filename);
+			if (url) {
+				OC.redirect(url);
+			}
 		});
 	}
 	$('#fileList tr').each(function () {

apps/files/js/filelist.js

@@ -780,6 +780,20 @@ var FileList={
 		$('#fileList tr.searchresult').each(function(i,e) {
 			$(e).removeClass("searchresult");
 		});
+	},
+
+	/**
+	 * Returns the download URL of the given file
+	 * @param filename file name of the file
+	 * @param dir optional directory in which the file name is, defaults to the current directory
+	 */
+	getDownloadUrl: function(filename, dir) {
+		var params = {
+			files: filename,
+			dir: dir || FileList.getCurrentDirectory(),
+			download: null
+		};
+		return OC.filePath('files', 'ajax', 'download.php') + '?' + OC.buildQueryString(params);
 	}
 };
 

apps/files/tests/js/fileactionsSpec.js

@@ -0,0 +1,61 @@
+/**
+* ownCloud
+*
+* @author Vincent Petry
+* @copyright 2014 Vincent Petry <pvince81@owncloud.com>
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+describe('FileActions tests', function() {
+	beforeEach(function() {
+		// init horrible parameters
+		var $body = $('body');
+		$body.append('<input type="hidden" id="dir" value="/subdir"></input>');
+		$body.append('<input type="hidden" id="permissions" value="31"></input>');
+		// dummy files table
+		$filesTable = $body.append('<table id="filestable"></table>');
+	});
+	afterEach(function() {
+		$('#dir, #permissions, #filestable').remove();
+	});
+	it('calling display() sets file actions', function() {
+		// note: download_url is actually the link target, not the actual download URL...
+		var $tr = FileList.addFile('testName.txt', 1234, new Date(), false, false, {download_url: 'test/download/url'});
+
+		// no actions before call
+		expect($tr.find('.action[data-action=Download]').length).toEqual(0);
+		expect($tr.find('.action[data-action=Rename]').length).toEqual(0);
+		expect($tr.find('.action.delete').length).toEqual(0);
+
+		FileActions.display($tr.find('td.filename'), true);
+
+		// actions defined after cal
+		expect($tr.find('.action[data-action=Download]').length).toEqual(1);
+		expect($tr.find('.action[data-action=Rename]').length).toEqual(1);
+		expect($tr.find('.action.delete').length).toEqual(1);
+	});
+	it('redirects to download URL when clicking download', function() {
+		var redirectStub = sinon.stub(OC, 'redirect');
+		// note: download_url is actually the link target, not the actual download URL...
+		var $tr = FileList.addFile('test download File.txt', 1234, new Date(), false, false, {download_url: 'test/download/url'});
+		FileActions.display($tr.find('td.filename'), true);
+
+		$tr.find('.action[data-action=Download]').click();
+
+		expect(redirectStub.calledOnce).toEqual(true);
+		expect(redirectStub.getCall(0).args[0]).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?files=test%20download%20File.txt&dir=%2Fsubdir&download');
+		redirectStub.restore();
+	});
+});

apps/files/tests/js/filelistSpec.js

@@ -21,22 +21,27 @@
 describe('FileList tests', function() {
 	beforeEach(function() {
 		// init horrible parameters
-		$('<input type="hidden" id="dir" value="/subdir"></input>').append('body');
+		var $body = $('body');
-		$('<input type="hidden" id="permissions" value="31"></input>').append('body');
+		$body.append('<input type="hidden" id="dir" value="/subdir"></input>');
+		$body.append('<input type="hidden" id="permissions" value="31"></input>');
+		// dummy files table
+		$body.append('<table id="filestable"></table>');
 	});
 	afterEach(function() {
-		$('#dir, #permissions').remove();
+		$('#dir, #permissions, #filestable').remove();
 	});
 	it('generates file element with correct attributes when calling addFile', function() {
 		var lastMod = new Date(10000);
+		// note: download_url is actually the link target, not the actual download URL...
 		var $tr = FileList.addFile('testName.txt', 1234, lastMod, false, false, {download_url: 'test/download/url'});
 
 		expect($tr).toBeDefined();
 		expect($tr[0].tagName.toLowerCase()).toEqual('tr');
+		expect($tr.find('a:first').attr('href')).toEqual('test/download/url');
 		expect($tr.attr('data-type')).toEqual('file');
 		expect($tr.attr('data-file')).toEqual('testName.txt');
 		expect($tr.attr('data-size')).toEqual('1234');
-		//expect($tr.attr('data-permissions')).toEqual('31');
+		expect($tr.attr('data-permissions')).toEqual('31');
 		//expect($tr.attr('data-mime')).toEqual('plain/text');
 	});
 	it('generates dir element with correct attributes when calling addDir', function() {
@@ -48,7 +53,11 @@ describe('FileList tests', function() {
 		expect($tr.attr('data-type')).toEqual('dir');
 		expect($tr.attr('data-file')).toEqual('testFolder');
 		expect($tr.attr('data-size')).toEqual('1234');
-		//expect($tr.attr('data-permissions')).toEqual('31');
+		expect($tr.attr('data-permissions')).toEqual('31');
 		//expect($tr.attr('data-mime')).toEqual('httpd/unix-directory');
 	});
+	it('returns correct download URL', function() {
+		expect(FileList.getDownloadUrl('some file.txt')).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?files=some%20file.txt&dir=%2Fsubdir&download');
+		expect(FileList.getDownloadUrl('some file.txt', '/anotherpath/abc')).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?files=some%20file.txt&dir=%2Fanotherpath%2Fabc&download');
+	});
 });

apps/files_sharing/js/public.js

@@ -34,18 +34,16 @@ $(document).ready(function() {
 				window.location = $(tr).find('a.name').attr('href');
 			}
 		});
-		FileActions.register('file', 'Download', OC.PERMISSION_READ, '', function(filename) {
+
+		// override since the format is different
+		FileList.getDownloadUrl = function(filename, dir) {
+			// we use this because we need the service and token attributes
 			var tr = FileList.findFileEl(filename);
 			if (tr.length > 0) {
-				window.location = $(tr).find('a.name').attr('href');
+				return $(tr).find('a.name').attr('href') + '&download';
 			}
-		});
+			return null;
-		FileActions.register('dir', 'Download', OC.PERMISSION_READ, '', function(filename) {
+		};
-			var tr = FileList.findFileEl(filename);
-			if (tr.length > 0) {
-				window.location = $(tr).find('a.name').attr('href')+'&download';
-			}
-		});
 	}
 
 	var file_upload_start = $('#file_upload_start');

core/js/js.js

@@ -252,6 +252,12 @@ var OC={
 		}
 		return link;
 	},
+	/**
+	 * Redirect to the target URL, can also be used for downloads.
+	 */
+	redirect: function(targetUrl) {
+		window.location = targetUrl;
+	},
 	/**
 	 * get the absolute path to an image file
 	 * @param app the app id to which the image belongs
@@ -364,6 +370,34 @@ var OC={
 		}
 		return result;
 	},
+
+	/**
+	 * Builds a URL query from a JS map.
+	 * @param params parameter map
+	 * @return string containing a URL query (without question) mark
+	 */
+	buildQueryString: function(params) {
+		var s = '';
+		var first = true;
+		if (!params) {
+			return s;
+		}
+		for (var key in params) {
+			var value = params[key];
+			if (first) {
+				first = false;
+			}
+			else {
+				s += '&';
+			}
+			s += encodeURIComponent(key);
+			if (value !== null && typeof(value) !== 'undefined') {
+				s += '=' + encodeURIComponent(value);
+			}
+		}
+		return s;
+	},
+
 	/**
 	 * Opens a popup with the setting for an app.
 	 * @param appid String. The ID of the app e.g. 'calendar', 'contacts' or 'files'.

core/js/tests/specs/coreSpec.js

@@ -67,4 +67,41 @@ describe('Core base tests', function() {
 			});
 		});
 	});
+	describe('Query string building', function() {
+		it('Returns empty string when empty params', function() {
+			expect(OC.buildQueryString()).toEqual('');
+			expect(OC.buildQueryString({})).toEqual('');
+		});
+		it('Encodes regular query strings', function() {
+			expect(OC.buildQueryString({
+				a: 'abc',
+				b: 'def'
+			})).toEqual('a=abc&b=def');
+		});
+		it('Encodes special characters', function() {
+			expect(OC.buildQueryString({
+				unicode: '汉字',
+			})).toEqual('unicode=%E6%B1%89%E5%AD%97');
+			expect(OC.buildQueryString({
+			   	b: 'spaace value',
+			   	'space key': 'normalvalue',
+			   	'slash/this': 'amp&ersand'
+			})).toEqual('b=spaace%20value&space%20key=normalvalue&slash%2Fthis=amp%26ersand');
+		});
+		it('Encodes data types and empty values', function() {
+			expect(OC.buildQueryString({
+				'keywithemptystring': '',
+			   	'keywithnull': null,
+			   	'keywithundefined': null,
+				something: 'else'
+			})).toEqual('keywithemptystring=&keywithnull&keywithundefined&something=else');
+			expect(OC.buildQueryString({
+			   	'booleanfalse': false,
+				'booleantrue': true
+			})).toEqual('booleanfalse=false&booleantrue=true');
+			expect(OC.buildQueryString({
+			   	'number': 123,
+			})).toEqual('number=123');
+		});
+	});
 });