From d44eeb752b656b0f61abf1ace4316a7f90b7dfd4 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 7 Jul 2016 05:01:36 -0400
Subject: [PATCH] Fix error on CR of child note, and show parent item title in
 merge pane

---
 chrome/content/zotero/bindings/merge.xml      |  30 +++-
 chrome/content/zotero/merge.js                |   4 +
 .../content/zotero/xpcom/sync/syncEngine.js   |   1 +
 chrome/content/zotero/xpcom/sync/syncLocal.js |   2 +
 chrome/skin/default/zotero/merge.css          |   8 +
 test/tests/syncEngineTest.js                  | 160 ++++++++++++++++++
 test/tests/syncLocalTest.js                   |  41 +++++
 7 files changed, 245 insertions(+), 1 deletion(-)

diff --git a/chrome/content/zotero/bindings/merge.xml b/chrome/content/zotero/bindings/merge.xml
index 33bbaf007..ad717d480 100644
--- a/chrome/content/zotero/bindings/merge.xml
+++ b/chrome/content/zotero/bindings/merge.xml
@@ -28,7 +28,8 @@
 
 <bindings xmlns="http://www.mozilla.org/xbl"
 		  xmlns:xbl="http://www.mozilla.org/xbl"
-		  xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+		  xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+		  xmlns:html="http://www.w3.org/1999/xhtml">
 	
 	<binding id="merge-group">
 		<resources>
@@ -44,6 +45,8 @@
 			]]>
 			</constructor>
 			
+			<field name="libraryID"/>
+			
 			<field name="_data"/>
 			<property name="data" onget="return this._data;">
 				<setter>
@@ -109,6 +112,9 @@
 					
 					this._leftpane.showButton = showButton;
 					this._rightpane.showButton = showButton;
+					this._leftpane.libraryID = this.libraryID;
+					this._rightpane.libraryID = this.libraryID;
+					this._mergepane.libraryID = this.libraryID;
 					this._leftpane.data = this._data.left;
 					this._rightpane.data = this._data.right;
 					this._mergepane.data = this._data.merge;
@@ -293,6 +299,8 @@
 				</setter>
 			</property>
 			
+			<field name="libraryID"/>
+			
 			<field name="_data"/>
 			<property name="data" onget="return this._data">
 				<setter>
@@ -338,6 +346,24 @@
 					
 					var objbox = document.createElement(elementName);
 					
+					var parentRow = this._id('parent-row');
+					if (val.parentItem) {
+						parentRow.textContent = '';
+						
+						let label = document.createElement('span');
+						label.textContent = Zotero.getString('pane.item.parentItem');
+						parentRow.appendChild(label);
+						
+						let parentItem = Zotero.Items.getByLibraryAndKey(this.libraryID, val.parentItem);
+						let text = document.createTextNode(" " + parentItem.getDisplayTitle(true));
+						parentRow.appendChild(text);
+						
+						parentRow.hidden = false;
+					}
+					else {
+						parentRow.hidden = true;
+					}
+					
 					if (this._id('object-placeholder')) {
 						var placeholder = this._id('object-placeholder');
 						placeholder.parentNode.replaceChild(objbox, placeholder);
@@ -356,6 +382,7 @@
 					
 					// Create item from JSON for metadata box
 					var item = new Zotero.Item(val.itemType);
+					item.libraryID = this.libraryID;
 					item.fromJSON(val);
 					objbox.item = item;
 				]]>
@@ -385,6 +412,7 @@
 			<xul:vbox flex="1">
 				<xul:groupbox anonid="merge-pane" flex="1">
 					<xul:caption anonid="caption"/>
+					<html:div anonid="parent-row" hidden="true"/>
 					<xul:box anonid="object-placeholder"/>
 					<xul:hbox anonid="delete-box" hidden="true" flex="1">
 						<xul:label value="&zotero.merge.deleted;"/>
