Fix firstCreator for unsaved items

Necessary when editing embedded citations that don't exist in library

chrome/content/zotero/xpcom/data/item.js

@@ -205,7 +205,7 @@ Zotero.Item.prototype._setParentKey = function() {
 //
 //////////////////////////////////////////////////////////////////////////////
 /*
- * Retrieves (and loads from DB, if necessary) an itemData field value
+ * Retrieves an itemData field value
  *
  * @param {String|Integer} field fieldID or fieldName
  * @param {Boolean} [unformatted] Skip any special processing of DB value
@@ -222,19 +222,11 @@ Zotero.Item.prototype.getField = function(field, unformatted, includeBaseMapped)
 	
 	this._requireData('primaryData');
 	
-	// TODO: Fix logic and add sortCreator
+	// TODO: Add sortCreator
 	if (field === 'firstCreator' && !this._id) {
 		// Hack to get a firstCreator for an unsaved item
-		var creatorsData = this.getCreators(true);
+		let creatorsData = this.getCreators(true);
-		if (creators.length === 0) {
+		return Zotero.Items.getFirstCreatorFromData(this.itemTypeID, creatorsData);
-			return "";
-		} else if (creators.length === 1) {
-			return creatorsData[0].lastName;
-		} else if (creators.length === 2) {
-			return creatorsData[0].lastName + " " + Zotero.getString('general.and') + " " + creatorsData[1].lastName;
-		} else if (creators.length > 3) {
-			return creatorsData[0].lastName + " " + Zotero.getString('general.etAl');
-		}
 	} else if (field === 'id' || this.ObjectsClass.isPrimaryField(field)) {
 		var privField = '_' + field;
 		//Zotero.debug('Returning ' + (this[privField] ? this[privField] : '') + ' (typeof ' + typeof this[privField] + ')');

chrome/content/zotero/xpcom/data/items.js

@@ -993,8 +993,12 @@ Zotero.Items = function() {
 	});
 	
 	
+	
 	/**
-	 * Given API JSON for an item, return the best first creator, regardless of creator order
+	 * Given API JSON for an item, return the best single first creator, regardless of creator order
+	 *
+	 * Note that this is just a single creator, not the firstCreator field return from the
+	 * Zotero.Item::firstCreator property or this.getFirstCreatorFromData()
 	 *
 	 * @return {Object|false} - Creator in API JSON format, or false
 	 */
@@ -1017,6 +1021,48 @@ Zotero.Items = function() {
 	};
 	
 	
+	/**
+	 * Return a firstCreator string from internal creators data (from Zotero.Item::getCreators()).
+	 *
+	 * Used in Zotero.Item::getField() for unsaved items
+	 *
+	 * @param {Integer} itemTypeID
+	 * @param {Object} creatorData
+	 * @return {String}
+	 */
+	this.getFirstCreatorFromData = function (itemTypeID, creatorsData) {
+		if (creatorsData.length === 0) {
+			return "";
+		}
+		
+		var validCreatorTypes = [
+			Zotero.CreatorTypes.getPrimaryIDForType(itemTypeID),
+			Zotero.CreatorTypes.getID('editor'),
+			Zotero.CreatorTypes.getID('contributor')
+		];
+	
+		for (let creatorTypeID of validCreatorTypes) {
+			let matches = creatorsData.filter(data => data.creatorTypeID == creatorTypeID)
+			if (!matches.length) {
+				continue;
+			}
+			if (matches.length === 1) {
+				return matches[0].lastName;
+			}
+			if (matches.length === 2) {
+				let a = matches[0];
+				let b = matches[1];
+				return a.lastName + " " + Zotero.getString('general.and') + " " + b.lastName;
+			}
+			if (matches.length >= 3) {
+				return matches[0].lastName + " " + Zotero.getString('general.etAl');
+			}
+		}
+		
+		return "";
+	};
+	
+	
 	/*
 	 * Generate SQL to retrieve firstCreator field
 	 *

test/tests/itemTest.js

@@ -18,6 +18,23 @@ describe("Zotero.Item", function () {
 			item.setField('title', 'foo');
 			assert.strictEqual(item.getField('invalid'), "");
 		});
+		
+		it("should return a firstCreator for an unsaved item", function* () {
+			var item = createUnsavedDataObject('item');
+			item.setCreators([
+				{
+					firstName: "A",
+					lastName: "B",
+					creatorType: "author"
+				},
+				{
+					firstName: "C",
+					lastName: "D",
+					creatorType: "editor"
+				}
+			]);
+			assert.equal(item.getField('firstCreator'), "B");
+		});
 	});
 	
 	describe("#setField", function () {

test/tests/itemsTest.js

@@ -158,6 +158,167 @@ describe("Zotero.Items", function () {
 		})
 	})
 	
+	describe("#getFirstCreatorFromData()", function () {
+		it("should handle single eligible creator", function* () {
+			for (let creatorType of ['author', 'editor', 'contributor']) {
+				assert.equal(
+					Zotero.Items.getFirstCreatorFromData(
+						Zotero.ItemTypes.getID('book'),
+						[
+							{
+								fieldMode: 0,
+								firstName: 'A',
+								lastName: 'B',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							}
+						]
+					),
+					'B',
+					creatorType
+				);
+			}
+		});
+		
+		it("should ignore single ineligible creator", function* () {
+			assert.strictEqual(
+				Zotero.Items.getFirstCreatorFromData(
+					Zotero.ItemTypes.getID('book'),
+					[
+						{
+							fieldMode: 0,
+							firstName: 'A',
+							lastName: 'B',
+							creatorTypeID: Zotero.CreatorTypes.getID('translator')
+						}
+					]
+				),
+				''
+			);
+		});
+		
+		it("should handle single eligible creator after ineligible creator", function* () {
+			for (let creatorType of ['author', 'editor', 'contributor']) {
+				assert.equal(
+					Zotero.Items.getFirstCreatorFromData(
+						Zotero.ItemTypes.getID('book'),
+						[
+							{
+								fieldMode: 0,
+								firstName: 'A',
+								lastName: 'B',
+								creatorTypeID: Zotero.CreatorTypes.getID('translator')
+							},
+							{
+								fieldMode: 0,
+								firstName: 'C',
+								lastName: 'D',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							}
+						]
+					),
+					'D',
+					creatorType
+				);
+			}
+		});
+		
+		it("should handle two eligible creators", function* () {
+			for (let creatorType of ['author', 'editor', 'contributor']) {
+				assert.equal(
+					Zotero.Items.getFirstCreatorFromData(
+						Zotero.ItemTypes.getID('book'),
+						[
+							{
+								fieldMode: 0,
+								firstName: 'A',
+								lastName: 'B',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							},
+							{
+								fieldMode: 0,
+								firstName: 'C',
+								lastName: 'D',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							}
+						]
+					),
+					'B ' + Zotero.getString('general.and') + ' D',
+					creatorType
+				);
+			}
+		});
+		
+		it("should handle three eligible creators", function* () {
+			for (let creatorType of ['author', 'editor', 'contributor']) {
+				assert.equal(
+					Zotero.Items.getFirstCreatorFromData(
+						Zotero.ItemTypes.getID('book'),
+						[
+							{
+								fieldMode: 0,
+								firstName: 'A',
+								lastName: 'B',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							},
+							{
+								fieldMode: 0,
+								firstName: 'C',
+								lastName: 'D',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							},
+							{
+								fieldMode: 0,
+								firstName: 'E',
+								lastName: 'F',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							}
+						]
+					),
+					'B ' + Zotero.getString('general.etAl'),
+					creatorType
+				);
+			}
+		});
+		
+		it("should handle two eligible creators with intervening creators", function* () {
+			for (let creatorType of ['author', 'editor', 'contributor']) {
+				assert.equal(
+					Zotero.Items.getFirstCreatorFromData(
+						Zotero.ItemTypes.getID('book'),
+						[
+							{
+								fieldMode: 0,
+								firstName: 'A',
+								lastName: 'B',
+								creatorTypeID: Zotero.CreatorTypes.getID('translator')
+							},
+							{
+								fieldMode: 0,
+								firstName: 'C',
+								lastName: 'D',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							},
+							{
+								fieldMode: 0,
+								firstName: 'E',
+								lastName: 'F',
+								creatorTypeID: Zotero.CreatorTypes.getID('translator')
+							},
+							{
+								fieldMode: 0,
+								firstName: 'G',
+								lastName: 'H',
+								creatorTypeID: Zotero.CreatorTypes.getID(creatorType)
+							}
+						]
+					),
+					'D ' + Zotero.getString('general.and') + ' H',
+					creatorType
+				);
+			}
+		});
+	});
+	
 	describe("#getAsync()", function() {
 		it("should return Zotero.Item for item ID", function* () {
 			let item = new Zotero.Item('journalArticle');