Fix potential sync error after Replace Online Library in group

https://forums.zotero.org/discussion/104431/syncing-problem

Replace Online Library can upload annotations created by others in a
group library, so if the upload resulted in a local write, "Cannot edit
item in library" was thrown, since annotations by others aren't
writable. This should've only been a problem if the uploaded data was
actually modified by the server, but we were also checking whether
objects were editable before checking if they had actually changed, so
it would happen for any upload of another person's annotation.

This fixes the order of checks when saving objects and makes an
edit-check exception for saving uploaded data for group annotations.

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

@@ -987,6 +987,11 @@ Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env)
 	
 	env.isNew = !this.id;
 	
+	if (!this.hasChanged()) {
+		Zotero.debug(this._ObjectType + ' ' + this.id + ' has not changed', 4);
+		return false;
+	}
+	
 	if (!env.options.skipEditCheck) {
 		if (!this.isEditable()) {
 			throw new Error("Cannot edit " + this._objectType + " in library "
@@ -999,11 +1004,6 @@ Zotero.DataObject.prototype._initSave = Zotero.Promise.coroutine(function* (env)
 		throw new Error("Cannot add " + this._objectType + " to a " + targetLib.libraryType + " library");
 	}
 	
-	if (!this.hasChanged()) {
-		Zotero.debug(this._ObjectType + ' ' + this.id + ' has not changed', 4);
-		return false;
-	}
-	
 	// Undo registerObject() on failure
 	if (env.isNew) {
 		var func = function () {

chrome/content/zotero/xpcom/sync/syncEngine.js

@@ -1331,7 +1331,15 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func
 						skipSyncedUpdate: true,
 						// We want to minimize the times when server writes actually result in local
 						// updates, but when they do, don't update the user-visible timestamp
-						skipDateModifiedUpdate: true
+						skipDateModifiedUpdate: true,
+						// Replace Online Library can reupload annotations created by others in a
+						// group library, and the server could theoretically override a value, so
+						// allow updating local annotations even if they were created by someone
+						// else. There might be other cases where this is necessary, but let's start
+						// with this.
+						skipEditCheck: objectType == 'item'
+							&& this.library.isGroup
+							&& toSave[i].isAnnotation()
 					});
 				}
 				if (this.library.libraryVersion == this.library.storageVersion) {

test/tests/syncEngineTest.js

@@ -3049,6 +3049,55 @@ describe("Zotero.Sync.Data.Engine", function () {
 		});
 		
 		
+		it("should allow applying remotely saved version to local annotation in group library", async function () {
+			var group = await createGroup({
+				libraryVersion: 5
+			});
+			var libraryID = group.libraryID;
+			({ engine, client, caller } = await setup({ libraryID }));
+			
+			var createdByUserID = 2352512;
+			await Zotero.Users.setName(createdByUserID, 'user');
+			
+			var attachment = await importFileAttachment('test.pdf', { libraryID });
+			attachment.synced = true;
+			await attachment.saveTx();
+			var annotation = await createAnnotation('highlight', attachment);
+			annotation.createdByUserID = createdByUserID;
+			await annotation.saveTx({
+				skipEditCheck: true
+			});
+			var responseJSON = annotation.toResponseJSON();
+			responseJSON.version = 10;
+			responseJSON.data.version = 10;
+			var newComment = 'new comment';
+			responseJSON.data.annotationComment = newComment;
+			
+			let response = {
+				successful: {
+					"0": responseJSON
+				},
+				unchanged: {},
+				failed: {}
+			};
+			setResponse({
+				method: "POST",
+				url: `groups/${group.id}/items`,
+				status: 200,
+				headers: {
+					"Last-Modified-Version": 10
+				},
+				json: response
+			})
+			
+			var result = await engine._startUpload();
+			
+			assert.equal(result, engine.UPLOAD_RESULT_SUCCESS);
+			assert.equal(annotation.version, 10);
+			assert.equal(annotation.annotationComment, newComment);
+		});
+		
+		
 		it("should prompt to reset library on 403 write response and reset on accept", function* () {
 			var group = yield createGroup({
 				libraryVersion: 5