diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index 3e5726602d..414d438f05 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -555,14 +555,17 @@ Zotero.DataObjects.prototype._loadPrimaryData = async function (libraryID, ids, sql += ' AND O.' + this._ZDO_id + ' IN (' + ids.join(',') + ')'; } + var columns = Object.keys(this._primaryDataSQLParts); + var idColumn = columns.indexOf(this._ZDO_id); await Zotero.DB.queryAsync( sql, params, { noCache: true, onRow: function (row) { - var id = row.getResultByName(this._ZDO_id); - var columns = Object.keys(this._primaryDataSQLParts); + var id = idColumn != -1 + ? row.getResultByIndex(idColumn) + : row.getResultByName(this._ZDO_id); var rowObj = {}; for (let i=0; i { + let itemID = row.getResultByIndex(0); + let creatorID = row.getResultByIndex(1); - item._creators = []; - item._creatorIDs = []; - item._loaded.creators = true; - item._clearChanged('creators'); - - if (!row.creatorID) { - lastItemID = row.itemID; - continue; + if (itemID != lastItemID) { + if (!this._objectCache[itemID]) { + throw new Error("Item " + itemID + " not loaded"); + } + item = this._objectCache[itemID]; + + item._creators = []; + item._creatorIDs = []; + item._loaded.creators = true; + item._clearChanged('creators'); + + if (!creatorID) { + lastItemID = itemID; + return; + } + + if (index <= maxOrderIndex) { + fixIncorrectIndexes(item, index, maxOrderIndex); + } + + index = 0; + maxOrderIndex = -1; } - if (index <= maxOrderIndex) { - fixIncorrectIndexes(item, index, maxOrderIndex); + lastItemID = itemID; + + let orderIndex = row.getResultByIndex(3); + if (orderIndex > maxOrderIndex) { + maxOrderIndex = orderIndex; } - index = 0; - maxOrderIndex = -1; + let creatorData = Zotero.Creators.get(creatorID); + creatorData.creatorTypeID = row.getResultByIndex(2); + item._creators[index] = creatorData; + item._creatorIDs[index] = creatorID; + index++; } - - lastItemID = row.itemID; - - if (row.orderIndex > maxOrderIndex) { - maxOrderIndex = row.orderIndex; - } - - let creatorData = Zotero.Creators.get(row.creatorID); - creatorData.creatorTypeID = row.creatorTypeID; - item._creators[index] = creatorData; - item._creatorIDs[index] = row.creatorID; - index++; - } + }); if (index <= maxOrderIndex) { fixIncorrectIndexes(item, index, maxOrderIndex); diff --git a/test/tests/itemFieldsTest.js b/test/tests/itemFieldsTest.js index 5ea3e09f09..c2c48ba0c1 100644 --- a/test/tests/itemFieldsTest.js +++ b/test/tests/itemFieldsTest.js @@ -64,4 +64,42 @@ describe("Zotero.ItemFields", function () { assert.equal(Zotero.ItemFields.getDirection('book', 'creator-0-lastName', 'ar'), 'rtl'); }); }); + + describe("#_getLoadInfo()", function () { + it("should resolve a base field to the item type's field", function () { + var info = Zotero.ItemFields._getLoadInfo( + Zotero.ItemTypes.getID('thesis'), Zotero.ItemFields.getID('publisher') + ); + assert.equal(info.fieldID, Zotero.ItemFields.getID('university')); + assert.isTrue(info.valid); + }); + + it("should mark a field that isn't valid for the item type", function () { + var info = Zotero.ItemFields._getLoadInfo( + Zotero.ItemTypes.getID('book'), Zotero.ItemFields.getID('websiteTitle') + ); + assert.isFalse(info.valid); + }); + + it("should return false for an unknown field", function () { + assert.isFalse(Zotero.ItemFields._getLoadInfo(Zotero.ItemTypes.getID('book'), 999999)); + }); + + // A plugin that replaces a lookup method must still be consulted + it("should use a replaced lookup method", function () { + var bookID = Zotero.ItemTypes.getID('book'); + var titleID = Zotero.ItemFields.getID('title'); + assert.isTrue(Zotero.ItemFields._getLoadInfo(bookID, titleID).valid); + + var original = Zotero.ItemFields.isValidForType; + Zotero.ItemFields.isValidForType = () => false; + try { + assert.isFalse(Zotero.ItemFields._getLoadInfo(bookID, titleID).valid); + } + finally { + Zotero.ItemFields.isValidForType = original; + } + assert.isTrue(Zotero.ItemFields._getLoadInfo(bookID, titleID).valid); + }); + }); }) diff --git a/test/tests/itemsTest.js b/test/tests/itemsTest.js index 9842f2bd60..fb442e012c 100644 --- a/test/tests/itemsTest.js +++ b/test/tests/itemsTest.js @@ -799,4 +799,75 @@ describe("Zotero.Items", function () { assert.include(ids, att.id); }); }); + + describe("#_loadItemData()", function () { + // Point a stored field at a raw value, bypassing setField(), then reload from the DB + async function loadRaw(item, field, raw) { + var fieldID = Zotero.ItemFields.getID(field); + var valueID = await Zotero.DB.valueQueryAsync("SELECT valueID FROM itemDataValues WHERE value=?", raw); + if (!valueID) { + valueID = Zotero.ID.get('itemDataValues'); + await Zotero.DB.queryAsync("INSERT INTO itemDataValues (valueID, value) VALUES (?, ?)", [valueID, raw]); + } + await Zotero.DB.queryAsync( + "REPLACE INTO itemData (itemID, fieldID, valueID) VALUES (?, ?, ?)", [item.id, fieldID, valueID] + ); + await Zotero.Items._loadDataTypeInLibrary('itemData', item.libraryID, [item.id]); + } + + it("should strip newlines from single-line fields when loading", async function () { + var item = await createDataObject('item', { title: "Title" }); + await loadRaw(item, 'title', "Line one\nLine two"); + assert.equal(item.getField('title'), "Line one Line two"); + }); + + it("should keep newlines in multiline fields when loading", async function () { + var item = await createDataObject('item'); + await loadRaw(item, 'abstractNote', "Line one\nLine two"); + assert.equal(item.getField('abstractNote'), "Line one\nLine two"); + }); + + it("should hyphenate ISBNs when loading", async function () { + var item = await createDataObject('item', { itemType: 'book' }); + await loadRaw(item, 'ISBN', "9780306406157"); + assert.equal(item.getField('ISBN'), "978-0-306-40615-7"); + }); + + it("should load a base-mapped field into the item type's field", async function () { + var item = await createDataObject('item', { itemType: 'thesis' }); + await loadRaw(item, 'university', "University of Somewhere"); + assert.equal(item.getField('university'), "University of Somewhere"); + assert.equal(item.getField('publisher', false, true), "University of Somewhere"); + }); + + it("should set absent fields to empty", async function () { + var item = await createDataObject('item', { itemType: 'book' }); + await Zotero.Items._loadDataTypeInLibrary('itemData', item.libraryID, [item.id]); + assert.strictEqual(item.getField('volume'), ""); + }); + + // Plugins wrap Zotero.Item.prototype.setField (e.g., zotero-plugin-toolkit's + // FieldHook), so loading must keep calling it for every value + it("should call setField for every loaded value", async function () { + var item = await createDataObject('item', { itemType: 'book', title: "Called" }); + var original = Zotero.Item.prototype.setField; + var calls = []; + Zotero.Item.prototype.setField = function (field, value, loadIn) { + if (this.id == item.id) { + calls.push([field, value, loadIn]); + } + return original.apply(this, arguments); + }; + try { + await Zotero.Items._loadDataTypeInLibrary('itemData', item.libraryID, [item.id]); + } + finally { + Zotero.Item.prototype.setField = original; + } + var titleID = Zotero.ItemFields.getID('title'); + assert.deepInclude(calls, [titleID, "Called", true]); + // One call per field of the item type: stored values plus absent ones set empty + assert.lengthOf(calls, Zotero.ItemFields.getItemTypeFields(item.itemTypeID).length); + }); + }); });