From b53892fe5458f0af8d63fc84dad640da7b00f883 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 8 Aug 2015 17:26:42 -0400 Subject: [PATCH] Fix various collection-dragging UI bugs Fixes #823, hopefully --- .../zotero/xpcom/collectionTreeView.js | 40 +- .../content/zotero/xpcom/data/collection.js | 4 - .../content/zotero/xpcom/libraryTreeView.js | 18 +- chrome/content/zotero/zoteroPane.js | 2 +- test/content/support.js | 14 +- test/tests/collectionTreeViewTest.js | 440 +++++++++++++----- 6 files changed, 359 insertions(+), 159 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index d4f0bbd83..b58e28589 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -366,18 +366,14 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* this.selection.select(selectedIndex) } } - else if(action == 'move') - { - yield this.reload(); - yield this.restoreSelection(currentTreeRow); - } else if (action == 'modify') { let row; let id = ids[0]; + let rowID = "C" + id; switch (type) { case 'collection': - row = this.getRowIndexByID("C" + id); + row = this.getRowIndexByID(rowID); if (row !== false) { // TODO: Only move if name changed let reopen = this.isContainerOpen(row); @@ -386,10 +382,15 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* } this._removeRow(row); yield this._addSortedRow('collection', id); - if (reopen) { - yield this.toggleOpenState(row); + if (!extraData[id].skipSelect) { + yield this.selectByID(currentTreeRow.id); + if (reopen) { + let newRow = this.getRowIndexByID(rowID); + if (!this.isContainerOpen(newRow)) { + yield this.toggleOpenState(newRow); + } + } } - yield this.restoreSelection(currentTreeRow); } break; @@ -399,13 +400,13 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* // TODO: Only move if name changed this._removeRow(row); yield this._addSortedRow('search', id); - yield this.restoreSelection(currentTreeRow); + yield this.selectByID(currentTreeRow.id); } break; default: yield this.reload(); - yield this.restoreSelection(currentTreeRow); + yield this.selectByID(currentTreeRow.id); break; } } @@ -434,7 +435,7 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* case 'group': yield this.reload(); - yield this.restoreSelection(currentTreeRow); + yield this.selectByID(currentTreeRow.id); break; } } @@ -451,7 +452,7 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* * This only adds a row if it would be visible without opening any containers * * @param {String} objectType - * @param {Integer} id + * @param {Integer} id - collectionID * @return {Integer|false} - Index at which the row was added, or false if it wasn't added */ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(function* (objectType, id) { @@ -904,13 +905,6 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi }); -Zotero.CollectionTreeView.prototype.restoreSelection = Zotero.Promise.coroutine(function* (collectionTreeRow) { - yield this.selectByID(collectionTreeRow.id); - // Swap back in the previous tree row to avoid reselection and subsequent items view refresh - this._rows[this.selection.currentIndex] = collectionTreeRow; -}) - - /** * @param {Integer} libraryID Library to select */ @@ -1817,7 +1811,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r // Collection drag within a library else { droppedCollection.parentID = targetCollectionID; - yield droppedCollection.save(); + yield droppedCollection.saveTx(); } } else if (dataType == 'zotero/item') { @@ -2302,7 +2296,7 @@ Zotero.CollectionTreeRow.prototype.getItems = Zotero.Promise.coroutine(function* }); Zotero.CollectionTreeRow.prototype.getSearchResults = Zotero.Promise.coroutine(function* (asTempTable) { - if (Zotero.CollectionTreeCache.lastTreeRow && Zotero.CollectionTreeCache.lastTreeRow !== this) { + if (Zotero.CollectionTreeCache.lastTreeRow && Zotero.CollectionTreeCache.lastTreeRow.id !== this.id) { Zotero.CollectionTreeCache.clear(); } @@ -2327,7 +2321,7 @@ Zotero.CollectionTreeRow.prototype.getSearchResults = Zotero.Promise.coroutine(f * This accounts for the collection, saved search, quicksearch, tags, etc. */ Zotero.CollectionTreeRow.prototype.getSearchObject = Zotero.Promise.coroutine(function* () { - if(Zotero.CollectionTreeCache.lastTreeRow !== this) { + if (Zotero.CollectionTreeCache.lastTreeRow && Zotero.CollectionTreeCache.lastTreeRow.id !== this.id) { Zotero.CollectionTreeCache.clear(); } diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 106012876..c94f5ad47 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -334,10 +334,6 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env) this.ObjectsClass.unregisterChildCollection(parentCollectionID, collectionID); }.bind(this)); } - - if (!isNew) { - Zotero.Notifier.queue('move', 'collection', collectionID); - } } }); diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js index ede36d300..3f456b088 100644 --- a/chrome/content/zotero/xpcom/libraryTreeView.js +++ b/chrome/content/zotero/xpcom/libraryTreeView.js @@ -131,10 +131,12 @@ Zotero.LibraryTreeView.prototype = { /** * Remove a row from the main array, decrement the row count, tell the treebox that the row - * count changed, delete the row from the map, and optionally update all rows above it in the map + * count changed, update the parent isOpen if necessary, delete the row from the map, and + * optionally update all rows above it in the map */ _removeRow: function (row, skipMapUpdate) { var id = this._rows[row].id; + var level = this.getLevel(row); var lastRow = row == this.rowCount - 1; if (lastRow && this.selection.isSelected(row)) { @@ -161,7 +163,19 @@ Zotero.LibraryTreeView.prototype = { this._rows.splice(row, 1); this.rowCount--; - this._treebox.rowCountChanged(row + 1, -1); + // According to the example on https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsITreeBoxObject#rowCountChanged + // this should start at row + 1 ("rowCountChanged(rowIndex+1, -1);"), but that appears to + // just be wrong. A negative count indicates removed rows, but the index should still + // start at the place where the removals begin, not after it going backward. + this._treebox.rowCountChanged(row, -1); + // Update isOpen if parent and no siblings + if (row != 0 + && this.getLevel(row - 1) < level + && this._rows[row] + && this.getLevel(row) != level) { + this._rows[row - 1].isOpen = false; + this._treebox.invalidateRow(row - 1); + } delete this._rowMap[id]; if (!skipMapUpdate) { for (let i in this._rowMap) { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index d3e3997d5..7840b2444 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -1108,7 +1108,7 @@ var ZoteroPane = new function() return; } - if (this.itemsView && this.itemsView.collectionTreeRow == collectionTreeRow) { + if (this.itemsView && this.itemsView.collectionTreeRow.id == collectionTreeRow.id) { Zotero.debug("Collection selection hasn't changed"); return; } diff --git a/test/content/support.js b/test/content/support.js index 3a3ee62ad..816964900 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -181,13 +181,23 @@ var waitForItemsLoad = Zotero.Promise.coroutine(function* (win, collectionRowToS * Waits for a single item event. Returns a promise for the item ID(s). */ function waitForItemEvent(event) { + return waitForNotifierEvent(event, 'item').then(x => x.ids); +} + +/** + * Wait for a single notifier event and return a promise for the data + */ +function waitForNotifierEvent(event, type) { var deferred = Zotero.Promise.defer(); var notifierID = Zotero.Notifier.registerObserver({notify:function(ev, type, ids, extraData) { if(ev == event) { Zotero.Notifier.unregisterObserver(notifierID); - deferred.resolve(ids); + deferred.resolve({ + ids: ids, + extraData: extraData + }); } - }}, ["item"]); + }}, [type]); return deferred.promise; } diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 4ad3ea0be..742c66fac 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -276,33 +276,39 @@ describe("Zotero.CollectionTreeView", function() { /** * Simulate a drag and drop * - * @param {String} targetRowID - Tree row id (e.g., "L123") - * @param {Integer[]} itemIDs + * @param {String} type - 'item' or 'collection' + * @param {String|Object} targetRow - Tree row id (e.g., "L123"), or { row, orient } + * @param {Integer[]} collectionIDs * @param {Promise} [promise] - If a promise is provided, it will be waited for and its - * value returned after the drag. Otherwise, an item 'add' - * event will be waited for, and the added ids will be - * returned. + * value returned after the drag. Otherwise, an 'add' event will be waited for, and + * an object with 'ids' and 'extraData' will be returned. */ - var drop = Zotero.Promise.coroutine(function* (targetRowID, itemIDs, promise) { - var row = cv.getRowIndexByID(targetRowID); + var drop = Zotero.Promise.coroutine(function* (objectType, targetRow, ids, promise) { + if (typeof targetRow == 'string') { + var row = cv.getRowIndexByID(targetRow); + var orient = 0; + } + else { + var { row, orient } = targetRow; + } var stub = sinon.stub(Zotero.DragDrop, "getDragTarget"); stub.returns(cv.getRow(row)); if (!promise) { - promise = waitForItemEvent("add"); + promise = waitForNotifierEvent("add", objectType); } - yield cv.drop(row, 0, { + yield cv.drop(row, orient, { dropEffect: 'copy', effectAllowed: 'copy', - mozSourceNode: win.document.getElementById('zotero-items-tree'), + mozSourceNode: win.document.getElementById(`zotero-${objectType}s-tree`), types: { contains: function (type) { - return type == 'zotero/item'; + return type == `zotero/${objectType}`; } }, getData: function (type) { - if (type == 'zotero/item') { - return itemIDs.join(","); + if (type == `zotero/${objectType}`) { + return ids.join(","); } } }); @@ -314,7 +320,7 @@ describe("Zotero.CollectionTreeView", function() { }); - var canDrop = Zotero.Promise.coroutine(function* (targetRowID, itemIDs) { + var canDrop = Zotero.Promise.coroutine(function* (type, targetRowID, ids) { var row = cv.getRowIndexByID(targetRowID); var stub = sinon.stub(Zotero.DragDrop, "getDragTarget"); @@ -322,15 +328,15 @@ describe("Zotero.CollectionTreeView", function() { var dt = { dropEffect: 'copy', effectAllowed: 'copy', - mozSourceNode: win.document.getElementById('zotero-items-tree'), + mozSourceNode: win.document.getElementById(`zotero-${type}s-tree`), types: { contains: function (type) { - return type == 'zotero/item'; + return type == `zotero/${type}`; } }, getData: function (type) { - if (type == 'zotero/item') { - return itemIDs.join(","); + if (type == `zotero/${type}`) { + return ids.join(","); } } }; @@ -342,130 +348,310 @@ describe("Zotero.CollectionTreeView", function() { return canDrop; }); + describe("with items", function () { + it("should add an item to a collection", function* () { + var collection = yield createDataObject('collection', false, { skipSelect: true }); + var item = yield createDataObject('item', false, { skipSelect: true }); + + // Add observer to wait for collection add + var deferred = Zotero.Promise.defer(); + var observerID = Zotero.Notifier.registerObserver({ + notify: function (event, type, ids, extraData) { + if (type == 'collection-item' && event == 'add' + && ids[0] == collection.id + "-" + item.id) { + setTimeout(function () { + deferred.resolve(); + }); + } + } + }, 'collection-item', 'test'); + + yield drop('item', 'C' + collection.id, [item.id], deferred.promise); + + Zotero.Notifier.unregisterObserver(observerID); + + yield cv.selectCollection(collection.id); + yield waitForItemsLoad(win); + + var itemsView = win.ZoteroPane.itemsView + assert.equal(itemsView.rowCount, 1); + var treeRow = itemsView.getRow(0); + assert.equal(treeRow.ref.id, item.id); + }) + + it("should copy an item with an attachment to a group", function* () { + var group = yield createGroup(); + + var item = yield createDataObject('item', false, { skipSelect: true }); + var file = getTestDataDirectory(); + file.append('test.png'); + var attachment = yield Zotero.Attachments.importFromFile({ + file: file, + parentItemID: item.id + }); + + // Hack to unload relations to test proper loading + // + // Probably need a better method for this + item._loaded.relations = false; + attachment._loaded.relations = false; + + var ids = (yield drop('item', 'L' + group.libraryID, [item.id])).ids; + + yield cv.selectLibrary(group.libraryID); + yield waitForItemsLoad(win); + + // Check parent + var itemsView = win.ZoteroPane.itemsView; + assert.equal(itemsView.rowCount, 1); + var treeRow = itemsView.getRow(0); + assert.equal(treeRow.ref.libraryID, group.libraryID); + assert.equal(treeRow.ref.id, ids[0]); + // New item should link back to original + var linked = yield item.getLinkedItem(group.libraryID); + assert.equal(linked.id, treeRow.ref.id); + + // Check attachment + assert.isTrue(itemsView.isContainer(0)); + yield itemsView.toggleOpenState(0); + assert.equal(itemsView.rowCount, 2); + treeRow = itemsView.getRow(1); + assert.equal(treeRow.ref.id, ids[1]); + // New attachment should link back to original + linked = yield attachment.getLinkedItem(group.libraryID); + assert.equal(linked.id, treeRow.ref.id); + + return group.eraseTx(); + }) + + it("should not copy an item or its attachment to a group twice", function* () { + var group = yield getGroup(); + + var itemTitle = Zotero.Utilities.randomString(); + var item = yield createDataObject('item', false, { skipSelect: true }); + var file = getTestDataDirectory(); + file.append('test.png'); + var attachment = yield Zotero.Attachments.importFromFile({ + file: file, + parentItemID: item.id + }); + var attachmentTitle = Zotero.Utilities.randomString(); + attachment.setField('title', attachmentTitle); + yield attachment.saveTx(); + + yield drop('item', 'L' + group.libraryID, [item.id]); + assert.isFalse(yield canDrop('item', 'L' + group.libraryID, [item.id])); + }) + + it("should remove a linked, trashed item in a group from the trash and collections", function* () { + var group = yield getGroup(); + var collection = yield createDataObject('collection', { libraryID: group.libraryID }); + + var item = yield createDataObject('item', false, { skipSelect: true }); + yield drop('item', 'L' + group.libraryID, [item.id]); + + var droppedItem = yield item.getLinkedItem(group.libraryID); + droppedItem.setCollections([collection.id]); + droppedItem.deleted = true; + yield droppedItem.saveTx(); + + // Add observer to wait for collection add + var deferred = Zotero.Promise.defer(); + var observerID = Zotero.Notifier.registerObserver({ + notify: function (event, type, ids) { + if (event == 'refresh' && type == 'trash' && ids[0] == group.libraryID) { + setTimeout(function () { + deferred.resolve(); + }); + } + } + }, 'trash', 'test'); + yield drop('item', 'L' + group.libraryID, [item.id], deferred.promise); + Zotero.Notifier.unregisterObserver(observerID); + + assert.isFalse(droppedItem.deleted); + // Should be removed from collections when removed from trash + assert.lengthOf(droppedItem.getCollections(), 0); + }) + }) - it("should add an item to a collection", function* () { - var collection = yield createDataObject('collection', false, { skipSelect: true }); - var item = yield createDataObject('item', false, { skipSelect: true }); + + describe("with collections", function () { + it("should make a subcollection top-level", function* () { + var collection1 = yield createDataObject('collection', { name: "A" }, { skipSelect: true }); + var collection2 = yield createDataObject('collection', { name: "C" }, { skipSelect: true }); + var collection3 = yield createDataObject('collection', { name: "D" }, { skipSelect: true }); + var collection4 = yield createDataObject('collection', { name: "B", parentKey: collection2.key }); + + var colIndex1 = cv.getRowIndexByID('C' + collection1.id); + var colIndex2 = cv.getRowIndexByID('C' + collection2.id); + var colIndex3 = cv.getRowIndexByID('C' + collection3.id); + var colIndex4 = cv.getRowIndexByID('C' + collection4.id); + + // Add observer to wait for collection add + var deferred = Zotero.Promise.defer(); + var observerID = Zotero.Notifier.registerObserver({ + notify: function (event, type, ids, extraData) { + if (type == 'collection' && event == 'modify' && ids[0] == collection4.id) { + setTimeout(function () { + deferred.resolve(); + }, 50); + } + } + }, 'collection', 'test'); + + yield drop( + 'collection', + { + row: 0, + orient: 1 + }, + [collection4.id], + deferred.promise + ); + + Zotero.Notifier.unregisterObserver(observerID); + + var newColIndex1 = cv.getRowIndexByID('C' + collection1.id); + var newColIndex2 = cv.getRowIndexByID('C' + collection2.id); + var newColIndex3 = cv.getRowIndexByID('C' + collection3.id); + var newColIndex4 = cv.getRowIndexByID('C' + collection4.id); + + assert.equal(newColIndex1, colIndex1); + assert.isBelow(newColIndex4, newColIndex2); + assert.isBelow(newColIndex2, newColIndex3); + assert.equal(cv.getRow(newColIndex4).level, cv.getRow(newColIndex1).level); + }) + + it("should move a subcollection and its subcollection down under another collection", function* () { + var collectionA = yield createDataObject('collection', { name: "A" }, { skipSelect: true }); + var collectionB = yield createDataObject('collection', { name: "B", parentKey: collectionA.key }); + var collectionC = yield createDataObject('collection', { name: "C", parentKey: collectionB.key }); + var collectionD = yield createDataObject('collection', { name: "D" }, { skipSelect: true }); + var collectionE = yield createDataObject('collection', { name: "E" }, { skipSelect: true }); + var collectionF = yield createDataObject('collection', { name: "F" }, { skipSelect: true }); + var collectionG = yield createDataObject('collection', { name: "G", parentKey: collectionD.key }); + var collectionH = yield createDataObject('collection', { name: "H", parentKey: collectionG.key }); + + var colIndexA = cv.getRowIndexByID('C' + collectionA.id); + var colIndexB = cv.getRowIndexByID('C' + collectionB.id); + var colIndexC = cv.getRowIndexByID('C' + collectionC.id); + var colIndexD = cv.getRowIndexByID('C' + collectionD.id); + var colIndexE = cv.getRowIndexByID('C' + collectionE.id); + var colIndexF = cv.getRowIndexByID('C' + collectionF.id); + var colIndexG = cv.getRowIndexByID('C' + collectionG.id); + var colIndexH = cv.getRowIndexByID('C' + collectionH.id); + + yield cv.selectCollection(collectionG.id); + + // Add observer to wait for collection add + var deferred = Zotero.Promise.defer(); + var observerID = Zotero.Notifier.registerObserver({ + notify: function (event, type, ids, extraData) { + if (type == 'collection' && event == 'modify' && ids[0] == collectionG.id) { + setTimeout(function () { + deferred.resolve(); + }, 50); + } + } + }, 'collection', 'test'); + + yield drop( + 'collection', + { + row: colIndexE, + orient: 0 + }, + [collectionG.id], + deferred.promise + ); + + Zotero.Notifier.unregisterObserver(observerID); + + var newColIndexA = cv.getRowIndexByID('C' + collectionA.id); + var newColIndexB = cv.getRowIndexByID('C' + collectionB.id); + var newColIndexC = cv.getRowIndexByID('C' + collectionC.id); + var newColIndexD = cv.getRowIndexByID('C' + collectionD.id); + var newColIndexE = cv.getRowIndexByID('C' + collectionE.id); + var newColIndexF = cv.getRowIndexByID('C' + collectionF.id); + var newColIndexG = cv.getRowIndexByID('C' + collectionG.id); + var newColIndexH = cv.getRowIndexByID('C' + collectionH.id); + + assert.isFalse(cv.isContainerOpen(newColIndexD)); + assert.isTrue(cv.isContainerEmpty(newColIndexD)); + assert.isTrue(cv.isContainerOpen(newColIndexE)); + assert.isFalse(cv.isContainerEmpty(newColIndexE)); + assert.equal(newColIndexE, newColIndexG - 1); + assert.equal(newColIndexG, newColIndexH - 1); + + // TODO: Check deeper subcollection open states + }) + }) + + it("should move a subcollection and its subcollection up under another collection", function* () { + var collectionA = yield createDataObject('collection', { name: "A" }, { skipSelect: true }); + var collectionB = yield createDataObject('collection', { name: "B", parentKey: collectionA.key }); + var collectionC = yield createDataObject('collection', { name: "C", parentKey: collectionB.key }); + var collectionD = yield createDataObject('collection', { name: "D" }, { skipSelect: true }); + var collectionE = yield createDataObject('collection', { name: "E" }, { skipSelect: true }); + var collectionF = yield createDataObject('collection', { name: "F" }, { skipSelect: true }); + var collectionG = yield createDataObject('collection', { name: "G", parentKey: collectionE.key }); + var collectionH = yield createDataObject('collection', { name: "H", parentKey: collectionG.key }); + + var colIndexA = cv.getRowIndexByID('C' + collectionA.id); + var colIndexB = cv.getRowIndexByID('C' + collectionB.id); + var colIndexC = cv.getRowIndexByID('C' + collectionC.id); + var colIndexD = cv.getRowIndexByID('C' + collectionD.id); + var colIndexE = cv.getRowIndexByID('C' + collectionE.id); + var colIndexF = cv.getRowIndexByID('C' + collectionF.id); + var colIndexG = cv.getRowIndexByID('C' + collectionG.id); + var colIndexH = cv.getRowIndexByID('C' + collectionH.id); + + yield cv.selectCollection(collectionG.id); // Add observer to wait for collection add var deferred = Zotero.Promise.defer(); var observerID = Zotero.Notifier.registerObserver({ - notify: function (event, type, ids) { - if (type == 'collection-item' && event == 'add' - && ids[0] == collection.id + "-" + item.id) { + notify: function (event, type, ids, extraData) { + if (type == 'collection' && event == 'modify' && ids[0] == collectionG.id) { setTimeout(function () { deferred.resolve(); - }); + }, 50); } } - }, 'collection-item', 'test'); + }, 'collection', 'test'); - var ids = yield drop("C" + collection.id, [item.id], deferred.promise); + yield Zotero.Promise.delay(2000); + + yield drop( + 'collection', + { + row: colIndexD, + orient: 0 + }, + [collectionG.id], + deferred.promise + ); Zotero.Notifier.unregisterObserver(observerID); - yield cv.selectCollection(collection.id); - yield waitForItemsLoad(win); + var newColIndexA = cv.getRowIndexByID('C' + collectionA.id); + var newColIndexB = cv.getRowIndexByID('C' + collectionB.id); + var newColIndexC = cv.getRowIndexByID('C' + collectionC.id); + var newColIndexD = cv.getRowIndexByID('C' + collectionD.id); + var newColIndexE = cv.getRowIndexByID('C' + collectionE.id); + var newColIndexF = cv.getRowIndexByID('C' + collectionF.id); + var newColIndexG = cv.getRowIndexByID('C' + collectionG.id); + var newColIndexH = cv.getRowIndexByID('C' + collectionH.id); - var itemsView = win.ZoteroPane.itemsView - assert.equal(itemsView.rowCount, 1); - var treeRow = itemsView.getRow(0); - assert.equal(treeRow.ref.id, item.id); - }) - - it("should copy an item with an attachment to a group", function* () { - var group = yield createGroup(); + assert.isFalse(cv.isContainerOpen(newColIndexE)); + assert.isTrue(cv.isContainerEmpty(newColIndexE)); + assert.isTrue(cv.isContainerOpen(newColIndexD)); + assert.isFalse(cv.isContainerEmpty(newColIndexD)); + assert.equal(newColIndexD, newColIndexG - 1); + assert.equal(newColIndexG, newColIndexH - 1); - var item = yield createDataObject('item', false, { skipSelect: true }); - var file = getTestDataDirectory(); - file.append('test.png'); - var attachment = yield Zotero.Attachments.importFromFile({ - file: file, - parentItemID: item.id - }); - - // Hack to unload relations to test proper loading - // - // Probably need a better method for this - item._loaded.relations = false; - attachment._loaded.relations = false; - - var ids = yield drop("L" + group.libraryID, [item.id]); - - yield cv.selectLibrary(group.libraryID); - yield waitForItemsLoad(win); - - // Check parent - var itemsView = win.ZoteroPane.itemsView; - assert.equal(itemsView.rowCount, 1); - var treeRow = itemsView.getRow(0); - assert.equal(treeRow.ref.libraryID, group.libraryID); - assert.equal(treeRow.ref.id, ids[0]); - // New item should link back to original - var linked = yield item.getLinkedItem(group.libraryID); - assert.equal(linked.id, treeRow.ref.id); - - // Check attachment - assert.isTrue(itemsView.isContainer(0)); - yield itemsView.toggleOpenState(0); - assert.equal(itemsView.rowCount, 2); - treeRow = itemsView.getRow(1); - assert.equal(treeRow.ref.id, ids[1]); - // New attachment should link back to original - linked = yield attachment.getLinkedItem(group.libraryID); - assert.equal(linked.id, treeRow.ref.id); - - return group.eraseTx(); - }) - - it("should not copy an item or its attachment to a group twice", function* () { - var group = yield getGroup(); - - var itemTitle = Zotero.Utilities.randomString(); - var item = yield createDataObject('item', false, { skipSelect: true }); - var file = getTestDataDirectory(); - file.append('test.png'); - var attachment = yield Zotero.Attachments.importFromFile({ - file: file, - parentItemID: item.id - }); - var attachmentTitle = Zotero.Utilities.randomString(); - attachment.setField('title', attachmentTitle); - yield attachment.saveTx(); - - var ids = yield drop("L" + group.libraryID, [item.id]); - assert.isFalse(yield canDrop("L" + group.libraryID, [item.id])); - }) - - it("should remove a linked, trashed item in a group from the trash and collections", function* () { - var group = yield getGroup(); - var collection = yield createDataObject('collection', { libraryID: group.libraryID }); - - var item = yield createDataObject('item', false, { skipSelect: true }); - var ids = yield drop("L" + group.libraryID, [item.id]); - - var droppedItem = yield item.getLinkedItem(group.libraryID); - droppedItem.setCollections([collection.id]); - droppedItem.deleted = true; - yield droppedItem.saveTx(); - - // Add observer to wait for collection add - var deferred = Zotero.Promise.defer(); - var observerID = Zotero.Notifier.registerObserver({ - notify: function (event, type, ids) { - if (event == 'refresh' && type == 'trash' && ids[0] == group.libraryID) { - setTimeout(function () { - deferred.resolve(); - }); - } - } - }, 'trash', 'test'); - var ids = yield drop("L" + group.libraryID, [item.id], deferred.promise); - Zotero.Notifier.unregisterObserver(observerID); - - assert.isFalse(droppedItem.deleted); - // Should be removed from collections when removed from trash - assert.lengthOf(droppedItem.getCollections(), 0); + // TODO: Check deeper subcollection open states }) }) })