When merging items, add master item to all collections

Also changes Zotero.Item.prototype.clone() to take an `options` object for its
second parameter instead of a boolean `skipTags`. The object includes
`skipTags` as well as a new `includeCollections` property to add the new item
to the same collections.

chrome/content/zotero/duplicatesMerge.js

@@ -141,7 +141,7 @@ var Zotero_Duplicates_Pane = new function () {
 		}
 		
 		_masterItem = item;
-		itembox.item = item.clone();
+		itembox.item = item.clone(null, { includeCollections: true });
 	}
 	
 	

chrome/content/zotero/xpcom/collectionTreeView.js

@@ -1853,7 +1853,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r
 		}
 		
 		// Create new clone item in target library
-		var newItem = item.clone(targetLibraryID, false, !options.tags);
+		var newItem = item.clone(targetLibraryID, { skipTags: !options.tags });
 		
 		// Set Rights field for My Publications
 		if (options.license) {
@@ -1880,7 +1880,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r
 			var noteIDs = item.getNotes();
 			var notes = Zotero.Items.get(noteIDs);
 			for each(var note in notes) {
-				let newNote = note.clone(targetLibraryID);
+				let newNote = note.clone(targetLibraryID, { skipTags: !options.tags });
 				newNote.parentID = newItemID;
 				yield newNote.save({
 					skipSelect: true

chrome/content/zotero/xpcom/data/item.js

@@ -3802,15 +3802,16 @@ Zotero.Item.prototype.multiDiff = function (otherItems, ignoreFields) {
 
 
 /**
- * Returns an unsaved copy of the item without an itemID or key
+ * Returns an unsaved copy of the item without itemID and key
  *
  * This is used to duplicate items and copy them between libraries.
  *
  * @param {Number} [libraryID] - libraryID of the new item, or the same as original if omitted
- * @param {Boolean} [skipTags=false] - Skip tags
+ * @param {Boolean} [options.skipTags=false] - Skip tags
+ * @param {Boolean} [options.includeCollections=false] - Add new item to all collections
  * @return {Promise<Zotero.Item>}
  */
-Zotero.Item.prototype.clone = function (libraryID, skipTags) {
+Zotero.Item.prototype.clone = function (libraryID, options = {}) {
 	Zotero.debug('Cloning item ' + this.id);
 	
 	if (libraryID !== undefined && libraryID !== null && typeof libraryID !== 'number') {
@@ -3857,10 +3858,17 @@ Zotero.Item.prototype.clone = function (libraryID, skipTags) {
 		}
 	}
 	
-	if (!skipTags) {
+	if (!options.skipTags) {
 		newItem.setTags(this.getTags());
 	}
 	
+	if (options.includeCollections) {
+		if (!sameLibrary) {
+			throw new Error("Can't include collections when cloning to different library");
+		}
+		newItem.setCollections(this.getCollections());
+	}
+	
 	if (sameLibrary) {
 		// DEBUG: this will add reverse-only relateds too
 		newItem.setRelations(this.getRelations());

chrome/content/zotero/xpcom/data/items.js

@@ -789,11 +789,7 @@ Zotero.Items = function() {
 				// old item, which will be put in the trash
 				
 				// Add collections to master
-				var collectionIDs = otherItem.getCollections();
+				otherItem.getCollections().forEach(id => item.addToCollection(id));
-				for each(var collectionID in collectionIDs) {
-					let collection = yield Zotero.Collections.getAsync(collectionID);
-					yield collection.addItem(item.id);
-				}
 				
 				// Add tags to master
 				var tags = otherItem.getTags();

chrome/content/zotero/zoteroPane.js

@@ -1719,12 +1719,12 @@ var ZoteroPane = new function()
 		var newItem;
 		
 		yield Zotero.DB.executeTransaction(function* () {
-			newItem = item.clone(null, !Zotero.Prefs.get('groups.copyTags'));
+			newItem = item.clone();
-			yield newItem.save();
+			// If in a collection, add new item to it
-			
 			if (self.collectionsView.selectedTreeRow.isCollection() && newItem.isTopLevelItem()) {
-				yield self.collectionsView.selectedTreeRow.ref.addItem(newItem.id);
+				newItem.setCollections([self.collectionsView.selectedTreeRow.ref.id]);
 			}
+			yield newItem.save();
 		});
 		
 		yield self.selectItem(newItem.id);

test/tests/duplicatesTest.js

@@ -58,5 +58,39 @@ describe("Duplicate Items", function () {
 			assert.property(rels, pred);
 			assert.equal(rels[pred], uri2);
 		});
+		
+		it("should combine collections from all items", function* () {
+			var collection1 = yield createDataObject('collection');
+			var collection2 = yield createDataObject('collection');
+			
+			var item1 = yield createDataObject('item', { setTitle: true, collections: [collection1.id] });
+			var item2 = item1.clone();
+			item2.setCollections([collection2.id]);
+			yield item2.saveTx();
+			
+			var userLibraryID = Zotero.Libraries.userLibraryID;
+			
+			var selected = yield cv.selectByID('D' + userLibraryID);
+			assert.ok(selected);
+			yield waitForItemsLoad(win);
+			
+			// Select the first item, which should select both
+			var iv = zp.itemsView;
+			var row = iv.getRowIndexByID(item1.id);
+			clickOnItemsRow(iv, row);
+			
+			// Click merge button
+			var button = win.document.getElementById('zotero-duplicates-merge-button');
+			button.click();
+			
+			yield waitForNotifierEvent('refresh', 'trash');
+			
+			// Items should be gone
+			assert.isFalse(iv.getRowIndexByID(item1.id));
+			assert.isFalse(iv.getRowIndexByID(item2.id));
+			assert.isTrue(item2.deleted);
+			assert.isTrue(collection1.hasItem(item1.id));
+			assert.isTrue(collection2.hasItem(item1.id));
+		});
 	});
 });