Fix duplicate merging

Fixes #919

chrome/content/zotero/duplicatesMerge.js

@@ -23,10 +23,13 @@
     ***** END LICENSE BLOCK *****
 */
 
+"use strict";
+
 var Zotero_Duplicates_Pane = new function () {
-	_items = [];
-	_otherItems = [];
-	_ignoreFields = ['dateAdded', 'dateModified', 'accessDate'];
+	var _masterItem;
+	var _items = [];
+	var _otherItems = [];
+	var _ignoreFields = ['dateAdded', 'dateModified', 'accessDate'];
 	
 	this.setItems = function (items, displayNumItemsOnTypeError) {
 		var itemTypeID, oldestItem, otherItems = [];
@@ -77,15 +80,13 @@ var Zotero_Duplicates_Pane = new function () {
 		// Update the UI
 		//
 		
-		var diff = oldestItem.multiDiff(otherItems, _ignoreFields);
-		
 		var button = document.getElementById('zotero-duplicates-merge-button');
 		var versionSelect = document.getElementById('zotero-duplicates-merge-version-select');
 		var itembox = document.getElementById('zotero-duplicates-merge-item-box');
 		var fieldSelect = document.getElementById('zotero-duplicates-merge-field-select');
 		
-		versionSelect.hidden = !diff;
-		if (diff) {
+		var alternatives = oldestItem.multiDiff(otherItems, _ignoreFields);
+		if (alternatives) {
 			// Populate menulist with Date Added values from all items
 			var dateList = document.getElementById('zotero-duplicates-merge-original-date');
 			
@@ -111,8 +112,8 @@ var Zotero_Duplicates_Pane = new function () {
 		}
 		
 		button.label = Zotero.getString('pane.item.duplicates.mergeItems', (otherItems.length + 1));
-		itembox.hiddenFields = diff ? [] : ['dateAdded', 'dateModified'];
-		fieldSelect.hidden = !diff;
+		versionSelect.hidden = fieldSelect.hidden = !alternatives;
+		itembox.hiddenFields = alternatives ? [] : ['dateAdded', 'dateModified'];
 		
 		this.setMaster(0);
 		
@@ -130,25 +131,25 @@ var Zotero_Duplicates_Pane = new function () {
 		// Add master item's values to the beginning of each set of
 		// alternative values so that they're still available if the item box
 		// modifies the item
-		Zotero.spawn(function* () {
-			var diff = item.multiDiff(_otherItems, _ignoreFields);
-			if (diff) {
-				let itemValues = item.toJSON();
-				for (let i in diff) {
-					diff[i].unshift(itemValues[i] !== undefined ? itemValues[i] : '');
-				}
-				itembox.fieldAlternatives = diff;
+		var alternatives = item.multiDiff(_otherItems, _ignoreFields);
+		if (alternatives) {
+			let itemValues = item.toJSON();
+			for (let i in alternatives) {
+				alternatives[i].unshift(itemValues[i] !== undefined ? itemValues[i] : '');
 			}
-			
-			var newItem = yield item.copy();
-			itembox.item = newItem;
-		});
+			itembox.fieldAlternatives = alternatives;
+		}
+		
+		_masterItem = item;
+		itembox.item = item.clone();
 	}
 	
 	
-	this.merge = function () {
+	this.merge = Zotero.Promise.coroutine(function* () {
 		var itembox = document.getElementById('zotero-duplicates-merge-item-box');
 		Zotero.CollectionTreeCache.clear();
-		Zotero.Items.merge(itembox.item, _otherItems);
-	}
+		// Update master item with any field alternatives from the item box
+		_masterItem.fromJSON(itembox.item.toJSON());
+		Zotero.Items.merge(_masterItem, _otherItems);
+	});
 }

chrome/content/zotero/xpcom/libraryTreeView.js

@@ -79,7 +79,7 @@ Zotero.LibraryTreeView.prototype = {
 	 * Return the index of the row with a given ID (e.g., "C123" for collection 123)
 	 *
 	 * @param {String} - Row id
-	 * @return {Integer}
+	 * @return {Integer|false}
 	 */
 	getRowIndexByID: function (id) {
 		var type = "";
@@ -87,7 +87,7 @@ Zotero.LibraryTreeView.prototype = {
 			var type = id[0];
 			id = ('' + id).substr(1);
 		}
-		return this._rowMap[type + id];
+		return this._rowMap[type + id] !== undefined ? this._rowMap[type + id] : false;
 	},
 	
 	

test/tests/duplicatesTest.js

@@ -0,0 +1,78 @@
+"use strict";
+
+describe("Duplicate Items", function () {
+	var win, zp, cv;
+	
+	beforeEach(function* () {
+		Zotero.Prefs.clear('duplicateLibraries');
+		win = yield loadZoteroPane();
+		zp = win.ZoteroPane;
+		cv = zp.collectionsView;
+		
+		return selectLibrary(win);
+	})
+	after(function () {
+		if (win) {
+			win.close();
+		}
+	});
+	
+	describe("Merging", function () {
+		it("should merge two items in duplicates view", function* () {
+			var item1 = yield createDataObject('item', { setTitle: true });
+			var item2 = item1.clone();
+			yield item2.saveTx();
+			var uri2 = Zotero.URI.getItemURI(item2);
+			
+			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 index = iv.getRowIndexByID(item1.id);
+			assert.isNumber(index);
+			
+			var x = {};
+			var y = {};
+			var width = {};
+			var height = {};
+			iv._treebox.getCoordsForCellItem(
+				index,
+				iv._treebox.columns.getNamedColumn('zotero-items-column-title'),
+				'text',
+				x, y, width, height
+			);
+			
+			// Select row to trigger multi-select
+			var tree = iv._treebox.treeBody;
+			var rect = tree.getBoundingClientRect();
+			var x = rect.left + x.value;
+			var y = rect.top + y.value;
+			tree.dispatchEvent(new MouseEvent("mousedown", {
+				clientX: x,
+				clientY: y,
+				detail: 1
+			}));
+			
+			assert.equal(iv.selection.count, 2);
+			
+			// 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);
+			var rels = item1.getRelations();
+			var pred = Zotero.Relations.replacedItemPredicate;
+			assert.property(rels, pred);
+			assert.equal(rels[pred], uri2);
+		});
+	});
+});