Skip to content

Commit

Permalink
Bug 970267 - [Download Manager] confirmation screen is displayed twic…
Browse files Browse the repository at this point in the history
…e when we try remove a not supported file
  • Loading branch information
crdlc committed Feb 21, 2014
1 parent 3c0eb42 commit 0219344
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 22 deletions.
51 changes: 35 additions & 16 deletions apps/settings/js/downloads/download_api_manager.js
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions apps/settings/js/downloads/downloads_list.js
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand All @@ -295,7 +300,7 @@
}

DownloadApiManager.deleteDownloads(
downloadIDs,
downloadItems,
function downloadsDeleted(downloadID) {
_removeDownloadsFromUI([downloadElements[downloadID]]);
},
Expand Down
23 changes: 21 additions & 2 deletions apps/settings/test/unit/download_api_manager_test.js
Expand Up @@ -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
Expand All @@ -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();
});
});
Expand Down

0 comments on commit 0219344

Please sign in to comment.