From 2154673dd3782cb756f10f45c2abbe7b858e63f7 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 29 May 2015 05:31:54 -0400 Subject: [PATCH] Return a Zotero.Item from all Zotero.Attachments methods These previously returned an itemID, but now that new saved items can be edited without a refetch, they should just return the new item. (Possible I missed a few spots where these are called.) --- chrome/content/zotero/xpcom/attachments.js | 54 +++++++++---------- .../zotero/xpcom/collectionTreeView.js | 4 +- chrome/content/zotero/xpcom/itemTreeView.js | 4 +- chrome/content/zotero/zoteroPane.js | 5 +- test/tests/attachmentsTest.js | 20 +++---- test/tests/collectionTreeViewTest.js | 2 +- test/tests/recognizePDFTest.js | 8 +-- 7 files changed, 43 insertions(+), 54 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 2f7f62acc..f02d78f74 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -39,7 +39,7 @@ Zotero.Attachments = new function(){ * @param {Integer} [options.libraryID] * @param {Integer[]|String[]} [options.parentItemID] - Parent item to add item to * @param {Integer[]} [options.collections] - Collection keys or ids to add new item to - * @return {Promise} + * @return {Promise} */ this.importFromFile = Zotero.Promise.coroutine(function* (options) { Zotero.debug('Importing attachment from file'); @@ -59,10 +59,10 @@ Zotero.Attachments = new function(){ throw new Error("parentItemID and collections cannot both be provided"); } - var itemID, newFile, contentType; - yield Zotero.DB.executeTransaction(function* () { + var attachmentItem, itemID, newFile, contentType; + return Zotero.DB.executeTransaction(function* () { // Create a new attachment - var attachmentItem = new Zotero.Item('attachment'); + attachmentItem = new Zotero.Item('attachment'); if (parentItemID) { let {libraryID: parentLibraryID, key: parentKey} = Zotero.Items.getLibraryAndKeyFromID(parentItemID); @@ -77,8 +77,7 @@ Zotero.Attachments = new function(){ if (collections) { attachmentItem.setCollections(collections); } - itemID = yield attachmentItem.save(); - attachmentItem = yield Zotero.Items.getAsync(itemID); + yield attachmentItem.save(); // Create directory for attachment files within storage directory destDir = yield this.createDirectoryForItem(attachmentItem); @@ -97,13 +96,12 @@ Zotero.Attachments = new function(){ yield attachmentItem.save(); }.bind(this)) .then(function () { - return _postProcessFile(itemID, newFile, contentType); + return _postProcessFile(attachmentItem.id, newFile, contentType); }) .catch(function (e) { - Zotero.debug(e, 1); + Zotero.logError(e); var msg = "Failed importing file " + file.path; - Components.utils.reportError(msg); - Zotero.debug(msg, 1); + Zotero.logError(msg); // Clean up try { @@ -112,12 +110,12 @@ Zotero.Attachments = new function(){ } } catch (e) { - Zotero.debug(e, 1); + Zotero.logError(e); } - - throw e; - }.bind(this)); - return itemID; + }.bind(this)) + .then(function () { + return attachmentItem; + }); }); @@ -155,7 +153,7 @@ Zotero.Attachments = new function(){ /** * @param {Object} options - 'file', 'url', 'title', 'contentType', 'charset', 'parentItemID' - * @return {Promise} + * @return {Promise} */ this.importSnapshotFromFile = Zotero.Promise.coroutine(function* (options) { Zotero.debug('Importing snapshot from file'); @@ -171,10 +169,10 @@ Zotero.Attachments = new function(){ throw new Error("parentItemID not provided"); } - var itemID, destDir, newFile; - yield Zotero.DB.executeTransaction(function* () { + var attachmentItem, itemID, destDir, newFile; + return Zotero.DB.executeTransaction(function* () { // Create a new attachment - var attachmentItem = new Zotero.Item('attachment'); + attachmentItem = new Zotero.Item('attachment'); let {libraryID, key: parentKey} = Zotero.Items.getLibraryAndKeyFromID(parentItemID); attachmentItem.libraryID = libraryID; attachmentItem.setField('title', title); @@ -188,7 +186,6 @@ Zotero.Attachments = new function(){ // create a proper item, but at the moment this is only called by // translate.js, which sets the metadata fields itself itemID = yield attachmentItem.save(); - attachmentItem = yield Zotero.Items.getAsync(itemID) destDir = this.getStorageDirectory(attachmentItem); yield _moveOrphanedDirectory(destDir); @@ -205,7 +202,7 @@ Zotero.Attachments = new function(){ return _postProcessFile(itemID, newFile, contentType); }) .catch(function (e) { - Zotero.debug(e, 1); + Zotero.logError(e); // Clean up try { @@ -214,13 +211,12 @@ Zotero.Attachments = new function(){ } } catch (e) { - Zotero.debug(e, 1); + Zotero.logError(e, 1); } - - throw e; - }.bind(this)); - - return itemID; + }.bind(this)) + .then(function () { + return attachmentItem; + }); }); @@ -361,7 +357,6 @@ Zotero.Attachments = new function(){ destFile.append(fileName); // Refetch item to update path - attachmentItem = yield Zotero.Items.getAsync(itemID); attachmentItem.attachmentPath = Zotero.Attachments.getPath( destFile, Zotero.Attachments.LINK_MODE_IMPORTED_URL ); @@ -496,7 +491,7 @@ Zotero.Attachments = new function(){ * TODO: what if called on file:// document? * * @param {Object} options - 'document', 'parentItemID', 'collections' - * @return {Promise} + * @return {Promise} */ this.linkFromDocument = Zotero.Promise.coroutine(function* (options) { Zotero.debug('Linking attachment from document'); @@ -653,7 +648,6 @@ Zotero.Attachments = new function(){ var destFile = destDir.clone(); destFile.append(fileName); - attachmentItem = yield Zotero.Items.getAsync(itemID); attachmentItem.attachmentPath = this.getPath( destFile, Zotero.Attachments.LINK_MODE_IMPORTED_URL ); diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index a09790150..d95bd20e1 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -2008,13 +2008,13 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r } if (dropEffect == 'link') { - var itemID = yield Zotero.Attachments.linkFromFile({ + yield Zotero.Attachments.linkFromFile({ file: file, collections: [parentCollectionID] }); } else { - var itemID = yield Zotero.Attachments.importFromFile({ + yield Zotero.Attachments.importFromFile({ file: file, libraryID: targetLibraryID, collections: [parentCollectionID] diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index b40ac8874..40e7ea28a 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -3080,14 +3080,14 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or } if (dropEffect == 'link') { - var itemID = yield Zotero.Attachments.linkFromFile({ + yield Zotero.Attachments.linkFromFile({ file: file, parentItemID: parentItemID, collections: parentCollectionID ? [parentCollectionID] : undefined }); } else { - var itemID = yield Zotero.Attachments.importFromFile({ + yield Zotero.Attachments.importFromFile({ file: file, libraryID: targetLibraryID, parentItemID: parentItemID, diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 62b402872..f3a161118 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -3282,16 +3282,15 @@ var ZoteroPane = new function() while (files.hasMoreElements()){ var file = files.getNext(); file.QueryInterface(Components.interfaces.nsILocalFile); - let attachmentID; if (link) { - attachmentID = yield Zotero.Attachments.linkFromFile({ + yield Zotero.Attachments.linkFromFile({ file: file, parentItemID: parentItemID, collections: collection ? [collection] : undefined }); } else { - attachmentID = yield Zotero.Attachments.importFromFile({ + yield Zotero.Attachments.importFromFile({ file: file, libraryID: libraryID, parentItemID: parentItemID, diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js index 8904e7227..0f45e5bfa 100644 --- a/test/tests/attachmentsTest.js +++ b/test/tests/attachmentsTest.js @@ -27,16 +27,15 @@ describe("Zotero.Attachments", function() { var parentItemID = yield item.saveTx(); // Create attachment and compare content - var itemID = yield Zotero.Attachments.importFromFile({ + var item = yield Zotero.Attachments.importFromFile({ file: tmpFile, parentItemID: parentItemID }); - var item = yield Zotero.Items.getAsync(itemID); var storedFile = item.getFile(); assert.equal((yield Zotero.File.getContentsAsync(storedFile)), contents); // Clean up - yield Zotero.Items.erase(itemID); + yield Zotero.Items.erase(item.id); }); it("should create a top-level attachment from a PNG file", function* () { @@ -45,15 +44,14 @@ describe("Zotero.Attachments", function() { var contents = yield Zotero.File.getBinaryContentsAsync(file); // Create attachment and compare content - var itemID = yield Zotero.Attachments.importFromFile({ + var item = yield Zotero.Attachments.importFromFile({ file: file }); - var item = yield Zotero.Items.getAsync(itemID); var storedFile = item.getFile(); assert.equal((yield Zotero.File.getBinaryContentsAsync(storedFile)), contents); // Clean up - yield Zotero.Items.erase(itemID); + yield Zotero.Items.erase(item.id); }); it("should create a top-level attachment from a PNG file in a collection", function* () { @@ -64,16 +62,15 @@ describe("Zotero.Attachments", function() { var collection = yield createDataObject('collection'); // Create attachment and compare content - var itemID = yield Zotero.Attachments.importFromFile({ + var item = yield Zotero.Attachments.importFromFile({ file: file, collections: [collection.id] }); - var item = yield Zotero.Items.getAsync(itemID); var storedFile = item.getFile(); assert.equal((yield Zotero.File.getBinaryContentsAsync(storedFile)), contents); // Clean up - yield Zotero.Items.erase(itemID); + yield Zotero.Items.erase(item.id); }); it("should create a child attachment from a PNG file", function* () { @@ -86,16 +83,15 @@ describe("Zotero.Attachments", function() { var parentItemID = yield item.saveTx(); // Create attachment and compare content - var itemID = yield Zotero.Attachments.importFromFile({ + var item = yield Zotero.Attachments.importFromFile({ file: file, parentItemID: parentItemID }); - var item = yield Zotero.Items.getAsync(itemID); var storedFile = item.getFile(); assert.equal((yield Zotero.File.getBinaryContentsAsync(storedFile)), contents); // Clean up - yield Zotero.Items.erase(itemID); + yield Zotero.Items.erase(item.id); }); }) diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 3459a7272..25fe247a6 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -236,7 +236,7 @@ describe("Zotero.CollectionTreeView", function() { }); var file = getTestDataDirectory(); file.append('test.png'); - var attachmentID = yield Zotero.Attachments.importFromFile({ + yield Zotero.Attachments.importFromFile({ file: file, parentItemID: item.id }); diff --git a/test/tests/recognizePDFTest.js b/test/tests/recognizePDFTest.js index fecdb4b87..20a27ca16 100644 --- a/test/tests/recognizePDFTest.js +++ b/test/tests/recognizePDFTest.js @@ -24,12 +24,12 @@ describe.skip("PDF Recognition", function() { // Import the PDF var testdir = getTestDataDirectory(); testdir.append("recognizePDF_test_DOI.pdf"); - var id = yield Zotero.Attachments.importFromFile({ + var item = yield Zotero.Attachments.importFromFile({ file: testdir }); // Recognize the PDF - win.ZoteroPane.selectItem(id); + win.ZoteroPane.selectItem(item.id); win.Zotero_RecognizePDF.recognizeSelected(); return waitForItemEvent("add").then(function(ids) { @@ -46,12 +46,12 @@ describe.skip("PDF Recognition", function() { // Import the PDF var testdir = getTestDataDirectory(); testdir.append("recognizePDF_test_GS.pdf"); - var id = yield Zotero.Attachments.importFromFile({ + var item = yield Zotero.Attachments.importFromFile({ file: testdir }); // Recognize the PDF - win.ZoteroPane.selectItem(id); + win.ZoteroPane.selectItem(item.id); win.Zotero_RecognizePDF.recognizeSelected(); return waitForItemEvent("add").then(function(ids) {