From ec0b993a17b73aa81ca8a76e6a7c27c375ea288e Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 3 May 2019 01:15:27 -0400
Subject: [PATCH] Fix erroneous sync conflict if creators are changed in the
 same way

---
 chrome/content/zotero/xpcom/sync/syncLocal.js |  17 ++-
 test/tests/syncLocalTest.js                   | 107 +++++++++++++++++-
 2 files changed, 115 insertions(+), 9 deletions(-)

diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js
index fab1727777..947b5d6de0 100644
--- a/chrome/content/zotero/xpcom/sync/syncLocal.js
+++ b/chrome/content/zotero/xpcom/sync/syncLocal.js
@@ -1493,12 +1493,6 @@ Zotero.Sync.Data.Local = {
 						}
 						break;
 					
-					case 'creators':
-						if (!Zotero.Creators.equals(c1.value, c2.value)) {
-							continue;
-						}
-						break;
-					
 					case 'tags':
 						if (!Zotero.Tags.equals(c1.value, c2.value)) {
 							// If just a type difference, treat as modify with type 0 if
@@ -1535,6 +1529,17 @@ Zotero.Sync.Data.Local = {
 				
 				// Changes are equal or in conflict
 				
+				// Creators changed the same way on both sides
+				if (c1.field == 'creators' && c1.op == 'modify' && c2.op == 'modify') {
+					let creators1 = c1.value;
+					let creators2 = c2.value;
+					if (creators1.length == creators2.length
+							&& creators1.every((c, index) => Zotero.Creators.equals(c, creators2[index]))) {
+						changeset2.splice(j--, 1);
+						continue;
+					}
+				}
+				
 				// Removed on both sides
 				if (c1.op == 'delete' && c2.op == 'delete') {
 					changeset2.splice(j--, 1);
diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js
index 030ee9d90f..f9ef47ab02 100644
--- a/test/tests/syncLocalTest.js
+++ b/test/tests/syncLocalTest.js
@@ -969,6 +969,13 @@ describe("Zotero.Sync.Data.Local", function() {
 					version: 1234,
 					itemType: "book",
 					title: "Title 1",
+					creators: [
+						{
+							firstName: "First1",
+							lastName: "Last1",
+							creatorType: "author"
+						}
+					],
 					url: "http://zotero.org/",
 					publicationTitle: "Publisher", // Remove locally
 					extra: "Extra", // Removed on both
@@ -1003,6 +1010,19 @@ describe("Zotero.Sync.Data.Local", function() {
 					version: 1234,
 					itemType: "book",
 					title: "Title 2", // Changed locally
+					creators: [
+						{
+							firstName: "First1",
+							lastName: "Last1",
+							creatorType: "author"
+						},
+						// Same new creator on local and remote
+						{
+							firstName: "First2",
+							lastName: "Last2",
+							creatorType: "editor"
+						}
+					],
 					url: "https://www.zotero.org/", // Same change on local and remote
 					place: "Place", // Added locally
 					dateModified: "2015-05-14 14:12:34", // Changed locally and remotely, but ignored
@@ -1037,6 +1057,19 @@ describe("Zotero.Sync.Data.Local", function() {
 					version: 1235,
 					itemType: "book",
 					title: "Title 1",
+					creators: [
+						{
+							firstName: "First1",
+							lastName: "Last1",
+							creatorType: "author"
+						},
+						// Same new creator on local and remote
+						{
+							firstName: "First2",
+							lastName: "Last2",
+							creatorType: "editor"
+						}
+					],
 					url: "https://www.zotero.org/",
 					publicationTitle: "Publisher",
 					date: "2015-05-15", // Added remotely
@@ -1273,8 +1306,6 @@ describe("Zotero.Sync.Data.Local", function() {
 				var result = Zotero.Sync.Data.Local._reconcileChanges(
 					'item', cacheJSON, json1, json2, ignoreFields
 				);
-				Zotero.debug('=-=-=-=');
-				Zotero.debug(result);
 				assert.lengthOf(result.changes, 0);
 				assert.sameDeepMembers(
 					result.conflicts,
@@ -1293,7 +1324,77 @@ describe("Zotero.Sync.Data.Local", function() {
 						]
 					]
 				);
-			})
+			});
+			
+			it("should return conflict when creator changes can't be automatically resolved", function () {
+				var cacheJSON = {
+					key: "AAAAAAAA",
+					version: 1234,
+					title: "Title",
+					creators: [
+						{
+							firstName: "First1",
+							lastName: "Last1",
+							creatorType: "author"
+						}
+					],
+					dateModified: "2015-05-14 12:34:56"
+				};
+				var json1 = {
+					key: "AAAAAAAA",
+					version: 1234,
+					title: "Title",
+					creators: [
+						{
+							firstName: "First2",
+							lastName: "Last2",
+							creatorType: "author"
+						}
+					],
+					dateModified: "2015-05-14 14:12:34"
+				};
+				var json2 = {
+					key: "AAAAAAAA",
+					version: 1235,
+					title: "Title",
+					creators: [
+						{
+							firstName: "First3",
+							lastName: "Last3",
+							creatorType: "author"
+						}
+					],
+					dateModified: "2015-05-14 13:45:12"
+				};
+				var ignoreFields = ['dateAdded', 'dateModified'];
+				var result = Zotero.Sync.Data.Local._reconcileChanges(
+					'item', cacheJSON, json1, json2, ignoreFields
+				);
+				assert.lengthOf(result.changes, 0);
+				assert.lengthOf(result.conflicts, 1);
+				assert.propertyVal(result.conflicts[0][0], 'field', 'creators');
+				assert.propertyVal(result.conflicts[0][0], 'op', 'modify');
+				assert.lengthOf(result.conflicts[0][0].value, 1);
+				assert.include(
+					result.conflicts[0][0].value[0],
+					{
+						firstName: 'First2',
+						lastName: 'Last2',
+						creatorType: 'author'
+					}
+				);
+				assert.propertyVal(result.conflicts[0][1], 'field', 'creators');
+				assert.propertyVal(result.conflicts[0][1], 'op', 'modify');
+				assert.lengthOf(result.conflicts[0][1].value, 1);
+				assert.include(
+					result.conflicts[0][1].value[0],
+					{
+						firstName: 'First3',
+						lastName: 'Last3',
+						creatorType: 'author'
+					}
+				);
+			});
 			
 			it("should automatically merge array/object members and generate conflicts for field changes in absence of cached version", function () {
 				var json1 = {