Skip to content

Commit

Permalink
Standardize connector server behavior for saves to read-only libraries
Browse files Browse the repository at this point in the history 
Return a 500 for read-only libraries for all save modes. Read-only views
within editable libraries will save to the library root.

Addresses zotero#185, RIS/BibTeX interception to read-only view behaves
differently from save button
  • Loading branch information
@dstillman
dstillman committed Sep 27, 2017
1 parent 18d15d8 commit 48d4d2d
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 49 deletions.
38 changes: 12 additions & 26 deletions chrome/content/zotero/xpcom/server_connector.js
View file
Expand Up @@ -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':
Expand Down Expand Up @@ -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
// My Library if present and editable, and otherwise fail
if (!library.editable || library.libraryType == 'publications') {
if (!library.editable) {
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
Expand Down Expand Up @@ -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
// My Library if present and editable, and otherwise fail
if (!library.editable || library.libraryType == 'publications') {
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;
}
if (!library.editable) {
Zotero.logError("Can't add item to read-only library " + library.name);
return [500, "application/json", JSON.stringify({ libraryEditable: false })];
}

// determine whether snapshot can be saved
Expand Down Expand Up @@ -644,11 +627,14 @@ Zotero.Server.Connector.Import.prototype = {
}
translate.setTranslator(translators[0]);
var { library, collection, editable } = Zotero.Server.Connector.getSaveTarget();
let arg = {};
if (editable) {
arg = { libraryID: library.libraryID, collections: collection ? [collection.id] : null };
if (!library.editable) {
Zotero.logError("Can't import into read-only library " + library.name);
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)];
})
}
Expand Down
68 changes: 45 additions & 23 deletions test/tests/server_connectorTest.js
View file
Expand Up @@ -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 ids1, ids2;
var promise = waitForItemEvent('add').then(function (ids) {
ids1 = ids;
return waitForItemEvent('add').then(function (ids) {
ids2 = ids;
});
});
yield Zotero.HTTP.request(
var req = yield Zotero.HTTP.request(
'POST',
connectorServerPath + "/connector/saveSnapshot",
{
Expand All @@ -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());

// Check parent item
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);
assert.equal(req.status, 500);
assert.isFalse(JSON.parse(req.responseText).libraryEditable);

// My Library should've been selected
// The selection should remain
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* () {
Expand Down Expand Up @@ -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
);
});
});
});

0 comments on commit 48d4d2d

Please sign in to comment.