From 41b6d4b702e8ed32f7ea51edffd0005639f77138 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 24 Jan 2014 12:44:31 +0100 Subject: [PATCH 1/3] Added OC.buidQueryString() utility function Makes it possible to create query strings by passing a JavaScript hash map and automatically encodes the keys and values. --- core/js/js.js | 28 +++++++++++++++++++++++++ core/js/tests/specs/coreSpec.js | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/core/js/js.js b/core/js/js.js index e84f482d67..976027dd06 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -364,6 +364,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'. diff --git a/core/js/tests/specs/coreSpec.js b/core/js/tests/specs/coreSpec.js index 827669f270..28c20a0642 100644 --- a/core/js/tests/specs/coreSpec.js +++ b/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'); + }); + }); }); From 0671c58e361f2ccf2cd23d73a9712c1a31e838ce Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 24 Jan 2014 13:19:44 +0100 Subject: [PATCH 2/3] Fixed filelist unit tests hidden params Also added dummy table --- apps/files/tests/js/filelistSpec.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 6b28a02989..be848e0e0b 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -21,11 +21,14 @@ describe('FileList tests', function() { beforeEach(function() { // init horrible parameters - $('').append('body'); - $('').append('body'); + var $body = $('body'); + $body.append(''); + $body.append(''); + // dummy files table + $body.append('
'); }); 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); @@ -36,7 +39,7 @@ describe('FileList tests', function() { 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 +51,7 @@ 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'); }); }); From c6695bbd764be9f43067c09894e36422c2b92b49 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 24 Jan 2014 13:32:31 +0100 Subject: [PATCH 3/3] Fixed download URL in public page - Refactored download URL building to make it overridable - Added download URL override in public page - Added JS unit tests for download URL - Added OC.redirect() method to facilitate unit testing --- apps/files/js/fileactions.js | 5 ++- apps/files/js/filelist.js | 14 ++++++ apps/files/tests/js/fileactionsSpec.js | 61 ++++++++++++++++++++++++++ apps/files/tests/js/filelistSpec.js | 6 +++ apps/files_sharing/js/public.js | 16 +++---- core/js/js.js | 6 +++ 6 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 apps/files/tests/js/fileactionsSpec.js diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 74bb711ef3..eb59e71a03 100644 --- a/apps/files/js/fileactions.js +++ b/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 () { diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 66968ab54c..63fd0f4ce0 100644 --- a/apps/files/js/filelist.js +++ b/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); } }; diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js new file mode 100644 index 0000000000..23f7b58dcd --- /dev/null +++ b/apps/files/tests/js/fileactionsSpec.js @@ -0,0 +1,61 @@ +/** +* ownCloud +* +* @author Vincent Petry +* @copyright 2014 Vincent Petry +* +* 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 . +* +*/ +describe('FileActions tests', function() { + beforeEach(function() { + // init horrible parameters + var $body = $('body'); + $body.append(''); + $body.append(''); + // dummy files table + $filesTable = $body.append('
'); + }); + 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(); + }); +}); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index be848e0e0b..61e026c072 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -32,10 +32,12 @@ describe('FileList tests', function() { }); 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'); @@ -54,4 +56,8 @@ describe('FileList tests', function() { 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'); + }); }); diff --git a/apps/files_sharing/js/public.js b/apps/files_sharing/js/public.js index 4c0b0ad9d4..79c15623c0 100644 --- a/apps/files_sharing/js/public.js +++ b/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'; } - }); - 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'; - } - }); + return null; + }; } var file_upload_start = $('#file_upload_start'); diff --git a/core/js/js.js b/core/js/js.js index 976027dd06..1c7d89ea05 100644 --- a/core/js/js.js +++ b/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