some fixes after review

hpi-schul-cloud · Jan 31, 2024 · 5fecb04 · 5fecb04
1 parent e490546
commit 5fecb04
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 42 deletions.
diff --git a/apps/server/src/modules/deletion/uc/deletion-request.uc.ts b/apps/server/src/modules/deletion/uc/deletion-request.uc.ts
@@ -376,14 +376,14 @@ export class DeletionRequestUc {
 
 	private async removeUsersDataFromNews(deletionRequest: DeletionRequest) {
 		this.logger.debug({ action: 'removeUsersDataFromNews', deletionRequest });
-		const newsesModified = await this.newsService.deleteCreatorOrUpdaterReference(deletionRequest.targetRefId);
+		const newsModified = await this.newsService.deleteCreatorOrUpdaterReference(deletionRequest.targetRefId);
 
 		await this.logDeletion(
 			deletionRequest,
-			newsesModified.domain,
-			newsesModified.operation,
-			newsesModified.count,
-			newsesModified.refs
+			newsModified.domain,
+			newsModified.operation,
+			newsModified.count,
+			newsModified.refs
 		);
 	}
 }
diff --git a/apps/server/src/modules/news/service/news.service.spec.ts b/apps/server/src/modules/news/service/news.service.spec.ts
@@ -45,25 +45,27 @@ describe(NewsService.name, () => {
 
 	describe('deleteCreatorReference', () => {
 		const setup = () => {
-			const user = userFactory.build();
+			const user1 = userFactory.build();
 			const user2 = userFactory.build();
 			const anotherUserId = new ObjectId().toHexString();
 
-			const news1 = teamNewsFactory.build({
-				creator: user,
-				updater: user,
+			const news1 = teamNewsFactory.buildWithId({
+				creator: user1,
 			});
-			const news2 = teamNewsFactory.build({
-				creator: user,
+			const news2 = teamNewsFactory.buildWithId({
 				updater: user2,
 			});
-			const news3 = teamNewsFactory.build({
-				creator: user2,
-				updater: user,
+			const news3 = teamNewsFactory.buildWithId({
+				creator: user1,
+				updater: user2,
 			});
 
-			const expectedResultWithUpdatedNews = DomainOperationBuilder.build(DomainName.NEWS, OperationType.UPDATE, 3, [
+			const expectedResultWithDeletedCreator = DomainOperationBuilder.build(DomainName.NEWS, OperationType.UPDATE, 2, [
 				news1.id,
+				news3.id,
+			]);
+
+			const expectedResultWithDeletedUpdater = DomainOperationBuilder.build(DomainName.NEWS, OperationType.UPDATE, 2, [
 				news2.id,
 				news3.id,
 			]);
@@ -77,36 +79,73 @@ describe(NewsService.name, () => {
 
 			return {
 				anotherUserId,
-				expectedResultWithUpdatedNews,
+				expectedResultWithDeletedCreator,
+				expectedResultWithDeletedUpdater,
 				expectedResultWithoutUpdatedNews,
-				user,
+				user1,
+				user2,
 				news1,
 				news2,
 				news3,
 			};
 		};
-		it('should successfully delete creator or updater reference from news', async () => {
-			const { expectedResultWithUpdatedNews, user, news1, news2, news3 } = setup();
 
-			repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[news1, news2, news3], 3]);
+		describe('when user is creator of news', () => {
+			it('it should be removed from news', async () => {
+				const { user1, news1, news3 } = setup();
+
+				repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[news1, news3], 2]);
+
+				await service.deleteCreatorOrUpdaterReference(user1.id);
 
-			const result = await service.deleteCreatorOrUpdaterReference(user.id);
+				expect(news1.creator).toBeUndefined();
+				expect(news3.creator).toBeUndefined();
+			});
+
+			it('it should return response for 2 news updated', async () => {
+				const { expectedResultWithDeletedCreator, user1, news1, news3 } = setup();
+
+				repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[news1, news3], 2]);
 
-			expect(news1.creator).toBeUndefined();
-			expect(news1.updater).toBeUndefined();
-			expect(news2.creator).toBeUndefined();
-			expect(news3.updater).toBeUndefined();
-			expect(result).toEqual(expectedResultWithUpdatedNews);
+				const result = await service.deleteCreatorOrUpdaterReference(user1.id);
+
+				expect(result).toEqual(expectedResultWithDeletedCreator);
+			});
 		});
 
-		it('should return 0 if news not found', async () => {
-			const { anotherUserId, expectedResultWithoutUpdatedNews } = setup();
+		describe('when user is updater of news', () => {
+			it('user should be removed from updater', async () => {
+				const { user2, news2, news3 } = setup();
 
-			repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[], 0]);
+				repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[news2, news3], 2]);
 
-			const result = await service.deleteCreatorOrUpdaterReference(anotherUserId);
+				await service.deleteCreatorOrUpdaterReference(user2.id);
 
-			expect(result).toEqual(expectedResultWithoutUpdatedNews);
+				expect(news2.updater).toBeUndefined();
+				expect(news3.updater).toBeUndefined();
+			});
+
+			it('it should return response for 2 news updated', async () => {
+				const { expectedResultWithDeletedUpdater, user2, news2, news3 } = setup();
+
+				repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[news2, news3], 2]);
+
+				const result = await service.deleteCreatorOrUpdaterReference(user2.id);
+
+				expect(result).toEqual(expectedResultWithDeletedUpdater);
+			});
+		});
+
+		describe('when user is neither creator nor updater', () => {
+			it('should return response with 0 updated news', async () => {
+				const { anotherUserId, expectedResultWithoutUpdatedNews } = setup();
+
+				repo.findByCreatorOrUpdaterId.mockResolvedValueOnce([[], 0]);
+
+				const result = await service.deleteCreatorOrUpdaterReference(anotherUserId);
+
+				expect(result).toEqual(expectedResultWithoutUpdatedNews);
+			});
 		});
 	});
 });
diff --git a/apps/server/src/shared/repo/news/news.repo.integration.spec.ts b/apps/server/src/shared/repo/news/news.repo.integration.spec.ts
@@ -307,29 +307,42 @@ describe('NewsRepo', () => {
 
 	describe('findByCreatorOrUpdaterId', () => {
 		const setup = async () => {
-			const user = userFactory.buildWithId();
-			const news = teamNewsFactory.build({
-				creator: user,
+			const user1 = userFactory.buildWithId();
+			const user2 = userFactory.buildWithId();
+			const news1 = teamNewsFactory.build({
+				creator: user1,
 			});
 			const news2 = teamNewsFactory.build({
-				updater: user,
+				updater: user2,
+			});
+			const news3 = teamNewsFactory.build({
+				updater: user1,
 			});
 
-			await em.persistAndFlush([news, news2]);
+			await em.persistAndFlush([news1, news2, news3]);
 			em.clear();
 
-			return { news, news2, user };
+			return { news1, news2, news3, user1, user2 };
 		};
-		it('should find a news entity by creatorId', async () => {
-			const { news, user, news2 } = await setup();
+		it('should find a news entity by creatorId and updaterId', async () => {
+			const { news1, user1, news3 } = await setup();
 
-			const result = await repo.findByCreatorOrUpdaterId(user.id);
+			const result = await repo.findByCreatorOrUpdaterId(user1.id);
 			expect(result).toBeDefined();
-			expect(result[0][0].id).toEqual(news.id);
-			expect(result[0][1].id).toEqual(news2.id);
+			expect(result[0][0].id).toEqual(news1.id);
+			expect(result[0][1].id).toEqual(news3.id);
 			expect(result[0].length).toEqual(2);
 		});
 
+		it('should find a news entity by updaterId', async () => {
+			const { user2, news2 } = await setup();
+
+			const result = await repo.findByCreatorOrUpdaterId(user2.id);
+			expect(result).toBeDefined();
+			expect(result[0][0].id).toEqual(news2.id);
+			expect(result[0].length).toEqual(1);
+		});
+
 		it('should throw an exception if not found', async () => {
 			const failNewsId = new ObjectId().toString();
 			const result = await repo.findByCreatorOrUpdaterId(failNewsId);