From 02193445c5ae206362aad90b0bece38f4882d92b Mon Sep 17 00:00:00 2001 From: crdlc Date: Mon, 10 Feb 2014 16:14:04 +0100 Subject: [PATCH] Bug 970267 - [Download Manager] confirmation screen is displayed twice when we try remove a not supported file --- .../js/downloads/download_api_manager.js | 51 +++++++++++++------ apps/settings/js/downloads/downloads_list.js | 13 +++-- .../test/unit/download_api_manager_test.js | 23 ++++++++- 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/apps/settings/js/downloads/download_api_manager.js b/apps/settings/js/downloads/download_api_manager.js index 32f7c90e70af..13edd2fa92cb 100644 --- a/apps/settings/js/downloads/download_api_manager.js +++ b/apps/settings/js/downloads/download_api_manager.js @@ -32,19 +32,38 @@ return DownloadFormatter.getUUID(download); } - function _deleteDownload(id, successCb, errorCb) { + /* + * It deletes a download + * + * @param{Object} It represents a download item that defines the download id + * and if the user's confirmation should be omitted (optional) + * + * @param{Function} Success callback + * + * @param{Function} Error callback + */ + function _deleteDownload(item, successCb, errorCb) { + var id = item.id; var download = downloadsCache[id]; - var reqShow = DownloadUI.show(DownloadUI.TYPE.DELETE, download); - - reqShow.onconfirm = function confirmed() { + function doDeleteDownload() { _deleteFromDownloadsCache(id); var reqRemove = DownloadHelper.remove(download); reqRemove.onsuccess = successCb; reqRemove.onerror = errorCb; - }; + } + + if (item.force) { + doDeleteDownload(); + } else { + var reqShow = DownloadUI.show(DownloadUI.TYPE.DELETE, download); - reqShow.oncancel = errorCb; + reqShow.onconfirm = function confirmed() { + doDeleteDownload(); + }; + + reqShow.oncancel = errorCb; + } } var DownloadApiManager = { @@ -105,34 +124,34 @@ }, deleteDownloads: - function(downloadIds, onDeletedSuccess, onDeletedError, oncomplete) { - if (downloadIds == null) { + function(downloadItems, onDeletedSuccess, onDeletedError, oncomplete) { + if (downloadItems == null) { if (typeof onDeletedError === 'function') { - onDeletedError(null, 'Download IDs not defined or null'); + onDeletedError(null, 'Download items not defined or null'); } return; } - if (downloadIds.length === 0) { + if (downloadItems.length === 0) { if (typeof oncomplete === 'function') { oncomplete(); } return; } - var currentId = downloadIds.pop(); + var currentItem = downloadItems.pop(); var self = this; - _deleteDownload(currentId, function onDelete() { - onDeletedSuccess && onDeletedSuccess(currentId); + _deleteDownload(currentItem, function onDelete() { + onDeletedSuccess && onDeletedSuccess(currentItem.id); self.deleteDownloads( - downloadIds, + downloadItems, onDeletedSuccess, onDeletedError, oncomplete ); }, function onError(msg) { - onDeletedError && onDeletedError(currentId, msg); + onDeletedError && onDeletedError(currentItem.id, msg); self.deleteDownloads( - downloadIds, + downloadItems, onDeletedSuccess, onDeletedError, oncomplete diff --git a/apps/settings/js/downloads/downloads_list.js b/apps/settings/js/downloads/downloads_list.js index 3dd8d2b5e6c0..198d7efaa0d0 100644 --- a/apps/settings/js/downloads/downloads_list.js +++ b/apps/settings/js/downloads/downloads_list.js @@ -215,7 +215,10 @@ var downloadId = DownloadItem.getDownloadId(d); var elementToDelete = _getElementForId(downloadId); DownloadApiManager.deleteDownloads( - [downloadId], + [{ + id: downloadId, + force: true // deleting download without confirmation + }], function onDeleted() { _removeDownloadsFromUI([elementToDelete]); _checkEmptyList(); @@ -282,9 +285,11 @@ function _deleteDownloads() { var downloadsChecked = _getAllChecked() || []; - var downloadIDs = [], downloadElements = {}; + var downloadItems = [], downloadElements = {}; for (var i = 0; i < downloadsChecked.length; i++) { - downloadIDs.push(downloadsChecked[i].value); + downloadItems.push({ + id: downloadsChecked[i].value + }); downloadElements[downloadsChecked[i].value] = downloadsChecked[i].parentNode.parentNode; } @@ -295,7 +300,7 @@ } DownloadApiManager.deleteDownloads( - downloadIDs, + downloadItems, function downloadsDeleted(downloadID) { _removeDownloadsFromUI([downloadElements[downloadID]]); }, diff --git a/apps/settings/test/unit/download_api_manager_test.js b/apps/settings/test/unit/download_api_manager_test.js index 23119d8d33a0..c5d0416bb65d 100644 --- a/apps/settings/test/unit/download_api_manager_test.js +++ b/apps/settings/test/unit/download_api_manager_test.js @@ -121,7 +121,7 @@ suite('DownloadApiManager', function() { }); this.sinon.spy(DownloadHelper, 'remove'); - DownloadApiManager.deleteDownloads([0], function() {}, function() { + DownloadApiManager.deleteDownloads([{id: 0}], function() {}, function() { // Once cancelled, we get the same object var download = DownloadApiManager.getDownload(0); // and the object still exists @@ -134,13 +134,32 @@ suite('DownloadApiManager', function() { test(' > deleteDownload given an ID (user confirms)', function(done) { this.sinon.spy(DownloadHelper, 'remove'); - DownloadApiManager.deleteDownloads([0], function() { + this.sinon.spy(DownloadUI, 'show'); + DownloadApiManager.deleteDownloads([{id: 0}], function() { // Once deleted, we try to get the same object var download = DownloadApiManager.getDownload(0); // Now the object does not exist assert.ok(!download); + sinon.assert.called(DownloadUI.show); assert.ok(DownloadHelper.remove.called); DownloadHelper.remove.restore(); + DownloadUI.show.restore(); + done(); + }); + }); + + test(' > force deleteDownload given an ID', function(done) { + this.sinon.spy(DownloadHelper, 'remove'); + this.sinon.spy(DownloadUI, 'show'); + DownloadApiManager.deleteDownloads([{id: 1, force: true}], function() { + // Once deleted, we try to get the same object + var download = DownloadApiManager.getDownload(1); + // Now the object does not exist + assert.ok(!download); + assert.ok(!DownloadUI.show.called); + sinon.assert.called(DownloadHelper.remove); + DownloadHelper.remove.restore(); + DownloadUI.show.restore(); done(); }); });