Don't delete all annotations when updating attachment item

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 "
+		let sql = "";
-			+ "(itemID, parentItemID, linkMode, contentType, charsetID, path, "
+		let cols = [
-				+ "syncState, storageModTime, storageHash, "
+			'parentItemID', 'linkMode', 'contentType', 'charsetID', 'path', 'syncState',
-				+ "lastProcessedModificationTime, pageIndex) "
+			'storageModTime', 'storageHash', 'lastProcessedModificationTime', 'pageIndex'
-			+ "VALUES (?,?,?,?,?,?,?,?,?,?,?)";
+		];
+		// 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

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);
+		});
 	})