Standardize connector server behavior for saves to read-only libraries

Return a 500 for read-only libraries for all save modes. Read-only views
within editable libraries will save to the library root.

Addresses #185, RIS/BibTeX interception to read-only view behaves
differently from save button

chrome/content/zotero/xpcom/server_connector.js

@@ -48,7 +48,6 @@ Zotero.Server.Connector = {
 					case 'L':
 						library = Zotero.Libraries.get(id);
 						editable = library.editable;
-						Zotero.debug("LIB IS " + editable);
 						break;
 					
 					case 'C':
@@ -372,11 +371,9 @@ Zotero.Server.Connector.SaveItem.prototype = {
 		var { library, collection, editable } = Zotero.Server.Connector.getSaveTarget();
 		var libraryID = library.libraryID;
 		
-		// If library isn't editable (or directly editable, in the case of My Publications), switch to
+		if (!library.editable) {
-		// My Library if present and editable, and otherwise fail
-		if (!library.editable || library.libraryType == 'publications') {
 			Zotero.logError("Can't add item to read-only library " + library.name);
-			return [500, "application/json", JSON.stringify({libraryEditable: false})];
+			return [500, "application/json", JSON.stringify({ libraryEditable: false })];
 		}
 		
 		var cookieSandbox = data.uri
@@ -461,23 +458,9 @@ Zotero.Server.Connector.SaveSnapshot.prototype = {
 		var { library, collection, editable } = Zotero.Server.Connector.getSaveTarget();
 		var libraryID = library.libraryID;
 		
-		// If library isn't editable (or directly editable, in the case of My Publications), switch to
+		if (!library.editable) {
-		// My Library if present and editable, and otherwise fail
+			Zotero.logError("Can't add item to read-only library " + library.name);
-		if (!library.editable || library.libraryType == 'publications') {
+			return [500, "application/json", JSON.stringify({ libraryEditable: false })];
-			let userLibrary = Zotero.Libraries.userLibrary;
-			if (userLibrary && userLibrary.editable) {
-				let zp = Zotero.getActiveZoteroPane();
-				if (zp) {
-					yield zp.collectionsView.selectLibrary(userLibrary.id);
-				}
-				library = userLibrary;
-				libraryID = userLibrary.id;
-				collection = null;
-			}
-			else {
-				Zotero.logError("Can't add item to read-only library " + library.name);
-				return 500;
-			}
 		}
 		
 		// determine whether snapshot can be saved
@@ -644,11 +627,14 @@ Zotero.Server.Connector.Import.prototype = {
 		}
 		translate.setTranslator(translators[0]);
 		var { library, collection, editable } = Zotero.Server.Connector.getSaveTarget();
-		let arg = {};
+		if (!library.editable) {
-		if (editable) {
+			Zotero.logError("Can't import into read-only library " + library.name);
-			arg = { libraryID: library.libraryID, collections: collection ? [collection.id] : null };
+			return [500, "application/json", JSON.stringify({ libraryEditable: false })];
 		}
-		let items = yield translate.translate(arg);
+		let items = yield translate.translate({
+			libraryID: library.libraryID,
+			collections: collection ? [collection.id] : null
+		});
 		return [201, "application/json", JSON.stringify(items)];
 	})
 }

test/tests/server_connectorTest.js

@@ -338,22 +338,14 @@ describe("Connector Server", function () {
 			assert.isTrue(collection.hasItem(item.id));
 		});
 		
-		it("should save a webpage item to My Library if a read-only library is selected", function* () {
+		it("should respond with 500 if a read-only library is selected", function* () {
 			var group = yield createGroup({
 				editable: false
 			});
 			yield selectLibrary(win, group.libraryID);
 			yield waitForItemsLoad(win);
 			
-			// saveSnapshot saves parent and child before returning
+			var req = yield Zotero.HTTP.request(
-			var ids1, ids2;
-			var promise = waitForItemEvent('add').then(function (ids) {
-				ids1 = ids;
-				return waitForItemEvent('add').then(function (ids) {
-					ids2 = ids;
-				});
-			});
-			yield Zotero.HTTP.request(
 				'POST',
 				connectorServerPath + "/connector/saveSnapshot",
 				{
@@ -363,29 +355,27 @@ describe("Connector Server", function () {
 					body: JSON.stringify({
 						url: "http://example.com",
 						html: "<html><head><title>Title</title><body>Body</body></html>"
-					})
+					}),
+					successCodes: false
 				}
 			);
 			
-			assert.isTrue(promise.isFulfilled());
+			assert.equal(req.status, 500);
+			assert.isFalse(JSON.parse(req.responseText).libraryEditable);
 			
-			// Check parent item
+			// The selection should remain
-			assert.lengthOf(ids1, 1);
-			var item = Zotero.Items.get(ids1[0]);
-			assert.equal(Zotero.ItemTypes.getName(item.itemTypeID), 'webpage');
-			assert.equal(item.getField('title'), 'Title');
-			assert.equal(item.libraryID, Zotero.Libraries.userLibraryID);
-			// Item should've been saved to My Library
-			assert.equal(item.libraryID, Zotero.Libraries.userLibraryID);
-			
-			// My Library should've been selected
 			assert.equal(
-				win.ZoteroPane.collectionsView.getSelectedLibraryID(), Zotero.Libraries.userLibraryID
+				win.ZoteroPane.collectionsView.getSelectedLibraryID(), group.libraryID
 			);
 		});
 	});
 	
 	describe("/connector/savePage", function() {
+		before(async function () {
+			await selectLibrary(win);
+			await waitForItemsLoad(win);
+		});
+		
 		// TEMP: Wait for indexing to complete, which happens after a 1-second delay, after a 201 has
 		// been returned to the connector. Would be better to make sure indexing has completed.
 		afterEach(function* () {
@@ -538,5 +528,37 @@ describe("Connector Server", function () {
 			let itemId = yield addedItemIDPromise;
 			assert.isTrue(collection.hasItem(itemId[0]));
 		});
+		
+		
+		it('should respond with 500 if read-only library is selected', function* () {
+			var group = yield createGroup({
+				editable: false
+			});
+			yield selectLibrary(win, group.libraryID);
+			yield waitForItemsLoad(win);
+			
+			var resource = `@book{test1,
+  title={Test1},
+  author={Owl},
+  year={1000},
+  publisher={Curly Braces Publishing}
+}`;
+			var req = yield Zotero.HTTP.request(
+				'POST',
+				endpoint,
+				{
+					headers: { "Content-Type": "application/x-bibtex" },
+					body: resource,
+					successCodes: false
+				}
+			);
+			assert.equal(req.status, 500);
+			assert.isFalse(JSON.parse(req.responseText).libraryEditable);
+			
+			// The selection should remain
+			assert.equal(
+				win.ZoteroPane.collectionsView.getSelectedLibraryID(), group.libraryID
+			);
+		});
 	});
 });