www

commit f93e6706fa6168f7df1510d54f321a20df47790f
parent b43a735b93051da1cd867d5eed1927b797973350
Author: Dan Stillman <dstillman@zotero.org>
Date:   Mon,  8 Jun 2015 04:04:38 -0400

Don't refresh collections view when removing groups

Remove rows one by one

Diffstat:
M
chrome/content/zotero/xpcom/collectionTreeView.js
 | 
41
+++++++++++++++++++----------------------
M
chrome/content/zotero/xpcom/data/item.js
 | 
4
+++-
M
test/tests/collectionTreeViewTest.js
 | 
37
++++++++++++++++++++++++++++++++-----

3 files changed, 54 insertions(+), 28 deletions(-)
diff --git 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<len; i++)
-			{
-				var row = rows[i];
+			for (let i = rows.length - 1; 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
@@ -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
@@ -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 () {