BC-11491 do not update pseudonyms when already existing (#6208)

Author: Loki-Afro

apps/server/src/modules/pseudonym/repo/external-tool-pseudonym.repo.integration.spec.ts

@@ -151,10 +151,10 @@ describe('ExternalToolPseudonymRepo', () => {
 			it('should create a new pseudonym if it does not exist', async () => {
 				const { domainObject } = setup();
 
-				const result: Pseudonym = await repo.createOrUpdate(domainObject);
+				const result: Pseudonym = await repo.createOrUpdate(domainObject.userId, domainObject.toolId);
 
 				expect(result.id).toBeTruthy();
-				expect(result.pseudonym).toEqual(domainObject.pseudonym);
+				expect(result.pseudonym).toEqual(expect.any(String));
 				expect(result.toolId).toEqual(domainObject.toolId);
 				expect(result.userId).toEqual(domainObject.userId);
 			});
@@ -177,23 +177,13 @@ describe('ExternalToolPseudonymRepo', () => {
 				};
 			};
 
-			it('should update the pseudonym value of the existing record', async () => {
+			it('should not change createdAt and pseudonym on update', async () => {
 				const { entity, domainObject } = await setup();
 
-				const result: Pseudonym = await repo.createOrUpdate(domainObject);
-
-				expect(result.id).toEqual(entity.id);
-				expect(result.pseudonym).toEqual(domainObject.pseudonym);
-				expect(result.toolId).toEqual(domainObject.toolId);
-				expect(result.userId).toEqual(domainObject.userId);
-			});
-
-			it('should not change createdAt on update', async () => {
-				const { entity, domainObject } = await setup();
-
-				const result: Pseudonym = await repo.createOrUpdate(domainObject);
+				const result: Pseudonym = await repo.createOrUpdate(domainObject.userId, domainObject.toolId);
 
 				expect(result.createdAt).toEqual(entity.createdAt);
+				expect(result.pseudonym).toEqual(entity.pseudonym);
 			});
 		});
 
@@ -221,14 +211,20 @@ describe('ExternalToolPseudonymRepo', () => {
 				const { domainObject1, domainObject2 } = setup();
 
 				await expect(
-					Promise.all([repo.createOrUpdate(domainObject1), repo.createOrUpdate(domainObject2)])
+					Promise.all([
+						repo.createOrUpdate(domainObject1.userId, domainObject1.toolId),
+						repo.createOrUpdate(domainObject2.userId, domainObject2.toolId),
+					])
 				).resolves.not.toThrow();
 			});
 
 			it('should result in exactly one document in the database', async () => {
 				const { domainObject1, domainObject2, sharedUserId, sharedToolId } = setup();
 
-				await Promise.all([repo.createOrUpdate(domainObject1), repo.createOrUpdate(domainObject2)]);
+				await Promise.all([
+					repo.createOrUpdate(domainObject1.userId, domainObject1.toolId),
+					repo.createOrUpdate(domainObject2.userId, domainObject2.toolId),
+				]);
 
 				const count = await em.count(ExternalToolPseudonymEntity, {
 					userId: new ObjectId(sharedUserId),

apps/server/src/modules/pseudonym/repo/external-tool-pseudonym.repo.ts

@@ -8,6 +8,7 @@ import { PseudonymSearchQuery } from '../domain';
 import { ExternalToolPseudonymEntity } from '../entity';
 import { PseudonymScope } from '../entity/pseudonym.scope';
 import { Pseudonym } from './pseudonym.do';
+import { v4 as uuidv4 } from 'uuid';
 
 @Injectable()
 export class ExternalToolPseudonymRepo {
@@ -33,22 +34,20 @@ export class ExternalToolPseudonymRepo {
 		return pseudonyms;
 	}
 
-	public async createOrUpdate(domainObject: Pseudonym): Promise<Pseudonym> {
+	public async createOrUpdate(userId: EntityId, toolId: EntityId): Promise<Pseudonym> {
 		const collection = this.em.getCollection(ExternalToolPseudonymEntity);
 
-		const userId = new ObjectId(domainObject.userId);
-		const toolId = new ObjectId(domainObject.toolId);
 		const now = new Date();
 
 		const result = await collection.findOneAndUpdate(
-			{ userId, toolId },
+			{ userId: new ObjectId(userId), toolId: new ObjectId(toolId) },
 			{
 				$set: {
-					pseudonym: domainObject.pseudonym,
 					updatedAt: now,
 				},
 				$setOnInsert: {
 					createdAt: now,
+					pseudonym: uuidv4(),
 				},
 			},
 			{

apps/server/src/modules/pseudonym/service/pseudonym.service.spec.ts

@@ -188,12 +188,7 @@ describe('PseudonymService', () => {
 
 				await service.findOrCreatePseudonym(user, externalTool);
 
-				expect(externalToolPseudonymRepo.createOrUpdate).toHaveBeenCalledWith(
-					expect.objectContaining({
-						userId: user.id,
-						toolId: externalTool.id,
-					})
-				);
+				expect(externalToolPseudonymRepo.createOrUpdate).toHaveBeenCalledWith(user.id, externalTool.id);
 			});
 
 			it('should return the pseudonym', async () => {

apps/server/src/modules/pseudonym/service/pseudonym.service.ts

@@ -1,11 +1,9 @@
 import { Logger } from '@core/logger';
-import { ObjectId } from '@mikro-orm/mongodb';
 import { ExternalTool } from '@modules/tool/external-tool/domain';
 import { UserDo } from '@modules/user';
 import { Inject, Injectable, InternalServerErrorException } from '@nestjs/common';
 import { Page } from '@shared/domain/domainobject';
 import { IFindOptions } from '@shared/domain/interface';
-import { v4 as uuidv4 } from 'uuid';
 import { PseudonymSearchQuery } from '../domain';
 import { PSEUDONYM_CONFIG_TOKEN, PseudonymConfig } from '../pseudonym.config';
 import { ExternalToolPseudonymRepo, Pseudonym } from '../repo';
@@ -45,16 +43,7 @@ export class PseudonymService {
 			throw new InternalServerErrorException('User or tool id is missing');
 		}
 
-		const pseudonym = new Pseudonym({
-			id: new ObjectId().toHexString(),
-			pseudonym: uuidv4(),
-			userId: user.id,
-			toolId: tool.id,
-			createdAt: new Date(),
-			updatedAt: new Date(),
-		});
-
-		const result = await this.externalToolPseudonymRepo.createOrUpdate(pseudonym);
+		const result = await this.externalToolPseudonymRepo.createOrUpdate(user.id, tool.id);
 
 		return result;
 	}