Automatic attachment file renaming changes

Automatic renaming is now done for dragging of an external file onto an
item (as added in 7cb95f41) as well as dragging as a linked file,
dragging a non-native (not text or HTML) URL, "Attach Link to File…",
"Attach Stored Copy of File…", and "Retrieve Metadata for PDF". It only
applies if a single file is being added and if the parent item has no
non-HTML attachments. By default, the renaming only applies to PDFs, but
this can be changed with the renameAttachmentFiles.automatic.fileTypes
hidden pref.

A new General pref, "Automatically rename attachment files using parent
metadata", controls whether the renaming happens.

Files saved via web translators are renamed regardless of this pref,
because they would often be gibberish otherwise.

Closes #113
This commit is contained in:
Dan Stillman 2018-02-27 01:40:07 -05:00
parent 68879a0ffc
commit f8b41c971c
10 changed files with 335 additions and 96 deletions

View file

@ -37,6 +37,7 @@
<preference id="pref-reportTranslationFailure" name="extensions.zotero.reportTranslationFailure" type="bool"/>
<preference id="pref-automaticSnapshots" name="extensions.zotero.automaticSnapshots" type="bool"/>
<preference id="pref-downloadAssociatedFiles" name="extensions.zotero.downloadAssociatedFiles" type="bool"/>
<preference id="pref-renameAttachmentFiles" name="extensions.zotero.renameAttachmentFiles.automatic" type="bool"/>
<preference id="pref-automaticTags" name="extensions.zotero.automaticTags" type="bool"/>
<preference id="pref-trashAutoEmptyDays" name="extensions.zotero.trashAutoEmptyDays" type="int"/>
@ -122,6 +123,7 @@
label="&zotero.preferences.automaticSnapshots;"
preference="pref-automaticSnapshots"/>
<checkbox label="&zotero.preferences.downloadAssociatedFiles;" preference="pref-downloadAssociatedFiles"/>
<checkbox label="&zotero.preferences.renameAttachmentFiles;" preference="pref-renameAttachmentFiles"/>
<checkbox label="&zotero.preferences.automaticTags;" preference="pref-automaticTags"/>
<hbox align="center">
<label value="&zotero.preferences.trashAutoEmptyDaysPre;"/>

View file

