diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 5ac8e894b..494813428 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -305,44 +305,41 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* if (action == 'delete') { var selectedIndex = this.selection.count ? this.selection.currentIndex : 0; - //Since a delete involves shifting of rows, we have to do it in order - - //sort the ids by row - var rows = []; - for (var i in ids) - { - switch (type) - { + // Since a delete involves shifting of rows, we have to do it in reverse order + let rows = []; + for (let i = 0; i < ids.length; i++) { + let id = ids[i]; + switch (type) { case 'collection': - if (typeof this._rowMap['C' + ids[i]] != 'undefined') { - rows.push(this._rowMap['C' + ids[i]]); + if (this._rowMap['C' + id] !== undefined) { + rows.push(this._rowMap['C' + id]); } break; case 'search': - if (typeof this._rowMap['S' + ids[i]] != 'undefined') { - rows.push(this._rowMap['S' + ids[i]]); + if (this._rowMap['S' + id] !== undefined) { + rows.push(this._rowMap['S' + id]); } break; case 'group': - // For now, just reload if a group is removed, since otherwise - // we'd have to remove collections too - yield this.reload(); - this.rememberSelection(savedSelection); + let row = this.getRowIndexByID("L" + extraData[id].libraryID); + let groupLevel = this.getLevel(row); + do { + rows.push(row); + row++; + } + while (row < this.rowCount && this.getLevel(row) > groupLevel); break; } } - if(rows.length > 0) - { + if (rows.length > 0) { rows.sort(function(a,b) { return a-b }); - for(var i=0, len=rows.length; i= 0; i--) { + let row = rows[i]; this._removeRow(row); - this._treebox.rowCountChanged(row, -1); } this._refreshRowMap(); diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 56d245700..39dd0fd14 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3743,7 +3743,9 @@ Zotero.Item.prototype._eraseData = Zotero.Promise.coroutine(function* (env) { } var parentItem = this.parentKey; - parentItem = parentItem ? this.ObjectsClass.getByLibraryAndKey(this.libraryID, parentItem) : null; + parentItem = parentItem + ? (yield this.ObjectsClass.getByLibraryAndKeyAsync(this.libraryID, parentItem)) + : null; // // Delete associated attachment files if (this.isAttachment()) { diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index b490b9212..3e60325ae 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -1,11 +1,11 @@ "use strict"; describe("Zotero.CollectionTreeView", function() { - var win, collectionsView; + var win, collectionsView, cv; before(function* () { win = yield loadZoteroPane(); - collectionsView = win.ZoteroPane.collectionsView; + cv = collectionsView = win.ZoteroPane.collectionsView; }); beforeEach(function () { // TODO: Add a selectCollection() function and select a collection instead? @@ -18,7 +18,6 @@ describe("Zotero.CollectionTreeView", function() { describe("collapse/expand", function () { it("should close and open My Library repeatedly", function* () { var libraryID = Zotero.Libraries.userLibraryID; - var cv = collectionsView; yield cv.selectLibrary(libraryID); var row = cv.selection.currentIndex; @@ -50,7 +49,6 @@ describe("Zotero.CollectionTreeView", function() { describe("#expandToCollection()", function () { it("should expand a collection to a subcollection", function* () { - var cv = collectionsView; var collection1 = yield createDataObject('collection'); var collection2 = createUnsavedDataObject('collection'); collection2.parentID = collection1.id; @@ -166,7 +164,6 @@ describe("Zotero.CollectionTreeView", function() { search.addCondition('title', 'contains', 'test'); var searchID = yield search.saveTx(); - var cv = win.ZoteroPane.collectionsView; var collectionRow = cv._rowMap["C" + collectionID]; var searchRow = cv._rowMap["S" + searchID]; var duplicatesRow = cv._rowMap["D" + Zotero.Libraries.userLibraryID]; @@ -186,6 +183,36 @@ describe("Zotero.CollectionTreeView", function() { assert.isBelow(searchRow, trashRow); } }) + + it("should remove a group and all children", function* () { + // Make sure Group Libraries separator and header exist already, + // since otherwise they'll interfere with the count + yield getGroup(); + + var originalRowCount = cv.rowCount; + + var group = yield createGroup(); + yield createDataObject('collection', { libraryID: group.libraryID }); + var c = yield createDataObject('collection', { libraryID: group.libraryID }); + yield createDataObject('collection', { libraryID: group.libraryID, parentID: c.id }); + yield createDataObject('collection', { libraryID: group.libraryID }); + yield createDataObject('collection', { libraryID: group.libraryID }); + + // Group, collections, and trash + assert.equal(cv.rowCount, originalRowCount + 7); + + var spy = sinon.spy(cv, "refresh"); + try { + yield group.eraseTx(); + + assert.equal(cv.rowCount, originalRowCount); + // Make sure the tree wasn't refreshed + sinon.assert.notCalled(spy); + } + finally { + spy.restore(); + } + }) }) describe("#drop()", function () {