From 88f886cb303cdce359e22d8b108a1b81176f13e5 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 22 Jan 2021 00:18:51 -0500
Subject: [PATCH] Don't delete all annotations when updating attachment item

---
 chrome/content/zotero/xpcom/data/item.js | 26 ++++++++++++++++++------
 test/tests/itemTest.js                   | 19 +++++++++++++++++
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index b6c4fe10e0..87edb15069 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -1725,11 +1725,20 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		}
 	}
 	if (this._changed.attachmentData) {
-		let sql = "REPLACE INTO itemAttachments "
-			+ "(itemID, parentItemID, linkMode, contentType, charsetID, path, "
-				+ "syncState, storageModTime, storageHash, "
-				+ "lastProcessedModificationTime, pageIndex) "
-			+ "VALUES (?,?,?,?,?,?,?,?,?,?,?)";
+		let sql = "";
+		let cols = [
+			'parentItemID', 'linkMode', 'contentType', 'charsetID', 'path', 'syncState',
+			'storageModTime', 'storageHash', 'lastProcessedModificationTime', 'pageIndex'
+		];
+		// TODO: Replace with UPSERT after SQLite 3.24.0
+		if (isNew) {
+			sql = "INSERT INTO itemAttachments "
+				+ "(itemID, " + cols.join(", ") + ") "
+				+ "VALUES (?,?,?,?,?,?,?,?,?,?,?)";
+		}
+		else {
+			sql = "UPDATE itemAttachments SET " + cols.join("=?, ") + "=? WHERE itemID=?";
+		}
 		let linkMode = this.attachmentLinkMode;
 		let contentType = this.attachmentContentType;
 		let charsetID = this.attachmentCharset
@@ -1747,7 +1756,6 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		}
 		
 		let params = [
-			itemID,
 			parentItemID,
 			{ int: linkMode },
 			contentType ? { string: contentType } : null,
@@ -1759,6 +1767,12 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 			lastProcessedModificationTime || null,
 			typeof pageIndex === 'number' ? pageIndex : null
 		];
+		if (isNew) {
+			params.unshift(itemID);
+		}
+		else {
+			params.push(itemID);
+		}
 		yield Zotero.DB.queryAsync(sql, params);
 		
 		// Clear cached child attachments of the parent
diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js
index a76d94e59e..49c8e45c7c 100644
--- a/test/tests/itemTest.js
+++ b/test/tests/itemTest.js
@@ -1501,6 +1501,25 @@ describe("Zotero.Item", function () {
 			assert.lengthOf(item.getAttachments(), 1);
 			assert.lengthOf(item.getNotes(), 1);
 		});
+		
+		// Make sure we're updating annotations rather than replacing and triggering ON DELETE CASCADE
+		it("should update attachment without deleting child annotations", async function () {
+			var attachment = await importFileAttachment('test.pdf');
+			var annotation = await createAnnotation('highlight', attachment);
+			
+			var annotationIDs = await Zotero.DB.columnQueryAsync(
+				"SELECT itemID FROM itemAnnotations WHERE parentItemID=?", attachment.id
+			);
+			assert.lengthOf(annotationIDs, 1);
+			
+			attachment.attachmentLastProcessedModificationTime = Date.now();
+			await attachment.saveTx();
+			
+			annotationIDs = await Zotero.DB.columnQueryAsync(
+				"SELECT itemID FROM itemAnnotations WHERE parentItemID=?", attachment.id
+			);
+			assert.lengthOf(annotationIDs, 1);
+		});
 	})