diff --git a/chrome/content/zotero/elements/librariesCollectionsBox.js b/chrome/content/zotero/elements/librariesCollectionsBox.js index b51ee68457..154e7a3d87 100644 --- a/chrome/content/zotero/elements/librariesCollectionsBox.js +++ b/chrome/content/zotero/elements/librariesCollectionsBox.js @@ -65,7 +65,7 @@ import { getCSSIcon } from 'components/icons'; } init() { - this._notifierID = Zotero.Notifier.registerObserver(this, ['item'], 'librariesCollectionsBox'); + this._notifierID = Zotero.Notifier.registerObserver(this, ['item', 'collection'], 'librariesCollectionsBox'); this._body = this.querySelector('.body'); this.initCollapsibleSection(); this._section.addEventListener('add', this._handleAdd); @@ -77,11 +77,19 @@ import { getCSSIcon } from 'components/icons'; } notify(action, type, ids) { + if (!this._item) return; if (action == 'modify' - && this._item + && type == "item" && (ids.includes(this._item.id) || this._linkedItems.some(item => ids.includes(item.id)))) { this._forceRenderAll(); } + + if (["modify", "trash"].includes(action) && type == "collection") { + let isRelevantCollection = ids.some(id => Zotero.Collections.get(id).hasItem(this._item)); + if (isRelevantCollection) { + this._forceRenderAll(); + } + } } _buildRow(obj, level, contextItem) { diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 85b7fc26d0..10dab3c779 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -357,8 +357,8 @@ Zotero.Collection.prototype._saveData = Zotero.Promise.coroutine(function* (env) // Add restored collection back into item's _collections cache this.getChildItems(false, true).forEach((item) => { - const collectionNotCached = item._collections.filter(c => c != this.id).length == 0; - if (collectionNotCached) { + let collectionCached = item._collections.includes(this.id); + if (!collectionCached) { item._collections.push(this.id); } }); diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 17bea222c7..1f32eafbde 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -797,7 +797,8 @@ Zotero.Items = function() { this._loadCollections = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { var sql = "SELECT itemID, collectionID FROM items " + "LEFT JOIN collectionItems USING (itemID) " - + "WHERE libraryID=?" + idSQL; + + "LEFT JOIN deletedCollections USING (collectionID) " + + "WHERE deletedCollections.collectionID IS NULL AND libraryID=?" + idSQL; var params = [libraryID]; var lastItemID; diff --git a/test/tests/collectionTest.js b/test/tests/collectionTest.js index 47d1e44d4d..fc80254369 100644 --- a/test/tests/collectionTest.js +++ b/test/tests/collectionTest.js @@ -94,7 +94,8 @@ describe("Zotero.Collection", function() { it("should restore deleted collection", async function () { var collection1 = await createDataObject('collection'); - var item1 = await createDataObject('item', { collections: [collection1.id] }); + var collection2 = await createDataObject('collection'); + var item1 = await createDataObject('item', { collections: [collection1.id, collection2.id] }); assert.include(item1.getCollections(), collection1.id); @@ -128,6 +129,26 @@ describe("Zotero.Collection", function() { assert.equal(await Zotero.Collections.getAsync(collection2.id), false); assert.equal(await Zotero.Collections.getAsync(collection3.id), false); }); + + it("should not load deleted collections into item's cache on startup", async function () { + var collection1 = await createDataObject('collection'); + var collection2 = await createDataObject('collection'); + var item = await createDataObject('item', { collections: [collection1.id, collection2.id] }); + + collection1.deleted = true; + await collection1.saveTx(); + + // Make sure the deleted collection is gone from item's cache + assert.notInclude(item.getCollections(), collection1.id); + assert.include(item.getCollections(), collection2.id); + + // Reload the item's data from DB + await Zotero.Items.get(item.id).reload(null, true); + + // Make sure the deleted collection is not back in item's cache + assert.notInclude(item.getCollections(), collection1.id); + assert.include(item.getCollections(), collection2.id); + }); }) describe("#version", function () { diff --git a/test/tests/itemPaneTest.js b/test/tests/itemPaneTest.js index 2f8e3d73bf..dd2de97ec0 100644 --- a/test/tests/itemPaneTest.js +++ b/test/tests/itemPaneTest.js @@ -394,6 +394,60 @@ describe("Item pane", function () { }); }); + describe("Libraries and collections pane", function () { + var item, collectionParent, collectionChild, section; + + // Fresh setup of an item belonging to 2 collections - parent and child - for each test + beforeEach(async function () { + collectionParent = await createDataObject('collection'); + collectionChild = await createDataObject('collection', { parentID: collectionParent.id }); + item = await createDataObject('item', { collections: [collectionParent.id, collectionChild.id] }); + await ZoteroPane.selectItem(item.id); + section = ZoteroPane.itemPane._itemDetails.getPane("libraries-collections"); + }); + + it("should update collection's name after rename", async function () { + collectionChild.name = "Updated collection name"; + collectionChild.saveTx(); + + await waitForNotifierEvent('modify', 'collection'); + + let collectionRow = section.querySelector(`.row[data-id="C${collectionChild.id}"]`); + assert.equal(collectionRow.innerText, collectionChild.name); + }); + + it("should remove collection that has been trashed", async function () { + collectionChild.deleted = true; + collectionChild.saveTx(); + + await waitForNotifierEvent('trash', 'collection'); + + let rowIDs = [...section.querySelectorAll(".row")].map(node => node.dataset.id); + assert.deepEqual(rowIDs, [`L${item.libraryID}`, `C${collectionParent.id}`]); + }); + + it("should bring back collection restored from trash", async function () { + collectionChild.deleted = true; + collectionChild.saveTx(); + + await waitForNotifierEvent('trash', 'collection'); + + // Make sure the collection is actually gone + let rowIDs = [...section.querySelectorAll(".row")].map(node => node.dataset.id); + assert.deepEqual(rowIDs, [`L${item.libraryID}`, `C${collectionParent.id}`]); + + // Restore the collection from trash + collectionChild.deleted = false; + collectionChild.saveTx(); + + await waitForNotifierEvent('modify', 'collection'); + + // The collection row should appear again + rowIDs = [...section.querySelectorAll(".row")].map(node => node.dataset.id); + assert.deepEqual(rowIDs, [`L${item.libraryID}`, `C${collectionParent.id}`, `C${collectionChild.id}`]); + }); + }); + describe("Attachments pane", function () { let paneID = "attachments";