diff --git a/chrome/content/zotero/merge.js b/chrome/content/zotero/merge.js
index a8d4a3caf..983592e9d 100644
--- a/chrome/content/zotero/merge.js
+++ b/chrome/content/zotero/merge.js
@@ -200,6 +200,10 @@ var Zotero_Merge_Window = new function () {
 		var mergeInfo = _getMergeInfo(_pos);
 		data.merge = mergeInfo.data;
 		data.selected = mergeInfo.selected;
+		if (!_conflicts[_pos].libraryID) {
+			throw new Error("libraryID not provided in conflict object");
+		}
+		_mergeGroup.libraryID = _conflicts[_pos].libraryID;
 		_mergeGroup.data = data;
 		
 		_updateResolveAllCheckbox();
diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
index 4b75f1b21..3077c8fb0 100644
--- a/chrome/content/zotero/xpcom/sync/syncEngine.js
+++ b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -643,6 +643,7 @@ Zotero.Sync.Data.Engine.prototype._downloadDeletions = Zotero.Promise.coroutine(
 			// Conflict resolution
 			else if (objectType == 'item') {
 				conflicts.push({
+					libraryID: this.libraryID,
 					left: obj.toJSON(),
 					right: {
 						deleted: true
diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js
index c05033777..52b568169 100644
--- a/chrome/content/zotero/xpcom/sync/syncLocal.js
+++ b/chrome/content/zotero/xpcom/sync/syncLocal.js
@@ -677,6 +677,7 @@ Zotero.Sync.Data.Local = {
 									Zotero.debug(jsonData);
 									Zotero.debug(result);
 									results.push({
+										libraryID,
 										key: objectKey,
 										processed: false,
 										conflict: true,
@@ -721,6 +722,7 @@ Zotero.Sync.Data.Local = {
 								switch (objectType) {
 								case 'item':
 									results.push({
+										libraryID,
 										key: objectKey,
 										processed: false,
 										conflict: true,
diff --git a/chrome/skin/default/zotero/merge.css b/chrome/skin/default/zotero/merge.css
index bd26c36e0..08c46383f 100644
--- a/chrome/skin/default/zotero/merge.css
+++ b/chrome/skin/default/zotero/merge.css
@@ -88,3 +88,11 @@ hbox:not([mergetype=note]) zoteromergepane:active[id=rightpane] groupbox caption
 zoteromergepane {
 	min-width: 28em;
 }
+
+zoteromergepane div[anonid=parent-row] {
+	margin-bottom: 5px;
+}
+
+zoteromergepane div[anonid=parent-row] span {
+	font-weight: bold;
+}
diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js
index 9fba92c5e..55c692e5a 100644
--- a/test/tests/syncEngineTest.js
+++ b/test/tests/syncEngineTest.js
@@ -2025,6 +2025,103 @@ describe("Zotero.Sync.Data.Engine", function () {
 			assert.lengthOf(keys, 0);
 		});
 		
+		it("should show conflict resolution window on note conflicts", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			({ engine, client, caller } = yield setup());
+			var type = 'item';
+			var objects = [];
+			var values = [];
+			var dateAdded = Date.now() - 86400000;
+			var responseJSON = [];
+			
+			for (let i = 0; i < 2; i++) {
+				values.push({
+					left: {},
+					right: {}
+				});
+				
+				// Create local object
+				let obj = objects[i] = new Zotero.Item('note');
+				obj.setNote(Zotero.Utilities.randomString());
+				obj.version = 10;
+				obj.dateAdded = Zotero.Date.dateToSQL(new Date(dateAdded), true);
+				// Set Date Modified values one minute apart to enforce order
+				obj.dateModified = Zotero.Date.dateToSQL(
+					new Date(dateAdded + (i * 60000)), true
+				);
+				yield obj.saveTx();
+				
+				let jsonData = obj.toJSON();
+				jsonData.key = obj.key;
+				jsonData.version = 10;
+				let json = {
+					key: obj.key,
+					version: jsonData.version,
+					data: jsonData
+				};
+				// Save original version in cache
+				yield Zotero.Sync.Data.Local.saveCacheObjects(type, libraryID, [json]);
+				
+				// Create updated JSON for download
+				values[i].right.note = jsonData.note = Zotero.Utilities.randomString();
+				values[i].right.version = json.version = jsonData.version = 15;
+				responseJSON.push(json);
+				
+				// Modify object locally
+				obj.setNote(Zotero.Utilities.randomString());
+				yield obj.saveTx({
+					skipDateModifiedUpdate: true
+				});
+				values[i].left.note = obj.getNote();
+				values[i].left.version = obj.getField('version');
+			}
+			
+			setResponse({
+				method: "GET",
+				url: `users/1/items?format=json&itemKey=${objects.map(o => o.key).join('%2C')}`
+					+ `&includeTrashed=1`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 15
+				},
+				json: responseJSON
+			});
+			
+			waitForWindow('chrome://zotero/content/merge.xul', function (dialog) {
+				var doc = dialog.document;
+				var wizard = doc.documentElement;
+				var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0];
+				
+				// 1 (remote)
+				// Remote version should be selected by default
+				assert.equal(mergeGroup.rightpane.getAttribute('selected'), 'true');
+				wizard.getButton('next').click();
+				
+				// 2 (local)
+				assert.equal(mergeGroup.rightpane.getAttribute('selected'), 'true');
+				// Select local object
+				mergeGroup.leftpane.click();
+				assert.equal(mergeGroup.leftpane.getAttribute('selected'), 'true');
+				if (Zotero.isMac) {
+					assert.isTrue(wizard.getButton('next').hidden);
+					assert.isFalse(wizard.getButton('finish').hidden);
+				}
+				else {
+					// TODO
+				}
+				wizard.getButton('finish').click();
+			});
+			yield engine._downloadObjects('item', objects.map(o => o.key));
+			
+			assert.equal(objects[0].getNote(), values[0].right.note);
+			assert.equal(objects[1].getNote(), values[1].left.note);
+			assert.equal(objects[0].getField('version'), values[0].right.version);
+			assert.equal(objects[1].getField('version'), values[1].left.version);
+			
+			var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID);
+			assert.lengthOf(keys, 0);
+		});
+		
 		it("should resolve all remaining conflicts with one side", function* () {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			({ engine, client, caller } = yield setup());
@@ -2190,6 +2287,69 @@ describe("Zotero.Sync.Data.Engine", function () {
 			assert.lengthOf(keys, 0);
 		})
 		
+		it("should handle local child note deletion, keeping deletion", function* () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			({ engine, client, caller } = yield setup());
+			var responseJSON = [];
+			
+			var parent = yield createDataObject('item');
+			
+			// Create object, generate JSON, and delete
+			var obj = new Zotero.Item('note');
+			obj.parentItemID = parent.id;
+			obj.setNote(Zotero.Utilities.randomString());
+			obj.version = 10;
+			yield obj.saveTx();
+			var jsonData = obj.toJSON();
+			var key = jsonData.key = obj.key;
+			jsonData.version = 10;
+			let json = {
+				key: obj.key,
+				version: jsonData.version,
+				data: jsonData
+			};
+			// Delete object locally
+			yield obj.eraseTx();
+			
+			json.version = jsonData.version = 15;
+			jsonData.note = Zotero.Utilities.randomString();
+			responseJSON.push(json);
+			
+			setResponse({
+				method: "GET",
+				url: `users/1/items?format=json&itemKey=${obj.key}&includeTrashed=1`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 15
+				},
+				json: responseJSON
+			});
+			
+			var windowOpened = false;
+			waitForWindow('chrome://zotero/content/merge.xul', function (dialog) {
+				windowOpened = true;
+				
+				var doc = dialog.document;
+				var wizard = doc.documentElement;
+				var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0];
+				
+				// Remote version should be selected by default
+				assert.equal(mergeGroup.rightpane.getAttribute('selected'), 'true');
+				assert.ok(mergeGroup.leftpane.pane.onclick);
+				// Select local deleted version
+				mergeGroup.leftpane.pane.click();
+				wizard.getButton('finish').click();
+			});
+			yield engine._downloadObjects('item', [obj.key]);
+			assert.isTrue(windowOpened);
+			
+			obj = Zotero.Items.getByLibraryAndKey(libraryID, key);
+			assert.isFalse(obj);
+			
+			var keys = yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID);
+			assert.lengthOf(keys, 0);
+		});
+		
 		it("should restore locally deleted item", function* () {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			({ engine, client, caller } = yield setup());
diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js
index 695dcf802..a1cd182dd 100644
--- a/test/tests/syncLocalTest.js
+++ b/test/tests/syncLocalTest.js
@@ -557,6 +557,47 @@ describe("Zotero.Sync.Data.Local", function() {
 	});
 	
 	
+	describe("#showConflictResolutionWindow()", function () {
+		it("should show title of note parent", function* () {
+			var parentItem = yield createDataObject('item', { title: "Parent" });
+			var note = new Zotero.Item('note');
+			note.parentKey = parentItem.key;
+			note.setNote("Test");
+			yield note.saveTx();
+			
+			var promise = waitForWindow('chrome://zotero/content/merge.xul', function (dialog) {
+				var doc = dialog.document;
+				var wizard = doc.documentElement;
+				var mergeGroup = wizard.getElementsByTagName('zoteromergegroup')[0];
+				
+				// Show title for middle and right panes
+				var parentText = Zotero.getString('pane.item.parentItem') + " Parent";
+				assert.equal(mergeGroup.leftpane._id('parent-row').textContent, "");
+				assert.equal(mergeGroup.rightpane._id('parent-row').textContent, parentText);
+				assert.equal(mergeGroup.mergepane._id('parent-row').textContent, parentText);
+				
+				wizard.getButton('finish').click();
+			});
+			
+			Zotero.Sync.Data.Local.showConflictResolutionWindow([
+				{
+					libraryID: note.libraryID,
+					key: note.key,
+					processed: false,
+					conflict: true,
+					left: {
+						deleted: true,
+						dateDeleted: "2016-07-07 12:34:56"
+					},
+					right: note.toJSON()
+				}
+			]);
+			
+			yield promise;
+		});
+	});
+	
+	
 	describe("#_reconcileChanges()", function () {
 		describe("items", function () {
 			it("should ignore non-conflicting local changes and return remote changes", function () {