Use extension-less filenames when adding multiple attachments at once (#4649)

And use a new notifier queue in the ZP routine (like we already did in
the item tree) to prevent notifier events from firing in the middle.
This commit is contained in:
Abe Jellinek 2024-08-29 14:54:30 -04:00 • committed by Dan Stillman
parent ee9ea5d5a1
commit dbce2b33b3
4 changed files with 120 additions and 59 deletions

View file

@ -2575,6 +2575,10 @@ var ItemTree = class ItemTree extends LibraryTree {
}
}
// If we have more than one file, we only want to call setAutoAttachmentTitle()
// at the end, once the attachments know whether they have siblings
let delaySetAutoAttachmentTitle = data.length > 1;
for (var i=0; i<data.length; i++) {
var file = data[i];
@ -2644,6 +2648,7 @@ var ItemTree = class ItemTree extends LibraryTree {
item = await Zotero.Attachments.linkFromFile({
file,
title: delaySetAutoAttachmentTitle ? '' : undefined,
parentItemID,
collections: parentCollectionID ? [parentCollectionID] : undefined,
saveOptions: {
@ -2659,6 +2664,7 @@ var ItemTree = class ItemTree extends LibraryTree {
item = await Zotero.Attachments.importFromFile({
file,
title: delaySetAutoAttachmentTitle ? '' : undefined,
fileBaseName,
libraryID: targetLibraryID,
parentItemID,
@ -2682,6 +2688,12 @@ var ItemTree = class ItemTree extends LibraryTree {
addedItems.push(item);
}
}
if (delaySetAutoAttachmentTitle) {
for (let item of addedItems) {
item.setAutoAttachmentTitle();
await item.saveTx();
}
}
// Select children created after drag-drop onto a top-level item
if (parentItemID && addedItems.length) {
await this.selectItems(addedItems.map(item => item.id));

View file

@ -4725,72 +4725,98 @@ var ZoteroPane = new function () {
files = fp.files;
}
var addedItems = [];
var notifierQueue = new Zotero.Notifier.Queue();
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.Attachments.shouldAutoRenameFile(link, libraryID)) {
let parentItem = Zotero.Items.get(parentItemID);
if (!parentItem.numNonHTMLFileAttachments()) {
fileBaseName = await Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
parentItem, files[0]
);
}
}
}
// If not adding to an item, add to the current collection
else {
collection = this.getSelectedCollection(true);
}
for (let file of files) {
let item;
if (link) {
// Rename linked file, with unique suffix if necessary
try {
if (fileBaseName) {
let ext = Zotero.File.getExtension(file);
let newName = await Zotero.File.rename(
file,
fileBaseName + (ext ? '.' + ext : ''),
{
unique: true
}
try {
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.Attachments.shouldAutoRenameFile(link, libraryID)) {
let parentItem = Zotero.Items.get(parentItemID);
if (!parentItem.numNonHTMLFileAttachments()) {
fileBaseName = await Zotero.Attachments.getRenamedFileBaseNameIfAllowedType(
parentItem, files[0]
);
// Update path in case the name was changed to be unique
file = PathUtils.join(PathUtils.parent(file), newName);
}
}
catch (e) {
Zotero.logError(e);
}
item = await Zotero.Attachments.linkFromFile({
file,
parentItemID,
collections: collection ? [collection] : undefined
});
}
// If not adding to an item, add to the current collection
else {
if (file.endsWith(".lnk")) {
let win = Services.wm.getMostRecentWindow("navigator:browser");
win.ZoteroPane.displayCannotAddShortcutMessage(file);
continue;
}
item = await Zotero.Attachments.importFromFile({
file,
libraryID,
fileBaseName,
parentItemID,
collections: collection ? [collection] : undefined
});
collection = this.getSelectedCollection(true);
}
addedItems.push(item);
// If we have more than one file, we only want to call setAutoAttachmentTitle()
// at the end, once the attachments know whether they have siblings
let delaySetAutoAttachmentTitle = files.length > 1;
for (let file of files) {
let item;
if (link) {
// Rename linked file, with unique suffix if necessary
try {
if (fileBaseName) {
let ext = Zotero.File.getExtension(file);
let newName = await Zotero.File.rename(
file,
fileBaseName + (ext ? '.' + ext : ''),
{
unique: true
}
);
// Update path in case the name was changed to be unique
file = PathUtils.join(PathUtils.parent(file), newName);
}
}
catch (e) {
Zotero.logError(e);
}
item = await Zotero.Attachments.linkFromFile({
file,
title: delaySetAutoAttachmentTitle ? '' : undefined,
parentItemID,
collections: collection ? [collection] : undefined,
saveOptions: {
notifierQueue
},
});
}
else {
if (file.endsWith(".lnk")) {
let win = Services.wm.getMostRecentWindow("navigator:browser");
win.ZoteroPane.displayCannotAddShortcutMessage(file);
continue;
}
item = await Zotero.Attachments.importFromFile({
file,
libraryID,
fileBaseName,
title: delaySetAutoAttachmentTitle ? '' : undefined,
parentItemID,
collections: collection ? [collection] : undefined,
saveOptions: {
notifierQueue
},
});
}
addedItems.push(item);
}
if (delaySetAutoAttachmentTitle) {
for (let item of addedItems) {
item.setAutoAttachmentTitle();
await item.saveTx();
}
}
}
finally {
await Zotero.Notifier.commit(notifierQueue);
}
// Select added child attachments

View file

@ -1402,8 +1402,7 @@ describe("Zotero.ItemTree", function () {
mozItemCount: 2,
})
var item1 = Zotero.Items.get((await promise)[0]);
var item2 = Zotero.Items.get((await waitForItemEvent('add'))[0]);
var [item1, item2] = Zotero.Items.get(await promise);
var progressWindow = await recognizerPromise;
progressWindow.close();
@ -1607,6 +1606,7 @@ describe("Zotero.ItemTree", function () {
var parentRow = view.getRowIndexByID(parentItem.id);
var originalFileName = 'empty.pdf';
var originalFilenameWithoutExtension = 'empty';
var file = getTestDataDirectory();
file.append(originalFileName);
@ -1629,6 +1629,10 @@ describe("Zotero.ItemTree", function () {
assert.equal(item.parentItemID, parentItem.id);
var path = await item.getFilePathAsync();
assert.equal(OS.Path.basename(path), originalFileName);
for (let item of Zotero.Items.get(itemIDs)) {
assert.equal(item.getField('title'), originalFilenameWithoutExtension);
}
});
it("should set an automatic title on the first file attachment of each supported type", async function () {

View file

@ -1567,7 +1567,26 @@ describe("ZoteroPane", function () {
assert.equal(parentItem.getAttachments().length, 4);
assert.equal(epubAttachment.getField('title'), Zotero.getString('file-type-ebook'));
});
it("shouldn't set type-based titles when multiple attachments of the same type are added at once", async function () {
let parentItem = await createDataObject('item');
// Add two PDF attachments at once, which will get titles based on their filenames
let file = getTestDataDirectory();
file.append('test.pdf');
let [pdfAttachment1, pdfAttachment2] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path, file.path]);
assert.equal(parentItem.getAttachments().length, 2);
assert.equal(pdfAttachment1.getField('title'), 'test');
assert.equal(pdfAttachment2.getField('title'), 'test');
// Add an EPUB attachment, which will get a default title
file = getTestDataDirectory();
file.append('stub.epub');
let [epubAttachment] = await zp.addAttachmentFromDialog(false, parentItem.id, [file.path]);
assert.equal(parentItem.getAttachments().length, 3);
assert.equal(epubAttachment.getField('title'), Zotero.getString('file-type-ebook'));
});
it("should select added file attachment", async function () {
let parentItem = await createDataObject('item');