Don't set author name of current user when copying annotation from group

Fixes #2358

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

@@ -902,9 +902,11 @@ Zotero.Items = function() {
 				}
 				let newAnnotation = annotation.clone(toItem.libraryID);
 				newAnnotation.parentItemID = toItem.id;
-				// If there's no explicit author and we're copying from a group, set the author
+				// If there's no explicit author and we're copying an annotation created by another
-				// to the creating user
+				// user from a group, set the author to the creating user
-				if (!annotation.annotationAuthorName && fromGroup) {
+				if (fromGroup
+						&& !annotation.annotationAuthorName
+						&& annotation.createdByUserID != Zotero.Users.getCurrentUserID()) {
 					newAnnotation.annotationAuthorName =
 						Zotero.Users.getName(annotation.createdByUserID);
 				}

test/tests/collectionTreeTest.js

@@ -815,6 +815,9 @@ describe("Zotero.CollectionTree", function() {
 			});
 			
 			it("should copy an item with a PDF attachment containing annotations to a group", async function () {
+				await Zotero.Users.setCurrentUserID(1);
+				await Zotero.Users.setName(1, 'Name');
+				
 				var group = await createGroup();
 				
 				var item = await createDataObject('item', false, { skipSelect: true });
@@ -860,6 +863,10 @@ describe("Zotero.CollectionTree", function() {
 			});
 			
 			it("should copy a group item with a PDF attachment containing annotations to the personal library", async function () {
+				await Zotero.Users.setCurrentUserID(1);
+				await Zotero.Users.setName(1, 'Name 1');
+				await Zotero.Users.setName(12345, 'Name 2');
+				
 				var group = await createGroup();
 				await cv.selectLibrary(group.libraryID);
 				
@@ -871,55 +878,15 @@ describe("Zotero.CollectionTree", function() {
 					parentItemID: groupItem.id
 				});
 				var annotation = await createAnnotation('highlight', attachment);
-				await Zotero.Users.setName(12345, 'Name');
+				await annotation.saveTx();
-				annotation.createdByUserID = 12345;
-				await annotation.saveTx({
-					skipEditCheck: true
-				});
 				
 				var ids = (await onDrop('item', 'L1', [groupItem.id])).ids;
 				var newItem = Zotero.Items.get(ids[0]);
 				
-				var newAttachment = Zotero.Items.get(newItem.getAttachments())[0];
-				
 				// Check annotation
+				var newAttachment = Zotero.Items.get(newItem.getAttachments())[0];
 				var annotations = newAttachment.getAnnotations();
 				assert.lengthOf(annotations, 1);
-				// Name transferred by Zotero.Items.copyChildItems()
-				assert.equal(annotations[0].annotationAuthorName, 'Name');
-				
-				return group.eraseTx();
-			});
-			
-			it("should preserve an explicit annotation author name when copied from a group to a personal library", async function () {
-				var group = await createGroup();
-				await cv.selectLibrary(group.libraryID);
-				
-				var groupItem = await createDataObject('item', { libraryID: group.libraryID });
-				var file = getTestDataDirectory();
-				file.append('test.pdf');
-				var attachment = await Zotero.Attachments.importFromFile({
-					file,
-					parentItemID: groupItem.id
-				});
-				var annotation = await createAnnotation('highlight', attachment);
-				await Zotero.Users.setName(1, 'Name');
-				annotation.createdByUserID = 1;
-				annotation.annotationAuthorName = 'Another Name';
-				await annotation.saveTx({
-					skipEditCheck: true
-				});
-				
-				var ids = (await onDrop('item', 'L1', [groupItem.id])).ids;
-				var newItem = Zotero.Items.get(ids[0]);
-				
-				var newAttachment = Zotero.Items.get(newItem.getAttachments())[0];
-				
-				// Check annotation
-				var annotations = newAttachment.getAnnotations();
-				assert.lengthOf(annotations, 1);
-				// Name transferred by Zotero.Items.copyChildItems()
-				assert.equal(annotations[0].annotationAuthorName, 'Another Name');
 				
 				return group.eraseTx();
 			});

test/tests/itemsTest.js

@@ -153,6 +153,71 @@ describe("Zotero.Items", function () {
 	});
 	
 	
+	describe("#copyChildItems()", function () {
+		var group;
+		
+		before(async function () {
+			group = await createGroup();
+		});
+		
+		after(async function () {
+			await group.eraseTx();
+		});
+		
+		it("should copy annotations from a group library to a personal library", async function () {
+			await Zotero.Users.setCurrentUserID(1);
+			await Zotero.Users.setName(1, 'Name 1');
+			await Zotero.Users.setName(12345, 'Name 2');
+			
+			var userLibraryID = Zotero.Libraries.userLibraryID;
+			
+			var item = await createDataObject('item', { libraryID: group.libraryID });
+			var file = getTestDataDirectory();
+			file.append('test.pdf');
+			var attachment = await Zotero.Attachments.importFromFile({
+				file,
+				parentItemID: item.id
+			});
+			// Annotation by this user
+			var annotation1 = await createAnnotation('highlight', attachment);
+			await annotation1.saveTx();
+			// Annotation by this user with createdByUserID set
+			var annotation2 = await createAnnotation('highlight', attachment);
+			annotation2.createdByUserID = 1;
+			await annotation2.saveTx({
+				skipEditCheck: true
+			});
+			// Annotation by another user
+			var annotation3 = await createAnnotation('highlight', attachment);
+			annotation3.createdByUserID = 12345;
+			await annotation3.saveTx({
+				skipEditCheck: true
+			});
+			
+			var newItem = item.clone(userLibraryID);
+			await newItem.saveTx();
+			var newAttachment = attachment.clone(userLibraryID);
+			await newAttachment.saveTx();
+			
+			await Zotero.DB.executeTransaction(async function () {
+				await Zotero.Items.copyChildItems(attachment, newAttachment);
+			});
+			
+			// Check annotations
+			var annotations = newAttachment.getAnnotations();
+			assert.lengthOf(annotations, 3);
+			var newAnnotation1 = annotations.find(o => o.annotationText == annotation1.annotationText);
+			var newAnnotation2 = annotations.find(o => o.annotationText == annotation2.annotationText);
+			var newAnnotation3 = annotations.find(o => o.annotationText == annotation3.annotationText);
+			// Current user's name shouldn't have been transferred
+			assert.isNull(newAnnotation1.annotationAuthorName);
+			assert.isNull(newAnnotation2.annotationAuthorName);
+			// Other user's should've been transferred
+			assert.equal(newAnnotation3.annotationAuthorName, 'Name 2');
+		});
+	});
+	
+	
 	describe("#merge()", function () {
 		it("should merge two items", function* () {
 			var item1 = yield createDataObject('item');