From 23c936f016f59aa708eeafc2b59d7d27e2bffba7 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sun, 15 Mar 2020 13:51:04 -0400
Subject: [PATCH] Fix creator migration from Extra

---
 chrome/content/zotero/xpcom/data/item.js      |  2 +-
 .../zotero/xpcom/utilities_internal.js        |  2 +-
 test/tests/schemaTest.js                      | 37 +++++++++++++++++++
 test/tests/utilities_internalTest.js          |  6 +--
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 7c74b8289f..e7b6084a43 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -4631,7 +4631,7 @@ Zotero.Item.prototype.migrateExtraFields = function () {
 		this.setField(field, value);
 	}
 	if (creators.length) {
-		this.setCreators([...item.getCreators(), ...creators]);
+		this.setCreators([...this.getCreators(), ...creators]);
 	}
 	this.setField('extra', extra);
 	if (!this.hasChanged()) {
diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js
index 1f49ee7a12..0a2bd62daa 100644
--- a/chrome/content/zotero/xpcom/utilities_internal.js
+++ b/chrome/content/zotero/xpcom/utilities_internal.js
@@ -1057,7 +1057,7 @@ Zotero.Utilities.Internal = {
 					creatorType: possibleCreatorType
 				};
 				if (value.includes('||')) {
-					let [first, last] = value.split(/\s*\|\|\s*/);
+					let [last, first] = value.split(/\s*\|\|\s*/);
 					c.firstName = first;
 					c.lastName = last;
 				}
diff --git a/test/tests/schemaTest.js b/test/tests/schemaTest.js
index a58847483d..c96109aa3f 100644
--- a/test/tests/schemaTest.js
+++ b/test/tests/schemaTest.js
@@ -74,6 +74,43 @@ describe("Zotero.Schema", function() {
 				assert.equal(item.getField('extra'), 'number-of-pages: 11\nThis is another line.');
 				assert.isFalse(item.synced);
 			});
+			
+			it("should migrate creator", async function () {
+				var item = await createDataObject('item', { itemType: 'book' });
+				item.setCreators([
+					{
+						firstName: 'Abc',
+						lastName: 'Def',
+						creatorType: 'author',
+						fieldMode: 0
+					}
+				]);
+				item.setField('extra', 'editor: Last || First\nFoo: Bar');
+				item.synced = true;
+				await item.saveTx();
+				
+				schema.version++;
+				schema.itemTypes.find(x => x.itemType == 'book').fields.splice(0, 1, { field: 'fooBar' })
+				var newLocales = {};
+				Object.keys(schema.locales).forEach((locale) => {
+					var o = schema.locales[locale];
+					o.fields.fooBar = 'Foo Bar';
+					newLocales[locale] = o;
+				});
+				await Zotero.Schema._updateGlobalSchemaForTest(schema);
+				await Zotero.Schema.migrateExtraFields();
+				
+				var creators = item.getCreators();
+				assert.lengthOf(creators, 2);
+				assert.propertyVal(creators[0], 'firstName', 'Abc');
+				assert.propertyVal(creators[0], 'lastName', 'Def');
+				assert.propertyVal(creators[0], 'creatorTypeID', Zotero.CreatorTypes.getID('author'));
+				assert.propertyVal(creators[1], 'firstName', 'First');
+				assert.propertyVal(creators[1], 'lastName', 'Last');
+				assert.propertyVal(creators[1], 'creatorTypeID', Zotero.CreatorTypes.getID('editor'));
+				assert.equal(item.getField('extra'), 'Foo: Bar');
+				assert.isFalse(item.synced);
+			});
 		});
 	});
 	
diff --git a/test/tests/utilities_internalTest.js b/test/tests/utilities_internalTest.js
index 71f2b177d0..66b947c8dd 100644
--- a/test/tests/utilities_internalTest.js
+++ b/test/tests/utilities_internalTest.js
@@ -224,7 +224,7 @@ describe("Zotero.Utilities.Internal", function () {
 		});
 		
 		it("should extract a CSL name", function () {
-			var str = 'container-author: First || Last';
+			var str = 'container-author: Last || First';
 			var { creators, extra } = Zotero.Utilities.Internal.extractExtraFields(str);
 			assert.lengthOf(creators, 1);
 			assert.propertyVal(creators[0], 'creatorType', 'bookAuthor');
@@ -235,7 +235,7 @@ describe("Zotero.Utilities.Internal", function () {
 		
 		it("should extract a CSL name that's valid for a given item type", function () {
 			var item = createUnsavedDataObject('item', { itemType: 'bookSection' });
-			var str = 'container-author: First || Last';
+			var str = 'container-author: Last || First';
 			var { creators, extra } = Zotero.Utilities.Internal.extractExtraFields(str, item);
 			assert.lengthOf(creators, 1);
 			assert.propertyVal(creators[0], 'creatorType', 'bookAuthor');
@@ -246,7 +246,7 @@ describe("Zotero.Utilities.Internal", function () {
 		
 		it("shouldn't extract a CSL name that's not valid for a given item type", function () {
 			var item = createUnsavedDataObject('item', { itemType: 'journalArticle' });
-			var str = 'container-author: First || Last';
+			var str = 'container-author: Last || First';
 			var { creators, extra } = Zotero.Utilities.Internal.extractExtraFields(str, item);
 			assert.lengthOf(creators, 0);
 			assert.strictEqual(extra, str);