N21-841 use user login migration (#4356)

* N21-841 fix close migration
hpi-schul-cloud · Sep 26, 2023 · 7e6a281 · 7e6a281
1 parent ddfdae2
commit 7e6a281
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 14 deletions.
diff --git a/...ver/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts b/...ver/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts
@@ -1202,5 +1202,54 @@ describe('UserLoginMigrationController (API)', () => {
 				expect(response.status).toEqual(HttpStatus.FORBIDDEN);
 			});
 		});
+
+		describe('when no user has migrate', () => {
+			const setup = async () => {
+				const sourceSystem: SystemEntity = systemFactory.withLdapConfig().buildWithId({ alias: 'SourceSystem' });
+				const targetSystem: SystemEntity = systemFactory.withOauthConfig().buildWithId({ alias: 'SANIS' });
+				const school: SchoolEntity = schoolFactory.buildWithId({
+					systems: [sourceSystem],
+					officialSchoolNumber: '12345',
+				});
+				const userLoginMigration: UserLoginMigrationEntity = userLoginMigrationFactory.buildWithId({
+					school,
+					targetSystem,
+					sourceSystem,
+					startedAt: new Date(2023, 1, 4),
+				});
+
+				const user: User = userFactory.buildWithId();
+
+				const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({ school }, [
+					Permission.USER_LOGIN_MIGRATION_ADMIN,
+				]);
+
+				await em.persistAndFlush([
+					sourceSystem,
+					targetSystem,
+					school,
+					adminAccount,
+					adminUser,
+					userLoginMigration,
+					user,
+				]);
+				em.clear();
+
+				const loggedInClient = await testApiClient.login(adminAccount);
+
+				return {
+					loggedInClient,
+					userLoginMigration,
+				};
+			};
+
+			it('should return nothing', async () => {
+				const { loggedInClient } = await setup();
+
+				const response: Response = await loggedInClient.post('/close');
+
+				expect(response.body).toEqual({});
+			});
+		});
 	});
 });
diff --git a/apps/server/src/modules/user-login-migration/controller/user-login-migration.controller.ts b/apps/server/src/modules/user-login-migration/controller/user-login-migration.controller.ts
@@ -2,6 +2,7 @@ import { Body, Controller, Get, Param, Post, Put, Query } from '@nestjs/common';
 import {
 	ApiForbiddenResponse,
 	ApiInternalServerErrorResponse,
+	ApiNoContentResponse,
 	ApiNotFoundResponse,
 	ApiOkResponse,
 	ApiOperation,
@@ -196,16 +197,19 @@ export class UserLoginMigrationController {
 	@ApiOkResponse({ description: 'User login migration closed', type: UserLoginMigrationResponse })
 	@ApiUnauthorizedResponse()
 	@ApiForbiddenResponse()
-	async closeMigration(@CurrentUser() currentUser: ICurrentUser): Promise<UserLoginMigrationResponse | void> {
-		const userLoginMigration: UserLoginMigrationDO = await this.closeUserLoginMigrationUc.closeMigration(
+	@ApiNoContentResponse({ description: 'User login migration was reverted' })
+	async closeMigration(@CurrentUser() currentUser: ICurrentUser): Promise<UserLoginMigrationResponse | undefined> {
+		const userLoginMigration: UserLoginMigrationDO | undefined = await this.closeUserLoginMigrationUc.closeMigration(
 			currentUser.userId,
 			currentUser.schoolId
 		);
 
-		const migrationResponse: UserLoginMigrationResponse =
-			UserLoginMigrationMapper.mapUserLoginMigrationDoToResponse(userLoginMigration);
-
-		return migrationResponse;
+		if (userLoginMigration) {
+			const migrationResponse: UserLoginMigrationResponse =
+				UserLoginMigrationMapper.mapUserLoginMigrationDoToResponse(userLoginMigration);
+			return migrationResponse;
+		}
+		return undefined;
 	}
 
 	@Post('migrate-to-oauth2')

diff --git a/apps/server/src/modules/user-login-migration/service/user-login-migration.service.ts b/apps/server/src/modules/user-login-migration/service/user-login-migration.service.ts
@@ -169,7 +169,6 @@ export class UserLoginMigrationService {
 	}
 
 	private async updateExistingMigration(userLoginMigrationDO: UserLoginMigrationDO) {
-		userLoginMigrationDO.startedAt = new Date();
 		userLoginMigrationDO.closedAt = undefined;
 		userLoginMigrationDO.finishedAt = undefined;
 

diff --git a/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.spec.ts b/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.spec.ts
@@ -179,6 +179,22 @@ describe('CloseUserLoginMigrationUc', () => {
 
 				expect(userLoginMigrationRevertService.revertUserLoginMigration).toHaveBeenCalledWith(closedUserLoginMigration);
 			});
+
+			it('should not mark all un-migrated users as outdated', async () => {
+				const { user, schoolId } = setup();
+
+				await uc.closeMigration(user.id, schoolId);
+
+				expect(schoolMigrationService.markUnmigratedUsersAsOutdated).not.toHaveBeenCalled();
+			});
+
+			it('should return  undefined', async () => {
+				const { user, schoolId } = setup();
+
+				const result = await uc.closeMigration(user.id, schoolId);
+
+				expect(result).toBeUndefined();
+			});
 		});
 
 		describe('when the user login migration was already closed', () => {

diff --git a/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.ts b/apps/server/src/modules/user-login-migration/uc/close-user-login-migration.uc.ts
@@ -16,7 +16,7 @@ export class CloseUserLoginMigrationUc {
 		private readonly authorizationService: AuthorizationService
 	) {}
 
-	async closeMigration(userId: EntityId, schoolId: EntityId): Promise<UserLoginMigrationDO> {
+	async closeMigration(userId: EntityId, schoolId: EntityId): Promise<UserLoginMigrationDO | undefined> {
 		const userLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationService.findMigrationBySchool(
 			schoolId
 		);
@@ -47,9 +47,9 @@ export class CloseUserLoginMigrationUc {
 
 			if (!hasSchoolMigratedUser) {
 				await this.userLoginMigrationRevertService.revertUserLoginMigration(updatedUserLoginMigration);
-			} else {
-				await this.schoolMigrationService.markUnmigratedUsersAsOutdated(schoolId);
+				return undefined;
 			}
+			await this.schoolMigrationService.markUnmigratedUsersAsOutdated(schoolId);
 
 			return updatedUserLoginMigration;
 		}

diff --git a/apps/server/src/modules/user-login-migration/uc/restart-user-login-migration.uc.ts b/apps/server/src/modules/user-login-migration/uc/restart-user-login-migration.uc.ts
@@ -39,8 +39,6 @@ export class RestartUserLoginMigrationUc {
 			userLoginMigration = await this.userLoginMigrationService.restartMigration(schoolId);
 
 			this.logger.info(new UserLoginMigrationStartLoggable(userId, schoolId));
-		} else {
-			// Do nothing, if migration is already started but not stopped.
 		}
 
 		return userLoginMigration;

diff --git a/apps/server/src/modules/user-login-migration/uc/start-user-login-migration.uc.ts b/apps/server/src/modules/user-login-migration/uc/start-user-login-migration.uc.ts
@@ -34,8 +34,6 @@ export class StartUserLoginMigrationUc {
 				userLoginMigration.id as string,
 				userLoginMigration.closedAt
 			);
-		} else {
-			// Do nothing, if migration is already started but not stopped.
 		}
 
 		return userLoginMigration;