@ -245,9 +245,18 @@ Zotero.Attachments = new function(){
/**
* @param {Object} options - 'libraryID', 'url', 'parentItemID', 'collections', 'title',
* 'fileBaseName', 'contentType', 'referrer', 'cookieSandbox',
* 'saveOptions'
* @param {Object} options
* @param {Integer} options.libraryID
* @param {String} options.url
* @param {Integer} [options.parentItemID]
* @param {Integer[]} [options.collections]
* @param {String} [options.title]
* @param {String} [options.fileBaseName]
* @param {Boolean} [options.renameIfAllowedType=false]
* @param {String} [options.contentType]
* @param {String} [options.referrer]
* @param {CookieSandbox} [options.cookieSandbox]
* @param {Object} [options.saveOptions]
* @return {Promise<Zotero.Item>} - A promise for the created attachment item
*/
this.importFromURL = Zotero.Promise.coroutine(function* (options) {
@ -257,6 +266,7 @@ Zotero.Attachments = new function(){
var collections = options.collections;
var title = options.title;
var fileBaseName = options.fileBaseName;
var renameIfAllowedType = options.renameIfAllowedType;
var contentType = options.contentType;
var referrer = options.referrer;
var cookieSandbox = options.cookieSandbox;
@ -320,6 +330,11 @@ Zotero.Attachments = new function(){
// Save using remote web browser persist
var externalHandlerImport = Zotero.Promise.coroutine(function* (contentType) {
// Rename attachment
if (renameIfAllowedType && !fileBaseName && this.getRenamedFileTypes().includes(contentType)) {
let parentItem = Zotero.Items.get(parentItemID);
fileBaseName = this.getFileBaseNameFromItem(parentItem);
}
if (fileBaseName) {
let ext = _getExtensionFromURL(url, contentType);
var fileName = fileBaseName + (ext != '' ? '.' + ext : '');
@ -804,6 +819,30 @@ Zotero.Attachments = new function(){
}
this.getRenamedFileTypes = function () {
try {
var types = Zotero.Prefs.get('renameAttachmentFiles.automatic.fileTypes');
return types ? types.split(',') : [];
}
catch (e) {
return [];
}
};
this.getRenamedFileBaseNameIfAllowedType = async function (parentItem, file) {
var types = this.getRenamedFileTypes();
var contentType = file.endsWith('.pdf')
// Don't bother reading file if there's a .pdf extension
? 'application/pdf'
: await Zotero.MIME.getMIMETypeFromFile(file);
if (!types.includes(contentType)) {
return false;
}
return this.getFileBaseNameFromItem(parentItem);
}
/**
* Create directory for attachment files within storage directory
*

View file

@ -2120,6 +2120,15 @@ Zotero.Item.prototype.numAttachments = function (includeTrashed) {
}
Zotero.Item.prototype.numNonHTMLFileAttachments = function () {
this._requireData('childItems');
return this.getAttachments()
.map(itemID => Zotero.Items.get(itemID))
.filter(item => item.isFileAttachment() && item.attachmentContentType != 'text/html')
.length;
};
Zotero.Item.prototype.getFile = function () {
Zotero.debug("Zotero.Item.prototype.getFile() is deprecated -- use getFilePath[Async]()", 2);

View file

@ -3231,26 +3231,22 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
var notifierQueue = new Zotero.Notifier.Queue;
try {
// If there's a single file being added to a parent, automatic renaming is enabled,
// and there are no other non-HTML attachments, we'll rename the file as long as it's
// an allowed type. The dragged data could be a URL, so we don't yet know the file type.
// This should be kept in sync with ZoteroPane.addAttachmentFromDialog().
let renameIfAllowedType = false;
let parentItem;
let numExistingFileAttachments;
if (parentItemID) {
if (parentItemID && data.length == 1 && Zotero.Prefs.get('renameAttachmentFiles.automatic')) {
parentItem = Zotero.Items.get(parentItemID);
numExistingFileAttachments = parentItem.getAttachments()
.map(itemID => Zotero.Items.get(itemID))
.filter(item => item.isFileAttachment())
.length;
if (!parentItem.numNonHTMLFileAttachments()) {
renameIfAllowedType = true;
}
}
for (var i=0; i<data.length; i++) {
var file = data[i];
let fileBaseName;
// If only one item is being dragged and it's the only attachment, run
// "Rename File from Parent Metadata" automatically
if (data.length == 1 && parentItem && !numExistingFileAttachments) {
fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem);
}
if (dataType == 'text/x-moz-url') {
var url = data[i];
if (url.indexOf('file:///') == 0) {
@ -3281,7 +3277,7 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
yield Zotero.Attachments.importFromURL({
libraryID: targetLibraryID,
url,
fileBaseName,
renameIfAllowedType,
parentItemID,
saveOptions: {
notifierQueue
@ -3298,7 +3294,36 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
// Otherwise file, so fall through
}
file = file.path;
// Rename file if it's an allowed type
let fileBaseName = false;
if (renameIfAllowedType) {
fileBaseName = yield Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
parentItem, file
);
}
if (dropEffect == 'link') {
// Rename linked file, with unique suffix if necessary
try {
if (fileBaseName) {
let ext = Zotero.File.getExtension(file);
let newName = yield Zotero.File.rename(
file,
fileBaseName + (ext ? '.' + ext : ''),
{
unique: true
}
);
// Update path in case the name was changed to be unique
file = OS.Path.join(OS.Path.dirname(file), newName);
}
}
catch (e) {
Zotero.logError(e);
}
yield Zotero.Attachments.linkFromFile({
file,
parentItemID,
@ -3309,9 +3334,9 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
});
}
else {
if (file.leafName.endsWith(".lnk")) {
if (file.endsWith(".lnk")) {
let win = Services.wm.getMostRecentWindow("navigator:browser");
win.ZoteroPane.displayCannotAddShortcutMessage(file.path);
win.ZoteroPane.displayCannotAddShortcutMessage(file);
continue;
}
@ -3328,10 +3353,10 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
// If moving, delete original file
if (dragData.dropEffect == 'move') {
try {
file.remove(false);
yield OS.File.remove(file);
}
catch (e) {
Components.utils.reportError("Error deleting original file " + file.path + " after drag");
Zotero.logError("Error deleting original file " + file + " after drag");
}
}
}

View file

@ -241,30 +241,48 @@ Zotero.RecognizePDF = new function () {
* @return {Promise}
*/
async function _processItem(itemID) {
let item = await Zotero.Items.getAsync(itemID);
let attachment = await Zotero.Items.getAsync(itemID);
if (!item || item.parentItemID) throw new Zotero.Exception.Alert('recognizePDF.fileNotFound');
let newItem = await _recognize(item);
if (newItem) {
// put new item in same collections as the old one
let itemCollections = item.getCollections();
await Zotero.DB.executeTransaction(async function () {
for (let itemCollection of itemCollections) {
let collection = Zotero.Collections.get(itemCollection);
await collection.addItem(newItem.id);
}
// put old item as a child of the new item
item.parentID = newItem.id;
await item.save();
});
return newItem
if (!attachment || attachment.parentItemID) {
throw new Zotero.Exception.Alert('recognizePDF.error');
}
return null;
let parentItem = await _recognize(attachment);
if (!parentItem) {
return null;
}
// Put new item in same collections as the old one
let collections = attachment.getCollections();
await Zotero.DB.executeTransaction(async function () {
if (collections.length) {
for (let collection of collections) {
parentItem.addToCollection(collection.id);
}
await parentItem.save();
}
// Put old item as a child of the new item
attachment.parentID = parentItem.id;
await attachment.save();
});
// Rename attachment file to match new metadata
if (Zotero.Prefs.get('renameAttachmentFiles')) {
let path = attachment.getFilePath();
let ext = Zotero.File.getExtension(path);
let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem);
let newName = fileBaseName + (ext ? '.' + ext : '');
let result = await attachment.renameAttachmentFile(newName, false, true);
if (result !== true) {
throw new Error("Error renaming " + path);
}
// Rename attachment title
attachment.setField('title', newName);
await attachment.saveTx();
}
return parentItem;
}
/**

View file

@ -3678,41 +3678,81 @@ var ZoteroPane = new function()
var fp = Components.classes["@mozilla.org/filepicker;1"]
.createInstance(nsIFilePicker);
fp.init(window, Zotero.getString('pane.item.attachments.select'), nsIFilePicker.modeOpenMultiple);
fp.appendFilters(Components.interfaces.nsIFilePicker.filterAll);
fp.appendFilters(nsIFilePicker.filterAll);
if(fp.show() == nsIFilePicker.returnOK)
{
if (!parentItemID) {
var collection = this.getSelectedCollection(true);
if (fp.show() != nsIFilePicker.returnOK) {
return;
}
var enumerator = fp.files;
var files = [];
while (enumerator.hasMoreElements()) {
let file = enumerator.getNext();
file.QueryInterface(Components.interfaces.nsIFile);
files.push(file.path);
}
var collection;
var fileBaseName;
if (parentItemID) {
// If only one item is being added, automatic renaming is enabled, and the parent item
// doesn't have any other non-HTML file attachments, rename the file.
// This should be kept in sync with itemTreeView::drop().
if (files.length == 1 && Zotero.Prefs.get('renameAttachmentFiles.automatic')) {
let parentItem = Zotero.Items.get(parentItemID);
if (!parentItem.numNonHTMLFileAttachments()) {
fileBaseName = await Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
parentItem, files[0]
);
}
}
var files = fp.files;
while (files.hasMoreElements()){
var file = files.getNext();
file.QueryInterface(Components.interfaces.nsILocalFile);
var attachmentID;
if (link) {
yield Zotero.Attachments.linkFromFile({
file: file,
parentItemID: parentItemID,
collections: collection ? [collection] : undefined
});
}
else {
if (file.leafName.endsWith(".lnk")) {
let wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
.getService(Components.interfaces.nsIWindowMediator);
let win = wm.getMostRecentWindow("navigator:browser");
win.ZoteroPane.displayCannotAddShortcutMessage(file.path);
continue;
}
// If not adding to an item, add to the current collection
else {
collection = this.getSelectedCollection(true);
}
for (let file of files) {
if (link) {
// Rename linked file, with unique suffix if necessary
try {
if (fileBaseName) {
let ext = Zotero.File.getExtension(file);
let newName = yield Zotero.File.rename(
file,
fileBaseName + (ext ? '.' + ext : ''),
{
unique: true
}
);
// Update path in case the name was changed to be unique
file = OS.Path.join(OS.Path.dirname(file), newName);
}
yield Zotero.Attachments.importFromFile({
file: file,
libraryID: libraryID,
parentItemID: parentItemID,
collections: collection ? [collection] : undefined
});
}
catch (e) {
Zotero.logError(e);
}
let item = yield Zotero.Attachments.linkFromFile({
file,
parentItemID,
collections: collection ? [collection] : undefined
});
}
else {
if (file.endsWith(".lnk")) {
let win = Services.wm.getMostRecentWindow("navigator:browser");
win.ZoteroPane.displayCannotAddShortcutMessage(file);
continue;
}
yield Zotero.Attachments.importFromFile({
file,
libraryID,
fileBaseName,
parentItemID,
collections: collection ? [collection] : undefined
});
}
}
});

View file

@ -27,6 +27,7 @@
<!ENTITY zotero.preferences.parseRISRefer "Use Zotero for downloaded BibTeX/RIS/Refer files">
<!ENTITY zotero.preferences.automaticSnapshots "Automatically take snapshots when creating items from web pages">
<!ENTITY zotero.preferences.downloadAssociatedFiles "Automatically attach associated PDFs and other files when saving items">
<!ENTITY zotero.preferences.renameAttachmentFiles "Automatically rename attachment files using parent metadata">
<!ENTITY zotero.preferences.automaticTags "Automatically tag items with keywords and subject headings">
<!ENTITY zotero.preferences.trashAutoEmptyDaysPre "Automatically remove items in the trash deleted more than">
<!ENTITY zotero.preferences.trashAutoEmptyDaysPost "days ago">

View file

@ -35,6 +35,8 @@ pref("extensions.zotero.automaticTags",true);
pref("extensions.zotero.fontSize", "1.0");
pref("extensions.zotero.layout", "standard");
pref("extensions.zotero.recursiveCollections", false);
pref("extensions.zotero.renameAttachmentFiles.automatic", true);
pref("extensions.zotero.renameAttachmentFiles.automatic.fileTypes", "application/pdf");
pref("extensions.zotero.attachmentRenameFormatString", '{%c - }{%y - }{%t{50}}');
pref("extensions.zotero.capitalizeTitles", false);
pref("extensions.zotero.launchNonNativeFiles", false);

View file

@ -695,6 +695,12 @@ describe("Zotero.ItemTreeView", function() {
file.append(pdfFilename);
pdfPath = file.path;
httpd.registerFile("/" + pdfFilename, file);
Zotero.Prefs.clear('renameAttachmentFiles.automatic');
});
afterEach(() => {
Zotero.Prefs.clear('renameAttachmentFiles.automatic');
});
after(function* () {
@ -781,7 +787,7 @@ describe("Zotero.ItemTreeView", function() {
assert.isFalse(view.isContainerEmpty(view.getRowIndexByID(item1.id)));
});
it("should create a top-level attachment when a file is dragged", function* () {
it("should create a stored top-level attachment when a file is dragged", function* () {
var file = getTestDataDirectory();
file.append('test.png');
@ -812,7 +818,7 @@ describe("Zotero.ItemTreeView", function() {
);
});
it("should create a top-level attachment when a URL is dragged", function* () {
it("should create a stored top-level attachment when a URL is dragged", function* () {
var promise = itemsView.waitForSelect();
itemsView.drop(0, -1, {
@ -840,7 +846,7 @@ describe("Zotero.ItemTreeView", function() {
);
});
it("should create a child attachment when a URL is dragged", function* () {
it("should create a stored child attachment when a URL is dragged", function* () {
var view = zp.itemsView;
var parentItem = yield createDataObject('item');
var parentRow = view.getRowIndexByID(parentItem.id);
@ -873,7 +879,7 @@ describe("Zotero.ItemTreeView", function() {
);
});
it("should rename a child attachment using parent metadata if no existing file attachments", async function () {
it("should rename a stored child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
@ -885,7 +891,7 @@ describe("Zotero.ItemTreeView", function() {
var parentRow = view.getRowIndexByID(parentItem.id);
var file = getTestDataDirectory();
file.append('test.png');
file.append('empty.pdf');
var promise = waitForItemEvent('add');
@ -910,11 +916,101 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
assert.equal(title, parentTitle + '.png');
assert.equal(OS.Path.basename(path), parentTitle + '.png');
assert.equal(title, parentTitle + '.pdf');
assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
});
it("shouldn't rename a child attachment using parent metadata if existing file attachments", async function () {
it("should rename a linked child attachment using parent metadata if no existing file attachments and pref enabled", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
await Zotero.Attachments.linkFromURL({
url: 'https://example.com',
title: 'Example',
parentItemID: parentItem.id
});
var parentRow = view.getRowIndexByID(parentItem.id);
var file = OS.Path.join(await getTempDirectory(), 'empty.pdf');
await OS.File.copy(
OS.Path.join(getTestDataDirectory().path, 'empty.pdf'),
file
);
file = Zotero.File.pathToFile(file);
var promise = waitForItemEvent('add');
itemsView.drop(parentRow, 0, {
dropEffect: 'link',
effectAllowed: 'link',
types: {
contains: function (type) {
return type == 'application/x-moz-file';
}
},
mozItemCount: 1,
mozGetDataAt: function (type, i) {
if (type == 'application/x-moz-file' && i == 0) {
return file;
}
}
})
var itemIDs = await promise;
var item = Zotero.Items.get(itemIDs[0]);
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
assert.equal(title, parentTitle + '.pdf');
assert.equal(OS.Path.basename(path), parentTitle + '.pdf');
});
it("shouldn't rename a stored child attachment using parent metadata if pref disabled", async function () {
Zotero.Prefs.set('renameAttachmentFiles.automatic', false);
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
await Zotero.Attachments.linkFromURL({
url: 'https://example.com',
title: 'Example',
parentItemID: parentItem.id
});
var parentRow = view.getRowIndexByID(parentItem.id);
var originalFileName = 'empty.pdf';
var file = getTestDataDirectory();
file.append(originalFileName);
var promise = waitForItemEvent('add');
itemsView.drop(parentRow, 0, {
dropEffect: 'copy',
effectAllowed: 'copy',
types: {
contains: function (type) {
return type == 'application/x-moz-file';
}
},
mozItemCount: 1,
mozGetDataAt: function (type, i) {
if (type == 'application/x-moz-file' && i == 0) {
return file;
}
}
})
var itemIDs = await promise;
var item = Zotero.Items.get(itemIDs[0]);
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
// Should match original filename, not parent title
assert.equal(title, originalFileName);
assert.equal(OS.Path.basename(path), originalFileName);
});
it("shouldn't rename a stored child attachment using parent metadata if existing file attachments", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
@ -924,8 +1020,9 @@ describe("Zotero.ItemTreeView", function() {
});
var parentRow = view.getRowIndexByID(parentItem.id);
var originalFileName = 'empty.pdf';
var file = getTestDataDirectory();
file.append('test.png');
file.append(originalFileName);
var promise = waitForItemEvent('add');
@ -950,18 +1047,19 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
assert.equal(title, 'test.png');
assert.equal(OS.Path.basename(path), 'test.png');
assert.equal(title, originalFileName);
assert.equal(OS.Path.basename(path), originalFileName);
});
it("shouldn't rename a child attachment using parent metadata if drag includes multiple files", async function () {
it("shouldn't rename a stored child attachment using parent metadata if drag includes multiple files", async function () {
var view = zp.itemsView;
var parentTitle = Zotero.Utilities.randomString();
var parentItem = await createDataObject('item', { title: parentTitle });
var parentRow = view.getRowIndexByID(parentItem.id);
var originalFileName = 'empty.pdf';
var file = getTestDataDirectory();
file.append('test.png');
file.append(originalFileName);
var promise = waitForItemEvent('add');
@ -986,8 +1084,8 @@ describe("Zotero.ItemTreeView", function() {
assert.equal(item.parentItemID, parentItem.id);
var title = item.getField('title');
var path = await item.getFilePathAsync();
assert.equal(title, 'test.png');
assert.equal(OS.Path.basename(path), 'test.png');
assert.equal(title, originalFileName);
assert.equal(OS.Path.basename(path), originalFileName);
});
});
})

View file

@ -27,31 +27,36 @@ describe("PDF Recognition", function() {
}
});
it("should recognize a PDF", function* () {
it("should recognize a PDF", async function () {
this.timeout(30000);
// Import the PDF
var testdir = getTestDataDirectory();
testdir.append("recognizePDF_test_GS.pdf");
var item = yield Zotero.Attachments.importFromFile({
var attachment = await Zotero.Attachments.importFromFile({
file: testdir
});
// Recognize the PDF
win.ZoteroPane.recognizeSelected();
var addedIDs = yield waitForItemEvent("add");
var modifiedIDs = yield waitForItemEvent("modify");
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"), "Scaling study of an improved fermion action on quenched lattices");
assert.lengthOf(modifiedIDs, 2);
yield Zotero.Promise.delay(0);
// Wait for status to show as complete
var progressWindow = getWindows("chrome://zotero/content/recognizePDFDialog.xul")[0];
var completeStr = Zotero.getString("recognizePDF.complete.label");
while (progressWindow.document.getElementById("label").value != completeStr) {
await Zotero.Promise.delay(20);
}
// The file should have been renamed
assert.equal(
progressWindow.document.getElementById("label").value,
Zotero.getString("recognizePDF.complete.label")
attachment.attachmentFilename,
Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
);
});
});