From: Vincent Petry Date: Fri, 16 May 2014 10:43:36 +0000 (+0200) Subject: Fixed issues with renaming X-Git-Tag: v7.0.0alpha2~231^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5a0281add84b0754404c9fa617c4f527fcef04dc;p=nextcloud-server.git Fixed issues with renaming - summary is now untouched on rename instead of bogus incrementation - fixes "Share" status icon by only triggring "fileActionsReady" once - row is now reinserted even when rename is cancelled, this removes the hacky/buggy code that tried to rename the element back - added unit tests to cover the fixed cases --- diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 38766e2b801..3dcd9dd3eaa 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -1184,10 +1184,22 @@ event.preventDefault(); try { var newName = input.val(); + input.tipsy('hide'); + form.remove(); + if (newName !== oldname) { checkInput(); - // mark as loading + // mark as loading (temp element) td.css('background-image', 'url('+ OC.imagePath('core', 'loading.gif') + ')'); + tr.attr('data-file', newName); + var basename = newName; + if (newName.indexOf('.') > 0 && tr.data('type') !== 'dir') { + basename = newName.substr(0, newName.lastIndexOf('.')); + } + td.find('a.name span.nametext').text(basename); + td.children('a.name').show(); + tr.find('.fileactions, .action').addClass('hidden'); + $.ajax({ url: OC.filePath('files','ajax','rename.php'), data: { @@ -1207,30 +1219,17 @@ // reinsert row self.files.splice(tr.index(), 1); tr.remove(); - self.add(fileInfo); + self.add(fileInfo, {updateSummary: false}); + self.$fileList.trigger($.Event('fileActionsReady')); } }); + } else { + // add back the old file info when cancelled + self.files.splice(tr.index(), 1); + tr.remove(); + self.add(oldFileInfo, {updateSummary: false}); + self.$fileList.trigger($.Event('fileActionsReady')); } - input.tipsy('hide'); - tr.data('renaming',false); - tr.attr('data-file', newName); - var path = td.children('a.name').attr('href'); - // FIXME this will fail if the path contains the filename. - td.children('a.name').attr('href', path.replace(encodeURIComponent(oldname), encodeURIComponent(newName))); - var basename = newName; - if (newName.indexOf('.') > 0 && tr.data('type') !== 'dir') { - basename = newName.substr(0, newName.lastIndexOf('.')); - } - td.find('a.name span.nametext').text(basename); - if (newName.indexOf('.') > 0 && tr.data('type') !== 'dir') { - if ( ! td.find('a.name span.extension').exists() ) { - td.find('a.name span.nametext').append(''); - } - td.find('a.name span.extension').text(newName.substr(newName.lastIndexOf('.'))); - } - form.remove(); - self.fileActions.display( tr.find('td.filename'), true); - td.children('a.name').show(); } catch (error) { input.attr('title', error); input.tipsy({gravity: 'w', trigger: 'manual'}); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index bfc983c7483..a3dc5b255a1 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -468,6 +468,22 @@ describe('OCA.Files.FileList tests', function() { }); }); describe('Renaming files', function() { + function doCancelRename() { + var $input; + for (var i = 0; i < testFiles.length; i++) { + fileList.add(testFiles[i]); + } + + // trigger rename prompt + fileList.rename('One.txt'); + $input = fileList.$fileList.find('input.filename'); + // keep same name + $input.val('One.txt'); + // trigger submit because triggering blur doesn't work in all browsers + $input.closest('form').trigger('submit'); + + expect(fakeServer.requests.length).toEqual(0); + } function doRename() { var $input, request; @@ -486,12 +502,6 @@ describe('OCA.Files.FileList tests', function() { request = fakeServer.requests[0]; expect(request.url.substr(0, request.url.indexOf('?'))).toEqual(OC.webroot + '/index.php/apps/files/ajax/rename.php'); expect(OC.parseQueryString(request.url)).toEqual({'dir': '/subdir', newname: 'Tu_after_three.txt', file: 'One.txt'}); - - // element is renamed before the request finishes - expect(fileList.findFileEl('One.txt').length).toEqual(0); - expect(fileList.findFileEl('Tu_after_three.txt').length).toEqual(1); - // input is gone - expect(fileList.$fileList.find('input.filename').length).toEqual(0); } it('Inserts renamed file entry at correct position if rename ajax call suceeded', function() { doRename(); @@ -542,22 +552,51 @@ describe('OCA.Files.FileList tests', function() { $tr = fileList.findFileEl('Tu_after_three.txt'); expect($tr.find('a.name').attr('href')).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2Fsubdir&files=Tu_after_three.txt'); }); - // FIXME: fix this in the source code! - xit('Correctly updates file link after rename when path has same name', function() { - var $tr; - // evil case: because of buggy code - $('#dir').val('/One.txt/subdir'); + it('Triggers "fileActionsReady" event after rename', function() { + var handler = sinon.stub(); + fileList.$fileList.on('fileActionsReady', handler); doRename(); - + expect(handler.notCalled).toEqual(true); fakeServer.requests[0].respond(200, {'Content-Type': 'application/json'}, JSON.stringify({ status: 'success', data: { name: 'Tu_after_three.txt' } })); + expect(handler.calledOnce).toEqual(true); + expect(fileList.$fileList.find('.test').length).toEqual(0); + }); + it('Leaves the summary alone when reinserting renamed element', function() { + var $summary = $('#filestable .summary'); + doRename(); + fakeServer.requests[0].respond(200, {'Content-Type': 'application/json'}, JSON.stringify({ + status: 'success', + data: { + name: 'Tu_after_three.txt' + } + })); + expect($summary.find('.info').text()).toEqual('1 folder and 3 files'); + }); + it('Leaves the summary alone when cancel renaming', function() { + var $summary = $('#filestable .summary'); + doCancelRename(); + expect($summary.find('.info').text()).toEqual('1 folder and 3 files'); + }); + it('Hides actions while rename in progress', function() { + var $tr; + doRename(); + // element is renamed before the request finishes $tr = fileList.findFileEl('Tu_after_three.txt'); - expect($tr.find('a.name').attr('href')).toEqual(OC.webroot + '/index.php/apps/files/ajax/download.php?dir=%2Fsubdir&files=One.txt'); + expect($tr.length).toEqual(1); + expect(fileList.findFileEl('One.txt').length).toEqual(0); + // file actions are hidden + expect($tr.find('.action').hasClass('hidden')).toEqual(true); + expect($tr.find('.fileactions').hasClass('hidden')).toEqual(true); + + // input and form are gone + expect(fileList.$fileList.find('input.filename').length).toEqual(0); + expect(fileList.$fileList.find('form').length).toEqual(0); }); }); describe('Moving files', function() {