mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Fix linked-file rename pref when retrieving metadata

Browse source 
Due to a typo in d0f7fd6df7, linked files were still being renamed even
with the pref off if metadata was found for the file. The test I added
was only for adding a file to an existing item, which didn't trigger
metadata retrieval.

This also adds a hook for stubbing the actual PDF recognition process so
we can test certain behaviors without making HTTP requests.
This commit is contained in:
Dan Stillman 2019-03-27 06:35:58 -04:00
parent a5c7955670
commit 71f9420cff
3 changed files with 90 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
chrome/content/zotero/xpcom/recognizePDF.js
View file

@ -47,6 +47,7 @@ Zotero.RecognizePDF = new function () {
_queue = [];
});
this.recognizeStub = null;
/**
* Triggers queue processing and returns when all items in the queue are processed
@ -287,7 +288,7 @@ Zotero.RecognizePDF = new function () {
var originalFilename = OS.Path.basename(path);
// Rename attachment file to match new metadata
if (Zotero.Attachments.shouldAutoRenameFile(attachment.linkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE)) {
if (Zotero.Attachments.shouldAutoRenameFile(attachment.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE)) {
let ext = Zotero.File.getExtension(path);
let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem);
let newName = fileBaseName + (ext ? '.' + ext : '');
@ -406,9 +407,13 @@ Zotero.RecognizePDF = new function () {
/**
* Retrieves metadata for a PDF and saves it as an item
* @param {Zotero.Item} item
* @return {Promise}
* @return {Promise<Zotero.Item>} - New item
*/
async function _recognize(item) {
if (Zotero.RecognizePDF.recognizeStub) {
return Zotero.RecognizePDF.recognizeStub(item);
}
let filePath = await item.getFilePath();
if (!filePath || !await OS.File.exists(filePath)) throw new Zotero.Exception.Alert('recognizePDF.fileNotFound');

2
test/tests/itemTreeViewTest.js
View file

@ -1139,7 +1139,7 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
});
it("shouldn't rename a linked child attachment using parent metadata if no existing file attachments and pref disabled", async function () {
it("shouldn't rename a linked child attachment using parent metadata if pref disabled", async function () {
Zotero.Prefs.set('autoRenameFiles.linked', false);
var view = zp.itemsView;

91
test/tests/recognizePDFTest.js
View file

@ -2,8 +2,6 @@ describe("PDF Recognition", function() {
var win;
before(function* () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(60000);
// Load Zotero pane and install PDF tools
yield Zotero.Promise.all([
@ -20,6 +18,8 @@ describe("PDF Recognition", function() {
win.close();
}
Zotero.ProgressQueues.get('recognize').cancel();
Zotero.RecognizePDF.recognizeStub = null;
Zotero.Prefs.clear('autoRenameFiles.linked');
});
after(function() {
@ -28,7 +28,8 @@ describe("PDF Recognition", function() {
}
});
it("should recognize a PDF by DOI", async function () {
it("should recognize a PDF by DOI and rename the file", async function () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(30000);
// Import the PDF
var testdir = getTestDataDirectory();
@ -64,6 +65,7 @@ describe("PDF Recognition", function() {
});
it("should recognize a PDF by arXiv ID", async function () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(30000);
// Import the PDF
var testdir = getTestDataDirectory();
@ -89,15 +91,10 @@ describe("PDF Recognition", function() {
while (progressWindow.document.getElementById("label").value != completeStr) {
await Zotero.Promise.delay(20);
}
// The file should have been renamed
assert.equal(
attachment.attachmentFilename,
Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
);
});
it("should put new item in same collection", async function () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(30000);
// Import the PDF
var testdir = getTestDataDirectory();
@ -128,6 +125,7 @@ describe("PDF Recognition", function() {
});
it("should recognize PDF by arXiv ID and put new item in same collection in group library", async function () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(30000);
var testdir = getTestDataDirectory();
testdir.append("recognizePDF_test_arXiv.pdf");
@ -159,6 +157,7 @@ describe("PDF Recognition", function() {
});
it.skip("should recognize PDF by ISBN and put new item in same collection in group library", async function () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(30000);
var testdir = getTestDataDirectory();
testdir.append("recognizePDF_test_ISBN.pdf");
@ -189,6 +188,7 @@ describe("PDF Recognition", function() {
});
it("should recognize PDF by title and put new item in same collection in group library", async function () {
if (Zotero.automatedTest) this.skip(); // TODO: Mock services
this.timeout(30000);
var testdir = getTestDataDirectory();
testdir.append("recognizePDF_test_title.pdf");
@ -217,4 +217,77 @@ describe("PDF Recognition", function() {
assert.isTrue(collection.hasItem(item.id));
});
it("should rename a linked file attachment using parent metadata if no existing file attachments and pref enabled", async function () {
Zotero.Prefs.set('autoRenameFiles.linked', true);
var itemTitle = Zotero.Utilities.randomString();;
Zotero.RecognizePDF.recognizeStub = async function () {
return createDataObject('item', { title: itemTitle });
};
// Link to the PDF
var tempDir = await getTempDirectory();
var tempFile = OS.Path.join(tempDir, 'test.pdf');
await OS.File.copy(OS.Path.join(getTestDataDirectory().path, 'test.pdf'), tempFile);
var attachment = await Zotero.Attachments.linkFromFile({
file: tempFile
});
win.ZoteroPane.recognizeSelected();
var addedIDs = await waitForItemEvent("add");
var modifiedIDs = await waitForItemEvent("modify");
assert.lengthOf(addedIDs, 1);
var item = Zotero.Items.get(addedIDs[0]);
assert.equal(item.getField("title"), itemTitle);
assert.lengthOf(modifiedIDs, 2);
// Wait for status to show as complete
var progressWindow = getWindows("chrome://zotero/content/progressQueueDialog.xul")[0];
var completeStr = Zotero.getString("general.finished");
while (progressWindow.document.getElementById("label").value != completeStr) {
await Zotero.Promise.delay(20);
}
// The file should have been renamed
assert.equal(
attachment.attachmentFilename,
Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
);
});
it("shouldn't rename a linked file attachment using parent metadata if pref disabled", async function () {
Zotero.Prefs.set('autoRenameFiles.linked', false);
var itemTitle = Zotero.Utilities.randomString();;
Zotero.RecognizePDF.recognizeStub = async function () {
return createDataObject('item', { title: itemTitle });
};
// Link to the PDF
var tempDir = await getTempDirectory();
var tempFile = OS.Path.join(tempDir, 'test.pdf');
await OS.File.copy(OS.Path.join(getTestDataDirectory().path, 'test.pdf'), tempFile);
var attachment = await Zotero.Attachments.linkFromFile({
file: tempFile
});
win.ZoteroPane.recognizeSelected();
var addedIDs = await waitForItemEvent("add");
var modifiedIDs = await waitForItemEvent("modify");
assert.lengthOf(addedIDs, 1);
var item = Zotero.Items.get(addedIDs[0]);
assert.equal(item.getField("title"), itemTitle);
assert.lengthOf(modifiedIDs, 2);
// Wait for status to show as complete
var progressWindow = getWindows("chrome://zotero/content/progressQueueDialog.xul")[0];
var completeStr = Zotero.getString("general.finished");
while (progressWindow.document.getElementById("label").value != completeStr) {
await Zotero.Promise.delay(20);
}
// The file should not have been renamed
assert.equal(attachment.attachmentFilename, 'test.pdf');
});
});