From b3220e83b1a3c5bd797b73b3e8b7db21ab9773a4 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Mon, 25 Jan 2021 03:15:26 -0500
Subject: [PATCH] Don't pass nested collections to translators as top-level
 collections

While we need to pass all items from descendant collections to
translators, only the immediate child collections of the selected
library or collection should be returned from Zotero.nextCollection().

I'm not sure how long we've been doing this wrong, but it resulted in
duplicated subcollections when round-tripping Zotero RDF. (TEI is the
only other translator that uses nextCollection(), so its exports were
probably similarly incorrect.)

https://forums.zotero.org/discussion/87135/export-import-creates-numerous-duplicate-subcollections
---
 .../xpcom/translation/translate_item.js       | 13 ++--
 test/tests/translateTest.js                   | 64 +++++++++++++++++++
 2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/chrome/content/zotero/xpcom/translation/translate_item.js b/chrome/content/zotero/xpcom/translation/translate_item.js
index 7e652b9cbb..1a9b029bc7 100644
--- a/chrome/content/zotero/xpcom/translation/translate_item.js
+++ b/chrome/content/zotero/xpcom/translation/translate_item.js
@@ -1021,12 +1021,13 @@ Zotero.Translate.ItemGetter.prototype = {
 		var items = new Set(collection.getChildItems());
 		
 		if(getChildCollections) {
-			// get child collections
-			this._collectionsLeft = Zotero.Collections.getByParent(collection.id, true);
+			// Get child collections
+			this._collectionsLeft = Zotero.Collections.getByParent(collection.id);
 			
-			// get items in child collections
-			for (let collection of this._collectionsLeft) {
-				var childItems = collection.getChildItems();
+			// Get items in all descendant collections
+			let descendantCollections = Zotero.Collections.getByParent(collection.id, true);
+			for (let collection of descendantCollections) {
+				let childItems = collection.getChildItems();
 				childItems.forEach(item => items.add(item));
 			}
 		}
@@ -1040,7 +1041,7 @@ Zotero.Translate.ItemGetter.prototype = {
 		this._itemsLeft = yield Zotero.Items.getAll(libraryID, true);
 		
 		if(getChildCollections) {
-			this._collectionsLeft = Zotero.Collections.getByLibrary(libraryID, true);
+			this._collectionsLeft = Zotero.Collections.getByLibrary(libraryID);
 		}
 		
 		this._itemsLeft.sort(function(a, b) { return a.id - b.id; });
diff --git a/test/tests/translateTest.js b/test/tests/translateTest.js
index 6dfc9b2790..6ee924da05 100644
--- a/test/tests/translateTest.js
+++ b/test/tests/translateTest.js
@@ -1148,6 +1148,70 @@ describe("Zotero.Translate", function() {
 			var attachment = Zotero.Items.get(attachments[0]);
 			assert.isTrue(yield attachment.fileExists());
 		});
+		
+		it("should round-trip collections via Zotero RDF", async function () {
+			this.timeout(60000);
+			await resetDB();
+			
+			var c1 = await createDataObject('collection', { name: '1' });
+			var c2 = await createDataObject('collection', { name: '2', parentID: c1.id });
+			var c3 = await createDataObject('collection', { name: '3', parentID: c2.id });
+			var c4 = await createDataObject('collection', { name: '4', parentID: c3.id });
+			var c5 = await createDataObject('collection', { name: '5', parentID: c4.id });
+			var item = await createDataObject('item', { collections: [c5.id] });
+			
+			var tmpDir = await getTempDirectory();
+			var libraryExportFile = OS.Path.join(tmpDir, 'export-library.rdf');
+			var collectionExportFile = OS.Path.join(tmpDir, 'export-collection.rdf');
+			
+			// Export library
+			var translation = new Zotero.Translate.Export();
+			translation.setLocation(Zotero.File.pathToFile(libraryExportFile));
+			translation.setLibraryID(Zotero.Libraries.userLibraryID);
+			translation.setTranslator('14763d24-8ba0-45df-8f52-b8d1108e7ac9'); // Zotero RDF
+			await translation.translate();
+			
+			// Export top-most collection
+			translation = new Zotero.Translate.Export();
+			translation.setLocation(Zotero.File.pathToFile(collectionExportFile));
+			translation.setCollection(c1);
+			translation.setTranslator('14763d24-8ba0-45df-8f52-b8d1108e7ac9'); // Zotero RDF
+			await translation.translate();
+			
+			async function check(file, mode) {
+				var collectionNames = [c1.name, c2.name, c3.name, c4.name, c5.name];
+				
+				var translation = new Zotero.Translate.Import();
+				translation.setLocation(Zotero.File.pathToFile(file));
+				var translators = await translation.getTranslators();
+				translation.setTranslator(translators[0]);
+				var importCollection = await createDataObject('collection');
+				await translation.translate({
+					libraryID: Zotero.Libraries.userLibraryID,
+					collections: [importCollection.id]
+				});
+				
+				// When exporting a library, the top-most collection should be included. When
+				// exporting a collection, the selected collection isn't included, so remove it.
+				if (mode == 'collection') {
+					collectionNames.shift();
+				}
+				
+				var collections = importCollection.getChildCollections();
+				assert.lengthOf(collections, 1, mode);
+				assert.equal(collections[0].name, collectionNames.shift(), mode)
+				
+				var name;
+				while (name = collectionNames.shift()) {
+					collections = collections[0].getChildCollections();
+					assert.lengthOf(collections, 1, mode);
+					assert.equal(collections[0].name, name, mode)
+				}
+			}
+			
+			await check(libraryExportFile, 'library');
+			await check(collectionExportFile, 'collection');
+		});
 	});