Don't merge into a missing snapshot attachment (#5459)

chrome/content/zotero/xpcom/data/items.js

@@ -1121,6 +1121,11 @@ Zotero.Items = function() {
 
 		let masterAttachments = (await this.getAsync(item.getAttachments(true)))
 			.filter(attachment => attachment.isWebAttachment());
+		let masterAttachmentFilesExist = await Promise.all(masterAttachments.map(
+			attachment => attachment.attachmentLinkMode === Zotero.Attachments.LINK_MODE_LINKED_URL
+				|| attachment.fileExists()
+		));
+		masterAttachments = masterAttachments.filter((_, i) => masterAttachmentFilesExist[i]);
 
 		for (let otherItem of otherItems) {
 			for (let otherAttachment of await this.getAsync(otherItem.getAttachments(true))) {
@@ -1192,6 +1197,7 @@ Zotero.Items = function() {
 			.filter(attachment => attachment.isFileAttachment());
 		let hashes = new Map();
 		await Promise.all(attachments.map(async (attachment) => {
+			// attachmentHash and _hashAttachmentText() implicitly check file existence
 			let hash = hashType === 'bytes'
 				? await attachment.attachmentHash
 				: await this._hashAttachmentText(attachment);

test/content/support.js

@@ -1017,13 +1017,37 @@ function importFileAttachment(filename, options = {}) {
 }
 
 
-function importTextAttachment() {
-	return importFileAttachment('test.txt', { contentType: 'text/plain', charset: 'utf-8' });
+function importTextAttachment(parentItem, options = {}) {
+	return importFileAttachment('test.txt', {
+		contentType: 'text/plain',
+		charset: 'utf-8',
+		parentID: parentItem ? parentItem.id : null,
+		title: options.title
+	});
 }
 
 
-function importHTMLAttachment() {
-	return importFileAttachment('test.html', { contentType: 'text/html', charset: 'utf-8' });
+function importHTMLAttachment(parentItem, options = {}) {
+	return importFileAttachment('test.html', {
+		contentType: 'text/html',
+		charset: 'utf-8',
+		parentID: parentItem ? parentItem.id : null,
+		title: options.title
+	});
+}
+
+
+function importSnapshotAttachment(parentItem, options = {}) {
+	let file = getTestDataDirectory();
+	file.append('test.html');
+	return Zotero.Attachments.importSnapshotFromFile({
+		title: options.title || 'Snapshot',
+		url: 'http://example.com',
+		file,
+		contentType: 'text/html',
+		charset: 'utf-8',
+		parentItemID: parentItem ? parentItem.id : null,
+	});
 }
 
 

test/tests/itemsTest.js

@@ -517,7 +517,7 @@ describe("Zotero.Items", function () {
 			assert.isFalse(attachment2.deleted);
 		});
 		
-		it("should ignore attachment with missing file", async function () {
+		it("should ignore PDF attachment with missing file", async function () {
 			let item1 = await createDataObject('item');
 			let attachment1 = await importPDFAttachment(item1);
 			
@@ -798,6 +798,50 @@ describe("Zotero.Items", function () {
 			assert.isTrue(attachment3.deleted);
 		});
 
+		it("should keep only snapshot that exists when merging non-master snapshot (missing) with equivalent master snapshot (exists)", async function () {
+			let item1 = await createDataObject('item');
+			let file = getTestDataDirectory();
+			file.append('test.html');
+			let attachment1 = await importSnapshotAttachment(item1);
+
+			let item2 = item1.clone();
+			await item2.saveTx();
+			let attachment2 = await importSnapshotAttachment(item2);
+			// Delete the second attachment file
+			await OS.File.remove(await attachment2.getFilePathAsync());
+
+			await Zotero.Items.merge(item1, [item2]);
+
+			assert.isFalse(item1.deleted);
+			assert.isFalse(attachment1.deleted);
+			assert.isTrue(await IOUtils.exists(await attachment1.getFilePathAsync()));
+			assert.equal(item1.numAttachments(true), 1);
+			assert.isTrue(item2.deleted);
+			assert.isTrue(attachment2.deleted);
+		});
+
+		it("should both snapshots when merging non-master snapshot (exists) with equivalent master snapshot (missing)", async function () {
+			let item1 = await createDataObject('item');
+			let file = getTestDataDirectory();
+			file.append('test.html');
+			let attachment1 = await importSnapshotAttachment(item1);
+			// Delete the first attachment file
+			await OS.File.remove(await attachment1.getFilePathAsync());
+
+			let item2 = item1.clone();
+			await item2.saveTx();
+			let attachment2 = await importSnapshotAttachment(item2);
+
+			await Zotero.Items.merge(item1, [item2]);
+
+			assert.isFalse(item1.deleted);
+			assert.isFalse(attachment1.deleted);
+			assert.equal(item1.numAttachments(true), 2);
+			assert.isTrue(item2.deleted);
+			assert.isFalse(attachment2.deleted);
+			assert.isTrue(await IOUtils.exists(await attachment2.getFilePathAsync()));
+		});
+		
 		it("should move related items of merged attachments", async function () {
 			let relatedItem = await createDataObject('item');