mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Revised logic of trashed collections in item cache (#4358)

Browse source 
- revert change from 2401a34031
that only loads un-trashed collections in _loadCollections.
If an item only belongs to deleted collections, item._loaded.collections = true
from _loadCollections will never run, so an exception
will be thrown in item.toJSON() when syncing happens.
Instead, to address the problem of item.getCollections()
having stale data #4307, add 'includeTrashed' parameter to
item.getCollections() based on which item._collections
will be filtered. Fixes: #4346
- revert earlier, no more necessary, changes from a532cfb475
to not alter item._collections cache when collections are being trashed or restored.
Collection is removed from item._collections only when it is permanently
erased.
- removed unnecessary test checking for consistent item._collections
value before and after reload, since item._collections is no longer
modified
- fix encountered bug where a trashed child collection is not
unloaded if a parent collection is erased without being trashed first.
- tweaked Zotero.Search sql construction to count items
that only belong to trashed collections into 'unfiled'. Fixes: #4347

---------

Co-authored-by: Dan Stillman
This commit is contained in:
abaevbog 2024-07-09 00:34:12 -07:00 • committed by GitHub
parent 58c445b616
commit 2d3375e9f6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 97 additions and 59 deletions
Download patch file Download diff file Expand all files Collapse all files

31
test/tests/collectionTest.js
View file

@ -102,8 +102,10 @@ describe("Zotero.Collection", function() {
collection1.deleted = true;
await collection1.saveTx();
// Deleted collection is gone from item's cache
// Trashed collection does not count as one of item's containers
assert.notInclude(item1.getCollections(), collection1.id);
// But it should still return it if includeTrashed=true is passed
assert.include(item1.getCollections(true), collection1.id);
// Restore deleted collection
collection1.deleted = false;
@ -114,40 +116,25 @@ describe("Zotero.Collection", function() {
// Collection is restored from trash
assert.notInclude(deleted, collection1.id);
// Collection is back in item's cache
// Item belongs to the restored collection
assert.include(item1.getCollections(), collection1.id);
});
it("should permanently delete collections from trash", async function () {
var collection1 = await createDataObject('collection');
var collection2 = await createDataObject('collection', { parentID: collection1.id });
var collection3 = await createDataObject('collection', { parentID: collection2.id });
var collection3 = await createDataObject('collection', { parentID: collection2.id, deleted: true });
var item = await createDataObject('item', { collections: [collection1.id, collection2.id, collection3.id] });
await collection1.eraseTx();
assert.equal(await Zotero.Collections.getAsync(collection1.id), false);
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);
// Erased collections are fully removed as item's containers
assert.equal(item.getCollections().length, 0);
assert.equal(item.getCollections(true).length, 0);
});
})