diff --git a/chrome/content/zotero/xpcom/connector/server_connector.js b/chrome/content/zotero/xpcom/connector/server_connector.js index 5d55db2f0..ba31e5ef8 100644 --- a/chrome/content/zotero/xpcom/connector/server_connector.js +++ b/chrome/content/zotero/xpcom/connector/server_connector.js @@ -28,19 +28,46 @@ Zotero.Server.Connector = { _waitingForSelection: {}, getSaveTarget: function () { - var zp = Zotero.getActiveZoteroPane(), - library = null, - collection = null, - editable = true; - try { - library = Zotero.Libraries.get(zp.getSelectedLibraryID()); - collection = zp.getSelectedCollection(); - editable = zp.collectionsView.editable; + var zp = Zotero.getActiveZoteroPane(); + var library = null; + var collection = null; + var editable = null; + + if (zp && zp.collectionsView) { + if (zp.collectionsView.editable) { + library = Zotero.Libraries.get(zp.getSelectedLibraryID()); + collection = zp.getSelectedCollection(); + editable = true; + } + // If not editable, switch to My Library if it exists and is editable + else { + let userLibrary = Zotero.Libraries.userLibrary; + if (userLibrary && userLibrary.editable) { + Zotero.debug("Save target isn't editable -- switching to My Library"); + + // Don't wait for this, because we don't want to slow down all conenctor + // requests by making this function async + zp.collectionsView.selectByID(userLibrary.treeViewID); + + library = userLibrary; + collection = null; + editable = true; + } + } } - catch (e) { + else { let id = Zotero.Prefs.get('lastViewedFolder'); if (id) { ({ library, collection, editable } = this.resolveTarget(id)); + if (!editable) { + let userLibrary = Zotero.Libraries.userLibrary; + if (userLibrary && userLibrary.editable) { + Zotero.debug("Save target isn't editable -- switching to My Library"); + let treeViewID = userLibrary.treeViewID; + Zotero.Prefs.set('lastViewedFolder', treeViewID); + ({ library, collection, editable } = this.resolveTarget(treeViewID)); + } + } } } @@ -48,7 +75,7 @@ Zotero.Server.Connector = { // (which should never be the case anymore) if (!library) { let userLibrary = Zotero.Libraries.userLibrary; - if (userLibrary) { + if (userLibrary && userLibrary.editable) { library = userLibrary; } } @@ -152,9 +179,9 @@ Zotero.Server.Connector.SaveSession.prototype.update = async function (targetID, this._currentTags = tags || ""; // Select new destination in collections pane - var win = Zotero.getActiveZoteroPane(); - if (win && win.collectionsView) { - await win.collectionsView.selectByID(targetID); + var zp = Zotero.getActiveZoteroPane(); + if (zp && zp.collectionsView) { + await zp.collectionsView.selectByID(targetID); } // If window is closed, select target collection re-open else { @@ -189,12 +216,12 @@ Zotero.Server.Connector.SaveSession.prototype.update = async function (targetID, await this._updateItems(this._items); // If a single item was saved, select it (or its parent, if it now has one) - if (win && win.collectionsView && this._items.size == 1) { + if (zp && zp.collectionsView && this._items.size == 1) { let item = Array.from(this._items)[0]; item = item.isTopLevelItem() ? item : item.parentItem; // Don't select if in trash if (!item.deleted) { - await win.selectItem(item.id); + await zp.selectItem(item.id); } } }; @@ -485,6 +512,8 @@ Zotero.Server.Connector.SavePage.prototype = { */ init: function(url, data, sendResponseCallback) { var { library, collection, editable } = Zotero.Server.Connector.getSaveTarget(); + + // Shouldn't happen as long as My Library exists if (!library.editable) { Zotero.logError("Can't add item to read-only library " + library.name); return sendResponseCallback(500, "application/json", JSON.stringify({ libraryEditable: false })); @@ -601,7 +630,7 @@ Zotero.Server.Connector.SaveItems.prototype = { } yield session.update(targetID); - // TODO: Default to My Library root, since it's changeable + // Shouldn't happen as long as My Library exists if (!library.editable) { Zotero.logError("Can't add item to read-only library " + library.name); return [500, "application/json", JSON.stringify({ libraryEditable: false })]; @@ -719,7 +748,7 @@ Zotero.Server.Connector.SaveSnapshot.prototype = { } await session.update(collection ? collection.treeViewID : library.treeViewID); - // TODO: Default to My Library root, since it's changeable + // Shouldn't happen as long as My Library exists if (!library.editable) { Zotero.logError("Can't add item to read-only library " + library.name); return [500, "application/json", JSON.stringify({ libraryEditable: false })]; @@ -955,6 +984,8 @@ Zotero.Server.Connector.Import.prototype = { translate.setTranslator(translators[0]); var { library, collection, editable } = Zotero.Server.Connector.getSaveTarget(); var libraryID = library.libraryID; + + // Shouldn't happen as long as My Library exists if (!library.editable) { Zotero.logError("Can't import into read-only library " + library.name); return [500, "application/json", JSON.stringify({ libraryEditable: false })]; diff --git a/test/tests/server_connectorTest.js b/test/tests/server_connectorTest.js index fb207981a..d2f6f525e 100644 --- a/test/tests/server_connectorTest.js +++ b/test/tests/server_connectorTest.js @@ -163,7 +163,7 @@ describe("Connector Server", function () { }); - it("should respond with 500 if read-only library is selected", function* () { + it("should switch to My Library if read-only library is selected", function* () { var group = yield createGroup({ editable: false }); @@ -188,7 +188,8 @@ describe("Connector Server", function () { uri: "http://example.com" }; - var req = yield Zotero.HTTP.request( + var promise = waitForItemEvent('add'); + var reqPromise = Zotero.HTTP.request( 'POST', connectorServerPath + "/connector/saveItems", { @@ -200,13 +201,19 @@ describe("Connector Server", function () { } ); - assert.equal(req.status, 500); - assert.isFalse(JSON.parse(req.responseText).libraryEditable); - - // The selection should remain + // My Library be selected, and the item should be in it + var ids = yield promise; assert.equal( - win.ZoteroPane.collectionsView.getSelectedLibraryID(), group.libraryID + win.ZoteroPane.collectionsView.getSelectedLibraryID(), + Zotero.Libraries.userLibraryID ); + assert.lengthOf(ids, 1); + var item = Zotero.Items.get(ids[0]); + assert.equal(item.libraryID, Zotero.Libraries.userLibraryID); + assert.equal(Zotero.ItemTypes.getName(item.itemTypeID), 'newspaperArticle'); + + var req = yield reqPromise; + assert.equal(req.status, 201); }); it("should use the provided proxy to deproxify item url", function* () { @@ -355,14 +362,15 @@ describe("Connector Server", function () { stub.restore(); }); - it("should respond with 500 if a read-only library is selected", function* () { + it("should switch to My Library if a read-only library is selected", function* () { var group = yield createGroup({ editable: false }); yield selectLibrary(win, group.libraryID); yield waitForItemsLoad(win); - var req = yield Zotero.HTTP.request( + var promise = waitForItemEvent('add'); + var reqPromise = Zotero.HTTP.request( 'POST', connectorServerPath + "/connector/saveSnapshot", { @@ -377,13 +385,18 @@ describe("Connector Server", function () { } ); - assert.equal(req.status, 500); - assert.isFalse(JSON.parse(req.responseText).libraryEditable); - - // The selection should remain + // My Library be selected, and the item should be in it + var ids = yield promise; assert.equal( - win.ZoteroPane.collectionsView.getSelectedLibraryID(), group.libraryID + win.ZoteroPane.collectionsView.getSelectedLibraryID(), + Zotero.Libraries.userLibraryID ); + assert.lengthOf(ids, 1); + var item = Zotero.Items.get(ids[0]); + assert.equal(item.libraryID, Zotero.Libraries.userLibraryID); + + var req = yield reqPromise; + assert.equal(req.status, 201); }); }); @@ -714,14 +727,15 @@ describe("Connector Server", function () { var collection = yield createDataObject('collection'); yield waitForItemsLoad(win); - var addedItemIDPromise = waitForItemEvent('add'); var resource = `@book{test1, title={Test1}, author={Owl}, year={1000}, publisher={Curly Braces Publishing} }`; - var response = yield Zotero.HTTP.request( + + var addedItemIDsPromise = waitForItemEvent('add'); + var req = yield Zotero.HTTP.request( 'POST', endpoint, { @@ -729,15 +743,15 @@ describe("Connector Server", function () { body: resource } ); - assert.equal(response.status, 201); - assert.equal(JSON.parse(response.responseText)[0].title, 'Test1'); + assert.equal(req.status, 201); + assert.equal(JSON.parse(req.responseText)[0].title, 'Test1'); - let itemId = yield addedItemIDPromise; - assert.isTrue(collection.hasItem(itemId[0])); + let itemIDs = yield addedItemIDsPromise; + assert.isTrue(collection.hasItem(itemIDs[0])); }); - it('should respond with 500 if read-only library is selected', function* () { + it('should switch to My Library if read-only library is selected', function* () { var group = yield createGroup({ editable: false }); @@ -750,6 +764,8 @@ describe("Connector Server", function () { year={1000}, publisher={Curly Braces Publishing} }`; + + var addedItemIDsPromise = waitForItemEvent('add'); var req = yield Zotero.HTTP.request( 'POST', endpoint, @@ -759,13 +775,16 @@ describe("Connector Server", function () { successCodes: false } ); - assert.equal(req.status, 500); - assert.isFalse(JSON.parse(req.responseText).libraryEditable); - // The selection should remain + assert.equal(req.status, 201); assert.equal( - win.ZoteroPane.collectionsView.getSelectedLibraryID(), group.libraryID + win.ZoteroPane.collectionsView.getSelectedLibraryID(), + Zotero.Libraries.userLibraryID ); + + let itemIDs = yield addedItemIDsPromise; + var item = Zotero.Items.get(itemIDs[0]); + assert.equal(item.libraryID, Zotero.Libraries.userLibraryID); }); }); });