From bb665a56b67d237a7c9a21344eae8fb33d0d910c Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 15 Feb 2017 23:13:35 -0500 Subject: [PATCH] Fix firstCreator for unsaved items Necessary when editing embedded citations that don't exist in library --- chrome/content/zotero/xpcom/data/item.js | 16 +-- chrome/content/zotero/xpcom/data/items.js | 48 ++++++- test/tests/itemTest.js | 17 +++ test/tests/itemsTest.js | 161 ++++++++++++++++++++++ 4 files changed, 229 insertions(+), 13 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 2543fc3e7..bc56d5b44 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/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); - if (creators.length === 0) { - 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'); - } + let creatorsData = this.getCreators(true); + return Zotero.Items.getFirstCreatorFromData(this.itemTypeID, creatorsData); } else if (field === 'id' || this.ObjectsClass.isPrimaryField(field)) { var privField = '_' + field; //Zotero.debug('Returning ' + (this[privField] ? this[privField] : '') + ' (typeof ' + typeof this[privField] + ')'); diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 94b6831dd..e70f463e9 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/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 * diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index 063ae69fd..5d26e5fa3 100644 --- a/test/tests/itemTest.js +++ b/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 () { diff --git a/test/tests/itemsTest.js b/test/tests/itemsTest.js index 2c2ecb317..fee5c07c1 100644 --- a/test/tests/itemsTest.js +++ b/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');