From 30cb2e172172ed805189012f06cf09e5433192cb Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Sat, 3 Aug 2024 01:30:07 -0400 Subject: [PATCH] Retrieve Metadata & Create Parent: Use `autoRenameFiles.fileTypes` (#4488) --- chrome/content/zotero/xpcom/attachments.js | 6 +++ chrome/content/zotero/xpcom/data/item.js | 4 +- .../content/zotero/xpcom/recognizeDocument.js | 8 ++-- chrome/content/zotero/zoteroPane.js | 2 +- test/tests/recognizeDocumentTest.js | 40 +++++++++++++++++-- test/tests/zoteroPaneTest.js | 21 +++++++++- 6 files changed, 68 insertions(+), 13 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 5e9d1c9f9d..855f410ca0 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -2503,6 +2503,12 @@ Zotero.Attachments = new function () { }; + this.shouldAutoRenameAttachment = function (attachment) { + return Zotero.Attachments.shouldAutoRenameFile(attachment.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) + && Zotero.Attachments.isRenameAllowedForType(attachment.attachmentContentType); + }; + + this.getRenamedFileBaseNameIfAllowedType = async function (parentItem, file) { var contentType = file.endsWith('.pdf') // Don't bother reading file if there's a .pdf extension diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 3b84c6ec44..55d65c481c 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3907,9 +3907,7 @@ Zotero.Item.prototype.setAutoAttachmentTitle = function ({ ignoreAutoRenamePrefs // file is being renamed, give it a default title ("PDF", "Webpage", etc.) let isFirstOfType = this.parentItemID && this.parentItem.numFileAttachmentsWithContentType(this.attachmentContentType) <= 1; - let isBeingRenamed = ignoreAutoRenamePrefs - || (Zotero.Attachments.shouldAutoRenameFile(this.isLinkedFileAttachment()) - && Zotero.Attachments.isRenameAllowedForType(this.attachmentContentType)); + let isBeingRenamed = ignoreAutoRenamePrefs || Zotero.Attachments.shouldAutoRenameAttachment(this); if (isFirstOfType && isBeingRenamed) { let defaultTitle = this._getDefaultTitleForAttachmentContentType(); if (defaultTitle !== null) { diff --git a/chrome/content/zotero/xpcom/recognizeDocument.js b/chrome/content/zotero/xpcom/recognizeDocument.js index a15b37b71d..e950da1ee8 100644 --- a/chrome/content/zotero/xpcom/recognizeDocument.js +++ b/chrome/content/zotero/xpcom/recognizeDocument.js @@ -292,7 +292,7 @@ Zotero.RecognizeDocument = new function () { var originalFilename = PathUtils.filename(path); // Rename attachment file to match new metadata - if (Zotero.Attachments.shouldAutoRenameFile(attachment.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE)) { + if (Zotero.Attachments.shouldAutoRenameAttachment(attachment)) { let ext = Zotero.File.getExtension(path); let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: originalTitle }); let newName = fileBaseName + (ext ? '.' + ext : ''); @@ -300,11 +300,9 @@ Zotero.RecognizeDocument = new function () { if (result !== true) { throw new Error("Error renaming " + path); } + attachment.setAutoAttachmentTitle({ ignoreAutoRenamePrefs: true }); + await attachment.saveTx(); } - - // Rename attachment title - attachment.setAutoAttachmentTitle(); - await attachment.saveTx(); try { let win = Zotero.getMainWindow(); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 590e149663..428d555e29 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -5343,7 +5343,7 @@ var ZoteroPane = new function() } for (let item of items) { - if (Zotero.Attachments.shouldAutoRenameFile(item.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE)) { + if (Zotero.Attachments.shouldAutoRenameAttachment(item)) { let path = item.getFilePath(); if (!path) { Zotero.debug('No path for attachment ' + item.key); diff --git a/test/tests/recognizeDocumentTest.js b/test/tests/recognizeDocumentTest.js index d4c5bc1f94..14525f3db5 100644 --- a/test/tests/recognizeDocumentTest.js +++ b/test/tests/recognizeDocumentTest.js @@ -238,7 +238,7 @@ describe("Document Recognition", function() { 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();; + var itemTitle = Zotero.Utilities.randomString(); Zotero.RecognizeDocument.recognizeStub = async function () { return createDataObject('item', { title: itemTitle }); }; @@ -279,10 +279,44 @@ describe("Document Recognition", function() { Zotero.getString('file-type-pdf') ); }); - + + it("shouldn't rename or change the title of a file attachment with a disabled type", async function () { + Zotero.Prefs.set('autoRenameFiles.fileTypes', 'x-nonexistent/type'); + + var itemTitle = Zotero.Utilities.randomString(); + Zotero.RecognizeDocument.recognizeStub = async function () { + return createDataObject('item', { title: itemTitle }); + }; + + var attachment = await importPDFAttachment(); + assert.equal(attachment.getField('title'), 'test'); + + 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.xhtml")[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'); + + // The title should not have changed + assert.equal(attachment.getField('title'), 'test'); + }); + 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();; + var itemTitle = Zotero.Utilities.randomString(); Zotero.RecognizeDocument.recognizeStub = async function () { return createDataObject('item', { title: itemTitle }); }; diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 6d5a2b3829..3e63a4fe87 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1698,7 +1698,26 @@ describe("ZoteroPane", function() { assert.equal(attachment.getField('title'), Zotero.getString('file-type-pdf')); }); - it("should not rename a linked attachment or set an automatic title when linked file renaming disabled", async function () { + it("shouldn't rename or change the title of an attachment with a disabled type", async function () { + Zotero.Prefs.set('autoRenameFiles.fileTypes', 'x-nonexistent/type'); + + let file = getTestDataDirectory(); + file.append('test.pdf'); + let attachment = await Zotero.Attachments.linkFromFile({ + file, + title: 'Attachment title' + }); + assert.equal(attachment.attachmentFilename, 'test.pdf'); + + let parent = await createParent(); + assert.equal(attachment.parentItem, parent); + assert.equal(attachment.attachmentFilename, 'test.pdf'); + assert.equal(attachment.getField('title'), 'Attachment title'); + + Zotero.Prefs.clear('autoRenameFiles.fileTypes'); + }); + + it("shouldn't rename a linked attachment or set an automatic title when linked file renaming disabled", async function () { Zotero.Prefs.set('autoRenameFiles.linked', false); let file = getTestDataDirectory();