From 84730e610c6b4caf4f954054b40dc3d96fb04b93 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Wed, 25 Nov 2020 03:18:07 -0500
Subject: [PATCH] Don't delete valid item/CSL type from Type in Extra on sync

After a local item change, Zotero uploads the JSON and then applies the
saved JSON returned from the API to the local object using `fromJSON()`,
the same as it would apply any other remote change.

`fromJSON()` is meant to migrate Extra lines into real types and fields
after future item type/field changes. It calls
`Z.Utilities.Internal.extractExtraFields()`, which looks for valid item
type or CSL type values in Type lines in Extra, handles the rest of
parsing accordingly, and passes back the parsed item type. `fromJSON()`
wasn't handling `itemType` in the response object, so the item type
didn't get applied and the Type line was stripped. This fixes that.

Since valid type values are now parsed, if you have a Journal Article
item with a Pages field and enter "Type: song" into Extra and sync, the
item will be converted to Audio Recording and `Pages: 123` will be
placed in Extra.

https://forums.zotero.org/discussion/comment/369221/#Comment_369221
---
 chrome/content/zotero/xpcom/data/item.js | 19 ++++++++++++-------
 test/tests/itemTest.js                   | 13 +++++++++++++
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 7101ccf33e..0217e2a1c5 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -4239,13 +4239,18 @@ Zotero.Item.prototype.fromJSON = function (json, options = {}) {
 	
 	var isValidForType = {};
 	var setFields = new Set();
-	var { fields: extraFields, creators: extraCreators, extra } = Zotero.Utilities.Internal.extractExtraFields(
-		json.extra || '',
-		this,
-		Object.keys(json)
-			// TEMP until we move creator lines to real creators
-			.concat('creators')
-	);
+	var { itemType, fields: extraFields, creators: extraCreators, extra } =
+		Zotero.Utilities.Internal.extractExtraFields(
+			json.extra || '',
+			this,
+			Object.keys(json)
+				// TEMP until we move creator lines to real creators
+				.concat('creators')
+		);
+	if (json.itemType != itemType) {
+		itemTypeID = Zotero.ItemTypes.getID(itemType);
+		this.setType(itemTypeID);
+	}
 	var invalidFieldLogLines = new Map();
 	
 	// Transfer valid fields from Extra to regular fields
diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js
index 8ad0402e84..650da004c7 100644
--- a/test/tests/itemTest.js
+++ b/test/tests/itemTest.js
@@ -1883,6 +1883,19 @@ describe("Zotero.Item", function () {
 			assert.equal(item.getField('extra'), `doi: ${doi2}`);
 		});*/
 		
+		it("should use valid CSL type from Extra", function () {
+			var json = {
+				itemType: "journalArticle",
+				pages: "123",
+				extra: "Type: song"
+			};
+			var item = new Zotero.Item;
+			item.fromJSON(json);
+			assert.equal(item.itemTypeID, Zotero.ItemTypes.getID('audioRecording'));
+			// A field valid for the old item type should be moved to Extra
+			assert.equal(item.getField('extra'), 'Pages: 123');
+		});
+		
 		it("should ignore creator field in Extra", async function () {
 			var json = {
 				itemType: "journalArticle",