From 7fc352b9b73a3e7acecb0d98424d9d9ef0112a78 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 13 Dec 2016 05:23:21 -0500
Subject: [PATCH] Fix "Add Item by Identifier" (broken by 78b1d2ee3)

And make lookup tests run when not on Travis, though we should still mock the
HTTP requests.
---
 .../zotero/xpcom/translation/translate.js     | 118 +++++++++---------
 test/tests/lookupTest.js                      |  19 ++-
 2 files changed, 74 insertions(+), 63 deletions(-)

diff --git a/chrome/content/zotero/xpcom/translation/translate.js b/chrome/content/zotero/xpcom/translation/translate.js
index 7e01065501..29b52f6eb8 100644
--- a/chrome/content/zotero/xpcom/translation/translate.js
+++ b/chrome/content/zotero/xpcom/translation/translate.js
@@ -202,6 +202,7 @@ Zotero.Translate.Sandbox = {
 			// TODO: This used to only be used for some modes. Since it's now used for everything with
 			// async saving, there's probably a bunch of code for the non-queued mode that can be removed.
 			translate.saveQueue.push(item);
+			translate._savingItems++;
 		},
 		
 		/**
@@ -1539,71 +1540,74 @@ Zotero.Translate.Base.prototype = {
 	 * Saves items to the database, taking care to defer attachmentProgress notifications
 	 * until after save
 	 */
-	_saveItems: function (items) {
+	_saveItems: Zotero.Promise.coroutine(function* (items) {
 		var itemDoneEventsDispatched = false;
 		var deferredProgress = [];
 		var attachmentsWithProgress = [];
 		
-		this._savingItems++;
-		return this._itemSaver.saveItems(
-			items.slice(),
-			function (attachment, progress, error) {
-				var attachmentIndex = this._savingAttachments.indexOf(attachment);
-				if(progress === false || progress === 100) {
-					if(attachmentIndex !== -1) {
-						this._savingAttachments.splice(attachmentIndex, 1);
+		try {
+			var newItems = yield this._itemSaver.saveItems(
+				items.slice(),
+				function (attachment, progress, error) {
+					var attachmentIndex = this._savingAttachments.indexOf(attachment);
+					if(progress === false || progress === 100) {
+						if(attachmentIndex !== -1) {
+							this._savingAttachments.splice(attachmentIndex, 1);
+						}
+					} else if(attachmentIndex === -1) {
+						this._savingAttachments.push(attachment);
 					}
-				} else if(attachmentIndex === -1) {
-					this._savingAttachments.push(attachment);
-				}
-				
-				if(itemDoneEventsDispatched) {
-					// itemDone event has already fired, so we can fire attachmentProgress
-					// notifications
-					this._runHandler("attachmentProgress", attachment, progress, error);
-					this._checkIfDone();
-				} else {
-					// Defer until after we fire the itemDone event
-					deferredProgress.push([attachment, progress, error]);
-					attachmentsWithProgress.push(attachment);
-				}
-			}.bind(this)
-		)
-		.then(function (newItems) {
-			// Remove attachments not being saved from item.attachments
-			for(var i=0; i<items.length; i++) {
-				var item = items[i];
-				for(var j=0; j<item.attachments.length; j++) {
-					if(attachmentsWithProgress.indexOf(item.attachments[j]) === -1) {
-						item.attachments.splice(j--, 1);
+					
+					if(itemDoneEventsDispatched) {
+						// itemDone event has already fired, so we can fire attachmentProgress
+						// notifications
+						this._runHandler("attachmentProgress", attachment, progress, error);
+						this._checkIfDone();
+					} else {
+						// Defer until after we fire the itemDone event
+						deferredProgress.push([attachment, progress, error]);
+						attachmentsWithProgress.push(attachment);
 					}
-				}
-			}
-			
-			// Trigger itemDone events
-			for(var i=0, nItems = items.length; i<nItems; i++) {
-				this._runHandler("itemDone", newItems[i], items[i]);
-			}
-			
-			// Specify that itemDone event was dispatched, so that we don't defer
-			// attachmentProgress notifications anymore
-			itemDoneEventsDispatched = true;
-			
-			// Run deferred attachmentProgress notifications
-			for(var i=0; i<deferredProgress.length; i++) {
-				this._runHandler("attachmentProgress", deferredProgress[i][0],
-					deferredProgress[i][1], deferredProgress[i][2]);
-			}
-			
-			this.newItems = this.newItems.concat(newItems);
-			this._savingItems--;
-			this._checkIfDone();
-		}.bind(this))
-		.catch(function (e) {
+				}.bind(this)
+			)
+		}
+		catch (e) {
+			this._savingItems -= items.length;
+			Zotero.debug("REDUCING SAVING ITEMS ERROR TO " + this._savingItems);
 			Zotero.logError(e);
 			this.complete(false, e);
-		}.bind(this));
-	},
+			return;
+		}
+		
+		// Remove attachments not being saved from item.attachments
+		for(var i=0; i<items.length; i++) {
+			var item = items[i];
+			for(var j=0; j<item.attachments.length; j++) {
+				if(attachmentsWithProgress.indexOf(item.attachments[j]) === -1) {
+					item.attachments.splice(j--, 1);
+				}
+			}
+		}
+		
+		// Trigger itemDone events
+		for(var i=0, nItems = items.length; i<nItems; i++) {
+			this._runHandler("itemDone", newItems[i], items[i]);
+		}
+		
+		// Specify that itemDone event was dispatched, so that we don't defer
+		// attachmentProgress notifications anymore
+		itemDoneEventsDispatched = true;
+		
+		// Run deferred attachmentProgress notifications
+		for(var i=0; i<deferredProgress.length; i++) {
+			this._runHandler("attachmentProgress", deferredProgress[i][0],
+				deferredProgress[i][1], deferredProgress[i][2]);
+		}
+		
+		this._savingItems -= items.length;
+		this.newItems = this.newItems.concat(newItems);
+		this._checkIfDone();
+	}),
 	
 	/**
 	 * Checks if saving done, and if so, fires done event
diff --git a/test/tests/lookupTest.js b/test/tests/lookupTest.js
index 7033049283..af1c85eec8 100644
--- a/test/tests/lookupTest.js
+++ b/test/tests/lookupTest.js
@@ -1,19 +1,26 @@
-function lookupIdentifier(win, identifier) {
+var lookupIdentifier = Zotero.Promise.coroutine(function* (win, identifier) {
 	var textbox = win.document.getElementById("zotero-lookup-textbox");
 	textbox.value = identifier;
-	win.Zotero_Lookup.accept(textbox);
-	return waitForItemEvent("add");
-}
+	var promise = waitForItemEvent("add");
+	yield win.Zotero_Lookup.accept(textbox);
+	return promise;
+});
 
-describe.skip("Add Item by Identifier", function() {
+describe("Add Item by Identifier", function() {
 	var win;
 	
 	before(function* () {
+		if (Zotero.automatedTest) {
+			this.skip();
+			return;
+		}
 		win = yield loadZoteroPane();
 	});
 	
 	after(function() {
-		win.close();
+		if (win) {
+			win.close();
+		}
 	});
 	
 	// TODO: mock external services: https://github.com/zotero/zotero/issues/699