diff --git a/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts b/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts index 95cb8fdde5e..486c814ccd7 100644 --- a/apps/server/src/modules/user-login-migration/controller/api-test/user-login-migration.api.spec.ts +++ b/apps/server/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 index 7ef6242eb72..19098167d96 100644 --- 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 { - const userLoginMigration: UserLoginMigrationDO = await this.closeUserLoginMigrationUc.closeMigration( + @ApiNoContentResponse({ description: 'User login migration was reverted' }) + async closeMigration(@CurrentUser() currentUser: ICurrentUser): Promise { + 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 index f1037997955..9f3d6c59e84 100644 --- 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 index 1ea3fa50954..6b796e69bba 100644 --- 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 index ad6646edae8..13058e64c49 100644 --- 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 { + async closeMigration(userId: EntityId, schoolId: EntityId): Promise { 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 index 514973db20b..42412e50fe3 100644 --- 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 index 8eefd9a85ee..9e670e7fdf3 100644 --- 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;