From 7cb95f41297477788ddbf014915dc9bdba9c1ac8 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Wed, 10 Jan 2018 00:39:16 -0500
Subject: [PATCH] Automatically rename dragged file attachments from parent
 metadata

Rename happens if only one file is dragged and the parent item has no
existing file attachments.

Closes #1405
---
 chrome/content/zotero/xpcom/attachments.js  |  14 ++-
 chrome/content/zotero/xpcom/itemTreeView.js |  20 ++++
 test/tests/itemTreeViewTest.js              | 117 ++++++++++++++++++++
 3 files changed, 149 insertions(+), 2 deletions(-)

diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index e79fc7968..3555029c9 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -40,6 +40,7 @@ Zotero.Attachments = new function(){
 	 * @param {Integer} [options.libraryID]
 	 * @param {Integer[]|String[]} [options.parentItemID] - Parent item to add item to
 	 * @param {Integer[]} [options.collections] - Collection keys or ids to add new item to
+	 * @param {String} [options.fileBaseName]
 	 * @param {String} [options.contentType]
 	 * @param {String} [options.charset]
 	 * @param {Object} [options.saveOptions] - Options to pass to Zotero.Item::save()
@@ -50,15 +51,24 @@ Zotero.Attachments = new function(){
 		
 		var libraryID = options.libraryID;
 		var file = Zotero.File.pathToFile(options.file);
+		var path = file.path;
+		var leafName = file.leafName;
 		var parentItemID = options.parentItemID;
 		var collections = options.collections;
+		var fileBaseName = options.fileBaseName;
 		var contentType = options.contentType;
 		var charset = options.charset;
 		var saveOptions = options.saveOptions;
 		
-		var newName = Zotero.File.getValidFileName(file.leafName);
+		if (fileBaseName) {
+			let ext = Zotero.File.getExtension(path);
+			var newName = fileBaseName + (ext != '' ? '.' + ext : '');
+		}
+		else {
+			var newName = Zotero.File.getValidFileName(OS.Path.basename(leafName));
+		}
 		
-		if (file.leafName.endsWith(".lnk")) {
+		if (leafName.endsWith(".lnk")) {
 			throw new Error("Cannot add Windows shortcut");
 		}
 		if (parentItemID && collections) {
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 7e138a09f..032e408cb 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -3233,9 +3233,26 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 		
 		var notifierQueue = new Zotero.Notifier.Queue;
 		try {
+			let parentItem;
+			let numExistingFileAttachments;
+			if (parentItemID) {
+				parentItem = Zotero.Items.get(parentItemID);
+				numExistingFileAttachments = parentItem.getAttachments()
+					.map(itemID => Zotero.Items.get(itemID))
+					.filter(item => item.isFileAttachment())
+					.length;
+			}
+			
 			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) {
@@ -3270,6 +3287,7 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 							yield Zotero.Attachments.importFromURL({
 								libraryID: targetLibraryID,
 								url,
+								fileBaseName,
 								parentItemID,
 								saveOptions: {
 									notifierQueue
@@ -3306,8 +3324,10 @@ Zotero.ItemTreeView.prototype.drop = Zotero.Promise.coroutine(function* (row, or
 						win.ZoteroPane.displayCannotAddShortcutMessage(file.path);
 						continue;
 					}
+					
 					yield Zotero.Attachments.importFromFile({
 						file,
+						fileBaseName,
 						libraryID: targetLibraryID,
 						parentItemID,
 						collections: parentCollectionID ? [parentCollectionID] : undefined,
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
index 3a4ee67a2..3f474892d 100644
--- a/test/tests/itemTreeViewTest.js
+++ b/test/tests/itemTreeViewTest.js
@@ -872,5 +872,122 @@ describe("Zotero.ItemTreeView", function() {
 				(yield Zotero.File.getBinaryContentsAsync(pdfPath))
 			);
 		});
+		
+		it("should rename a child attachment using parent metadata if no existing file attachments", 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 = getTestDataDirectory();
+			file.append('test.png');
+			
+			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();
+			assert.equal(title, parentTitle + '.png');
+			assert.equal(OS.Path.basename(path), parentTitle + '.png');
+		});
+		
+		it("shouldn't rename a 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 });
+			await Zotero.Attachments.linkFromFile({
+				file: OS.Path.join(getTestDataDirectory().path, 'test.png'),
+				parentItemID: parentItem.id
+			});
+			var parentRow = view.getRowIndexByID(parentItem.id);
+			
+			var file = getTestDataDirectory();
+			file.append('test.png');
+			
+			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();
+			assert.equal(title, 'test.png');
+			assert.equal(OS.Path.basename(path), 'test.png');
+		});
+		
+		it("shouldn't rename a 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 file = getTestDataDirectory();
+			file.append('test.png');
+			
+			var promise = waitForItemEvent('add');
+			
+			itemsView.drop(parentRow, 0, {
+				dropEffect: 'copy',
+				effectAllowed: 'copy',
+				types: {
+					contains: function (type) {
+						return type == 'application/x-moz-file';
+					}
+				},
+				mozItemCount: 2,
+				mozGetDataAt: function (type, i) {
+					if (type == 'application/x-moz-file' && i <= 1) {
+						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, 'test.png');
+			assert.equal(OS.Path.basename(path), 'test.png');
+		});
 	});
 })