From 69a0ca0d0a6f2379e06a41ba521a567aae98a764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Mon, 19 Aug 2024 10:36:59 +0200 Subject: [PATCH 1/2] exclude migrated users from auto match --- .../src/modules/user-import/loggable/index.ts | 1 + ...lconnex-fetch-import-users.service.spec.ts | 28 +++-- .../schulconnex-fetch-import-users.service.ts | 5 +- .../service/user-import.service.spec.ts | 97 +++++++++++---- .../service/user-import.service.ts | 14 ++- .../uc/user-import-fetch.uc.spec.ts | 115 ++++++++++++------ .../user-import/uc/user-import-fetch.uc.ts | 40 +++--- 7 files changed, 202 insertions(+), 98 deletions(-) diff --git a/apps/server/src/modules/user-import/loggable/index.ts b/apps/server/src/modules/user-import/loggable/index.ts index 6b4929ce0b9..5866aa22e61 100644 --- a/apps/server/src/modules/user-import/loggable/index.ts +++ b/apps/server/src/modules/user-import/loggable/index.ts @@ -11,3 +11,4 @@ export { UserMigrationIsNotEnabled } from './user-migration-not-enable.loggable' export { UserMigrationIsNotEnabledLoggableException } from './user-migration-not-enable-loggable-exception'; export { UserMigrationCanceledLoggable } from './user-migration-canceled.loggable'; export { UserAlreadyMigratedLoggable } from './user-already-migrated.loggable'; +export { UserLoginMigrationNotActiveLoggableException } from './user-login-migration-not-active.loggable-exception'; diff --git a/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.spec.ts b/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.spec.ts index 2c9d6e8d848..12c9b355367 100644 --- a/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.spec.ts +++ b/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.spec.ts @@ -78,8 +78,8 @@ describe(SchulconnexFetchImportUsersService.name, () => { describe('when fetching the data', () => { const setup = () => { const externalUserData: SchulconnexResponse = schulconnexResponseFactory.build(); - const system: System = systemFactory.build(); const systemEntity: SystemEntity = systemEntityFactory.buildWithId(); + const system: System = systemFactory.build({ id: systemEntity.id }); const school: SchoolEntity = schoolEntityFactory.buildWithId({ systems: [systemEntity], externalId: 'externalSchoolId', @@ -142,26 +142,27 @@ describe(SchulconnexFetchImportUsersService.name, () => { describe('when the user was not migrated yet', () => { const setup = () => { const externalUserData: SchulconnexResponse = schulconnexResponseFactory.build(); - const system: SystemEntity = systemEntityFactory.buildWithId(); + const systemEntity: SystemEntity = systemEntityFactory.buildWithId(); + const system: System = systemFactory.build({ id: systemEntity.id }); const school: SchoolEntity = schoolEntityFactory.buildWithId({ - systems: [system], + systems: [systemEntity], externalId: 'externalSchoolId', }); - const importUser: ImportUser = createImportUser(externalUserData, school, system); + const importUser: ImportUser = createImportUser(externalUserData, school, systemEntity); const migratedUser: UserDO = userDoFactory.build({ externalId: externalUserData.pid }); userService.findByExternalId.mockResolvedValueOnce(null); return { - systemId: system.id, + system, importUsers: [importUser], migratedUser, }; }; it('should return the import users', async () => { - const { systemId, importUsers } = setup(); + const { system, importUsers } = setup(); - const result: ImportUser[] = await service.filterAlreadyMigratedUser(importUsers, systemId); + const result: ImportUser[] = await service.filterAlreadyMigratedUser(importUsers, system); expect(result).toHaveLength(1); }); @@ -170,25 +171,26 @@ describe(SchulconnexFetchImportUsersService.name, () => { describe('when the user already was migrated', () => { const setup = () => { const externalUserData: SchulconnexResponse = schulconnexResponseFactory.build(); - const system: SystemEntity = systemEntityFactory.buildWithId(); + const systemEntity: SystemEntity = systemEntityFactory.buildWithId(); + const system: System = systemFactory.build({ id: systemEntity.id }); const school: SchoolEntity = schoolEntityFactory.buildWithId({ - systems: [system], + systems: [systemEntity], externalId: 'externalSchoolId', }); - const importUser: ImportUser = createImportUser(externalUserData, school, system); + const importUser: ImportUser = createImportUser(externalUserData, school, systemEntity); const migratedUser: UserDO = userDoFactory.build({ externalId: externalUserData.pid }); userService.findByExternalId.mockResolvedValueOnce(migratedUser); return { - systemId: system.id, + system, importUsers: [importUser], }; }; it('should return an empty array', async () => { - const { systemId, importUsers } = setup(); + const { system, importUsers } = setup(); - const result: ImportUser[] = await service.filterAlreadyMigratedUser(importUsers, systemId); + const result: ImportUser[] = await service.filterAlreadyMigratedUser(importUsers, system); expect(result).toHaveLength(0); }); diff --git a/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.ts b/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.ts index 2a49438a7c3..26c4e0c395f 100644 --- a/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.ts +++ b/apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.ts @@ -5,7 +5,6 @@ import { UserService } from '@modules/user'; import { Injectable } from '@nestjs/common'; import { UserDO } from '@shared/domain/domainobject'; import { ImportUser, SchoolEntity } from '@shared/domain/entity'; -import { EntityId } from '@shared/domain/types'; import { UserImportSchoolExternalIdMissingLoggableException } from '../loggable'; import { SchulconnexImportUserMapper } from '../mapper'; @@ -38,11 +37,11 @@ export class SchulconnexFetchImportUsersService { return mappedImportUsers; } - public async filterAlreadyMigratedUser(importUsers: ImportUser[], systemId: EntityId): Promise { + public async filterAlreadyMigratedUser(importUsers: ImportUser[], system: System): Promise { const filteredUsers: ImportUser[] = ( await Promise.all( importUsers.map(async (importUser: ImportUser): Promise => { - const foundUser: UserDO | null = await this.userService.findByExternalId(importUser.externalId, systemId); + const foundUser: UserDO | null = await this.userService.findByExternalId(importUser.externalId, system.id); return foundUser ? null : importUser; }) ) diff --git a/apps/server/src/modules/user-import/service/user-import.service.spec.ts b/apps/server/src/modules/user-import/service/user-import.service.spec.ts index 87d683f2868..36c46aa92ef 100644 --- a/apps/server/src/modules/user-import/service/user-import.service.spec.ts +++ b/apps/server/src/modules/user-import/service/user-import.service.spec.ts @@ -1,24 +1,24 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { MongoMemoryDatabaseModule } from '@infra/database'; -import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { ObjectId } from '@mikro-orm/mongodb'; import { LegacySchoolService } from '@modules/legacy-school'; import { System, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; import { InternalServerErrorException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; -import { LegacySchoolDo } from '@shared/domain/domainobject'; +import { LegacySchoolDo, UserLoginMigrationDO } from '@shared/domain/domainobject'; import { ImportUser, MatchCreator, SchoolEntity, User } from '@shared/domain/entity'; import { SchoolFeature } from '@shared/domain/types'; import { ImportUserRepo } from '@shared/repo'; import { - cleanupCollections, importUserFactory, legacySchoolDoFactory, schoolEntityFactory, setupEntities, systemFactory, userFactory, + userLoginMigrationDOFactory, } from '@shared/testing'; import { Logger } from '@src/core/logger'; import { UserMigrationCanceledLoggable, UserMigrationIsNotEnabled } from '../loggable'; @@ -28,20 +28,15 @@ import { UserImportService } from './user-import.service'; describe(UserImportService.name, () => { let module: TestingModule; let service: UserImportService; - let em: EntityManager; + let configService: DeepMocked; let importUserRepo: DeepMocked; let systemService: DeepMocked; let userService: DeepMocked; let logger: DeepMocked; let schoolService: DeepMocked; - const config: UserImportConfig = { - FEATURE_USER_MIGRATION_SYSTEM_ID: new ObjectId().toHexString(), - FEATURE_USER_MIGRATION_ENABLED: true, - FEATURE_MIGRATION_WIZARD_WITH_USER_LOGIN_MIGRATION: true, - IMPORTUSER_SAVE_ALL_MATCHES_REQUEST_TIMEOUT_MS: 8000, - }; + let config: UserImportConfig; beforeAll(async () => { await setupEntities(); @@ -52,9 +47,7 @@ describe(UserImportService.name, () => { UserImportService, { provide: ConfigService, - useValue: { - get: jest.fn().mockImplementation((key: keyof UserImportConfig) => config[key]), - }, + useValue: createMock(), }, { provide: ImportUserRepo, @@ -80,7 +73,7 @@ describe(UserImportService.name, () => { }).compile(); service = module.get(UserImportService); - em = module.get(EntityManager); + configService = module.get(ConfigService); importUserRepo = module.get(ImportUserRepo); systemService = module.get(SystemService); userService = module.get(UserService); @@ -88,12 +81,23 @@ describe(UserImportService.name, () => { schoolService = module.get(LegacySchoolService); }); - afterAll(async () => { - await module.close(); + beforeEach(() => { + config = { + FEATURE_USER_MIGRATION_SYSTEM_ID: new ObjectId().toHexString(), + FEATURE_USER_MIGRATION_ENABLED: true, + FEATURE_MIGRATION_WIZARD_WITH_USER_LOGIN_MIGRATION: true, + IMPORTUSER_SAVE_ALL_MATCHES_REQUEST_TIMEOUT_MS: 8000, + }; + + configService.get.mockImplementation((key: keyof UserImportConfig) => config[key]); }); - beforeEach(async () => { - await cleanupCollections(em); + afterEach(() => { + jest.resetAllMocks(); + }); + + afterAll(async () => { + await module.close(); }); describe('saveImportUsers', () => { @@ -213,6 +217,7 @@ describe(UserImportService.name, () => { describe('when all users have unique names', () => { const setup = () => { const school: SchoolEntity = schoolEntityFactory.buildWithId(); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ schoolId: school.id }); const user1: User = userFactory.buildWithId({ firstName: 'First1', lastName: 'Last1' }); const user2: User = userFactory.buildWithId({ firstName: 'First2', lastName: 'Last2' }); const importUser1: ImportUser = importUserFactory.buildWithId({ @@ -234,13 +239,14 @@ describe(UserImportService.name, () => { user2, importUser1, importUser2, + userLoginMigration, }; }; it('should return all users as auto matched', async () => { - const { user1, user2, importUser1, importUser2 } = setup(); + const { user1, user2, importUser1, importUser2, userLoginMigration } = setup(); - const result: ImportUser[] = await service.matchUsers([importUser1, importUser2]); + const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration); expect(result).toEqual([ { ...importUser1, user: user1, matchedBy: MatchCreator.AUTO }, @@ -252,6 +258,7 @@ describe(UserImportService.name, () => { describe('when the imported users have the same names', () => { const setup = () => { const school: SchoolEntity = schoolEntityFactory.buildWithId(); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ schoolId: school.id }); const user1: User = userFactory.buildWithId({ firstName: 'First', lastName: 'Last' }); const importUser1: ImportUser = importUserFactory.buildWithId({ school, @@ -271,13 +278,14 @@ describe(UserImportService.name, () => { user1, importUser1, importUser2, + userLoginMigration, }; }; it('should return the users without a match', async () => { - const { importUser1, importUser2 } = setup(); + const { importUser1, importUser2, userLoginMigration } = setup(); - const result: ImportUser[] = await service.matchUsers([importUser1, importUser2]); + const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration); expect(result).toEqual([importUser1, importUser2]); }); @@ -286,6 +294,7 @@ describe(UserImportService.name, () => { describe('when existing users in svs have the same names', () => { const setup = () => { const school: SchoolEntity = schoolEntityFactory.buildWithId(); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ schoolId: school.id }); const user1: User = userFactory.buildWithId({ firstName: 'First', lastName: 'Last' }); const user2: User = userFactory.buildWithId({ firstName: 'First', lastName: 'Last' }); const importUser1: ImportUser = importUserFactory.buildWithId({ @@ -301,13 +310,14 @@ describe(UserImportService.name, () => { user1, user2, importUser1, + userLoginMigration, }; }; it('should return the users without a match', async () => { - const { importUser1 } = setup(); + const { importUser1, userLoginMigration } = setup(); - const result: ImportUser[] = await service.matchUsers([importUser1]); + const result: ImportUser[] = await service.matchUsers([importUser1], userLoginMigration); expect(result).toEqual([importUser1]); }); @@ -316,6 +326,7 @@ describe(UserImportService.name, () => { describe('when import users have the same name ', () => { const setup = () => { const school: SchoolEntity = schoolEntityFactory.buildWithId(); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ schoolId: school.id }); const user1: User = userFactory.buildWithId({ firstName: 'First', lastName: 'Last' }); const importUser1: ImportUser = importUserFactory.buildWithId({ school, @@ -335,13 +346,47 @@ describe(UserImportService.name, () => { user1, importUser1, importUser2, + userLoginMigration, }; }; it('should return the users without a match', async () => { - const { importUser1, importUser2 } = setup(); + const { importUser1, importUser2, userLoginMigration } = setup(); + + const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration); + + result.forEach((importUser) => expect(importUser.matchedBy).toBeUndefined()); + }); + }); + + describe('when a user is already migarted', () => { + const setup = () => { + const school: SchoolEntity = schoolEntityFactory.buildWithId(); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ schoolId: school.id }); + const user1: User = userFactory.buildWithId({ + firstName: 'First', + lastName: 'Last', + lastLoginSystemChange: userLoginMigration.startedAt, + }); + const importUser1: ImportUser = importUserFactory.buildWithId({ + school, + firstName: user1.firstName, + lastName: user1.lastName, + }); + + userService.findUserBySchoolAndName.mockResolvedValueOnce([user1]); + + return { + user1, + importUser1, + userLoginMigration, + }; + }; + + it('should return the user without a match', async () => { + const { importUser1, userLoginMigration } = setup(); - const result: ImportUser[] = await service.matchUsers([importUser1, importUser2]); + const result: ImportUser[] = await service.matchUsers([importUser1], userLoginMigration); result.forEach((importUser) => expect(importUser.matchedBy).toBeUndefined()); }); diff --git a/apps/server/src/modules/user-import/service/user-import.service.ts b/apps/server/src/modules/user-import/service/user-import.service.ts index db3fbeb5c70..d63fdb29c2d 100644 --- a/apps/server/src/modules/user-import/service/user-import.service.ts +++ b/apps/server/src/modules/user-import/service/user-import.service.ts @@ -3,7 +3,7 @@ import { System, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; import { Injectable, InternalServerErrorException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; -import { LegacySchoolDo } from '@shared/domain/domainobject'; +import { LegacySchoolDo, UserLoginMigrationDO } from '@shared/domain/domainobject'; import { ImportUser, MatchCreator, SchoolEntity, User } from '@shared/domain/entity'; import { SchoolFeature } from '@shared/domain/types'; import { ImportUserRepo } from '@shared/repo'; @@ -44,7 +44,7 @@ export class UserImportService { } } - public async matchUsers(importUsers: ImportUser[]): Promise { + public async matchUsers(importUsers: ImportUser[], userLoginMigration: UserLoginMigrationDO): Promise { const importUserMap: Map = new Map(); importUsers.forEach((importUser) => { @@ -55,16 +55,20 @@ export class UserImportService { const matchedImportUsers: ImportUser[] = await Promise.all( importUsers.map(async (importUser: ImportUser): Promise => { - const user: User[] = await this.userService.findUserBySchoolAndName( + const users: User[] = await this.userService.findUserBySchoolAndName( importUser.school.id, importUser.firstName, importUser.lastName ); + const unmigratedUsers: User[] = users.filter( + (user: User) => !user.lastLoginSystemChange || user.lastLoginSystemChange < userLoginMigration.startedAt + ); + const key = `${importUser.school.id}_${importUser.firstName}_${importUser.lastName}`; - if (user.length === 1 && importUserMap.get(key) === 1) { - importUser.user = user[0]; + if (users.length === 1 && unmigratedUsers.length === 1 && importUserMap.get(key) === 1) { + importUser.user = unmigratedUsers[0]; importUser.matchedBy = MatchCreator.AUTO; } diff --git a/apps/server/src/modules/user-import/uc/user-import-fetch.uc.spec.ts b/apps/server/src/modules/user-import/uc/user-import-fetch.uc.spec.ts index 8394306979f..fb34bc9c7b9 100644 --- a/apps/server/src/modules/user-import/uc/user-import-fetch.uc.spec.ts +++ b/apps/server/src/modules/user-import/uc/user-import-fetch.uc.spec.ts @@ -1,14 +1,23 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { AuthorizationService } from '@modules/authorization'; -import { ConfigService } from '@nestjs/config'; -import { System } from '@modules/system'; +import { System, SystemService } from '@modules/system'; import { SystemEntity } from '@modules/system/entity'; +import { ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; +import { UserLoginMigrationDO } from '@shared/domain/domainobject'; import { ImportUser, User } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; -import { importUserFactory, setupEntities, systemEntityFactory, systemFactory, userFactory } from '@shared/testing'; -import { UserMigrationIsNotEnabledLoggableException } from '../loggable'; +import { + importUserFactory, + setupEntities, + systemEntityFactory, + systemFactory, + userFactory, + userLoginMigrationDOFactory, +} from '@shared/testing'; +import { UserLoginMigrationService } from '../../user-login-migration'; +import { UserLoginMigrationNotActiveLoggableException, UserMigrationIsNotEnabledLoggableException } from '../loggable'; import { SchulconnexFetchImportUsersService, UserImportService } from '../service'; import { UserImportConfig } from '../user-import-config'; import { UserImportFetchUc } from './user-import-fetch.uc'; @@ -17,16 +26,14 @@ describe(UserImportFetchUc.name, () => { let module: TestingModule; let uc: UserImportFetchUc; + let configService: DeepMocked; let schulconnexFetchImportUsersService: DeepMocked; let authorizationService: DeepMocked; let userImportService: DeepMocked; + let userLoginMigrationService: DeepMocked; + let systemService: DeepMocked; - const config: UserImportConfig = { - FEATURE_USER_MIGRATION_ENABLED: true, - FEATURE_USER_MIGRATION_SYSTEM_ID: new ObjectId().toHexString(), - FEATURE_MIGRATION_WIZARD_WITH_USER_LOGIN_MIGRATION: true, - IMPORTUSER_SAVE_ALL_MATCHES_REQUEST_TIMEOUT_MS: 0, - }; + let config: UserImportConfig; beforeAll(async () => { await setupEntities(); @@ -36,9 +43,7 @@ describe(UserImportFetchUc.name, () => { UserImportFetchUc, { provide: ConfigService, - useValue: { - get: jest.fn().mockImplementation((key: keyof UserImportConfig) => config[key]), - }, + useValue: createMock(), }, { provide: SchulconnexFetchImportUsersService, @@ -52,19 +57,35 @@ describe(UserImportFetchUc.name, () => { provide: UserImportService, useValue: createMock(), }, + { + provide: UserLoginMigrationService, + useValue: createMock(), + }, + { + provide: SystemService, + useValue: createMock(), + }, ], }).compile(); uc = module.get(UserImportFetchUc); + configService = module.get(ConfigService); schulconnexFetchImportUsersService = module.get(SchulconnexFetchImportUsersService); authorizationService = module.get(AuthorizationService); userImportService = module.get(UserImportService); + userLoginMigrationService = module.get(UserLoginMigrationService); + systemService = module.get(SystemService); }); beforeEach(() => { - config.FEATURE_USER_MIGRATION_ENABLED = true; - config.FEATURE_USER_MIGRATION_SYSTEM_ID = new ObjectId().toHexString(); - config.FEATURE_MIGRATION_WIZARD_WITH_USER_LOGIN_MIGRATION = true; + config = { + FEATURE_USER_MIGRATION_ENABLED: true, + FEATURE_USER_MIGRATION_SYSTEM_ID: new ObjectId().toHexString(), + FEATURE_MIGRATION_WIZARD_WITH_USER_LOGIN_MIGRATION: true, + IMPORTUSER_SAVE_ALL_MATCHES_REQUEST_TIMEOUT_MS: 0, + }; + + configService.get.mockImplementation((key: keyof UserImportConfig) => config[key]); }); afterAll(async () => { @@ -78,26 +99,29 @@ describe(UserImportFetchUc.name, () => { describe('fetchImportUsers', () => { describe('when fetching and matching users', () => { const setup = () => { - const system: SystemEntity = systemEntityFactory.buildWithId( - undefined, - config.FEATURE_USER_MIGRATION_SYSTEM_ID - ); - const systemDo: System = systemFactory.build({ id: system.id }); + const systemEntity: SystemEntity = systemEntityFactory.buildWithId(); + const system: System = systemFactory.build({ id: systemEntity.id }); const user: User = userFactory.buildWithId(); const importUser: ImportUser = importUserFactory.build({ - system, + system: systemEntity, + }); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ + targetSystemId: system.id, }); authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); - userImportService.getMigrationSystem.mockResolvedValueOnce(systemDo); + userLoginMigrationService.findMigrationBySchool.mockResolvedValueOnce(userLoginMigration); + systemService.findByIdOrFail.mockResolvedValueOnce(system); schulconnexFetchImportUsersService.getData.mockResolvedValueOnce([importUser]); schulconnexFetchImportUsersService.filterAlreadyMigratedUser.mockResolvedValueOnce([importUser]); userImportService.matchUsers.mockResolvedValueOnce([importUser]); return { user, + systemEntity, system, importUser, + userLoginMigration, }; }; @@ -114,18 +138,15 @@ describe(UserImportFetchUc.name, () => { await uc.populateImportUsers(user.id); - expect(schulconnexFetchImportUsersService.filterAlreadyMigratedUser).toHaveBeenCalledWith( - [importUser], - system.id - ); + expect(schulconnexFetchImportUsersService.filterAlreadyMigratedUser).toHaveBeenCalledWith([importUser], system); }); it('should match the users', async () => { - const { user, importUser } = setup(); + const { user, importUser, userLoginMigration } = setup(); await uc.populateImportUsers(user.id); - expect(userImportService.matchUsers).toHaveBeenCalledWith([importUser]); + expect(userImportService.matchUsers).toHaveBeenCalledWith([importUser], userLoginMigration); }); it('should delete all existing imported users of the school', async () => { @@ -146,27 +167,51 @@ describe(UserImportFetchUc.name, () => { }); }); - describe('when the migration feature is not enabled', () => { + describe('when the school has not started the migration', () => { const setup = () => { - config.FEATURE_USER_MIGRATION_ENABLED = false; + const user: User = userFactory.buildWithId(); + + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + userLoginMigrationService.findMigrationBySchool.mockResolvedValueOnce(null); + + return { + user, + }; + }; + + it('should throw error', async () => { + const { user } = setup(); + + await expect(uc.populateImportUsers(user.id)).rejects.toThrow(UserLoginMigrationNotActiveLoggableException); + }); + }); + describe('when the school has already closed the migration', () => { + const setup = () => { const user: User = userFactory.buildWithId(); + const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({ + closedAt: new Date(), + }); + + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + userLoginMigrationService.findMigrationBySchool.mockResolvedValueOnce(userLoginMigration); return { user, + userLoginMigration, }; }; - it('should throw an error', async () => { + it('should throw error', async () => { const { user } = setup(); - await expect(uc.populateImportUsers(user.id)).rejects.toThrow(UserMigrationIsNotEnabledLoggableException); + await expect(uc.populateImportUsers(user.id)).rejects.toThrow(UserLoginMigrationNotActiveLoggableException); }); }); - describe('when the target system id is not defined', () => { + describe('when the migration feature is not enabled', () => { const setup = () => { - config.FEATURE_USER_MIGRATION_SYSTEM_ID = ''; + config.FEATURE_USER_MIGRATION_ENABLED = false; const user: User = userFactory.buildWithId(); diff --git a/apps/server/src/modules/user-import/uc/user-import-fetch.uc.ts b/apps/server/src/modules/user-import/uc/user-import-fetch.uc.ts index 743e942449d..839cf83b56a 100644 --- a/apps/server/src/modules/user-import/uc/user-import-fetch.uc.ts +++ b/apps/server/src/modules/user-import/uc/user-import-fetch.uc.ts @@ -1,11 +1,13 @@ import { AuthorizationService } from '@modules/authorization'; -import { System } from '@modules/system'; +import { System, SystemService } from '@modules/system'; +import { UserLoginMigrationService } from '@modules/user-login-migration'; import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; +import { UserLoginMigrationDO } from '@shared/domain/domainobject'; import { ImportUser, User } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; -import { UserMigrationIsNotEnabledLoggableException } from '../loggable'; +import { UserLoginMigrationNotActiveLoggableException, UserMigrationIsNotEnabledLoggableException } from '../loggable'; import { SchulconnexFetchImportUsersService, UserImportService } from '../service'; import { UserImportConfig } from '../user-import-config'; @@ -15,36 +17,42 @@ export class UserImportFetchUc { private readonly configService: ConfigService, private readonly schulconnexFetchImportUsersService: SchulconnexFetchImportUsersService, private readonly authorizationService: AuthorizationService, - private readonly userImportService: UserImportService + private readonly userImportService: UserImportService, + private readonly userLoginMigrationService: UserLoginMigrationService, + private readonly systemService: SystemService ) {} public async populateImportUsers(currentUserId: EntityId): Promise { - this.checkMigrationEnabled(currentUserId); + if (!this.configService.get('FEATURE_USER_MIGRATION_ENABLED')) { + throw new UserMigrationIsNotEnabledLoggableException(currentUserId); + } const user: User = await this.authorizationService.getUserWithPermissions(currentUserId); this.authorizationService.checkAllPermissions(user, [Permission.IMPORT_USER_MIGRATE]); - const system: System = await this.userImportService.getMigrationSystem(); + const userLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationService.findMigrationBySchool( + user.school.id + ); + + if (!userLoginMigration || userLoginMigration?.closedAt) { + throw new UserLoginMigrationNotActiveLoggableException(user.school.id); + } + + const system: System = await this.systemService.findByIdOrFail(userLoginMigration.targetSystemId); const fetchedData: ImportUser[] = await this.schulconnexFetchImportUsersService.getData(user.school, system); const filteredFetchedData: ImportUser[] = await this.schulconnexFetchImportUsersService.filterAlreadyMigratedUser( fetchedData, - this.configService.get('FEATURE_USER_MIGRATION_SYSTEM_ID') + system ); - const matchedImportUsers: ImportUser[] = await this.userImportService.matchUsers(filteredFetchedData); + const matchedImportUsers: ImportUser[] = await this.userImportService.matchUsers( + filteredFetchedData, + userLoginMigration + ); await this.userImportService.deleteImportUsersBySchool(user.school); await this.userImportService.saveImportUsers(matchedImportUsers); } - - private checkMigrationEnabled(userId: EntityId): void { - if ( - !this.configService.get('FEATURE_USER_MIGRATION_ENABLED') || - !this.configService.get('FEATURE_USER_MIGRATION_SYSTEM_ID') - ) { - throw new UserMigrationIsNotEnabledLoggableException(userId); - } - } } From 4750449a7eed1b19fdee980bab3b3455bc4afa91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Mon, 19 Aug 2024 10:53:49 +0200 Subject: [PATCH 2/2] fix test --- .../api-test/import-user-populate.api.spec.ts | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/user-import/controller/api-test/import-user-populate.api.spec.ts b/apps/server/src/modules/user-import/controller/api-test/import-user-populate.api.spec.ts index 21b9b6b005a..0ff4a8c1320 100644 --- a/apps/server/src/modules/user-import/controller/api-test/import-user-populate.api.spec.ts +++ b/apps/server/src/modules/user-import/controller/api-test/import-user-populate.api.spec.ts @@ -7,7 +7,14 @@ import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Permission, RoleName } from '@shared/domain/interface'; import { SchoolFeature } from '@shared/domain/types'; -import { roleFactory, schoolEntityFactory, systemEntityFactory, TestApiClient, userFactory } from '@shared/testing'; +import { + roleFactory, + schoolEntityFactory, + systemEntityFactory, + TestApiClient, + userFactory, + userLoginMigrationFactory, +} from '@shared/testing'; import { accountFactory } from '@src/modules/account/testing'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; @@ -112,11 +119,15 @@ describe('ImportUser Controller Populate (API)', () => { describe('when users school has no external id', () => { const setup = async () => { const { account, school, system } = await authenticatedUser([Permission.IMPORT_USER_MIGRATE], [], false); - const loggedInClient = await testApiClient.login(account); + school.externalId = undefined; + const config: ServerConfig = serverConfig(); config.FEATURE_USER_MIGRATION_SYSTEM_ID = system.id; - school.externalId = undefined; + const userLoginMigration = userLoginMigrationFactory.buildWithId({ school }); + await em.persistAndFlush([userLoginMigration]); + + const loggedInClient = await testApiClient.login(account); return { loggedInClient }; }; @@ -144,6 +155,9 @@ describe('ImportUser Controller Populate (API)', () => { config.FEATURE_USER_MIGRATION_ENABLED = true; config.FEATURE_USER_MIGRATION_SYSTEM_ID = system.id; + const userLoginMigration = userLoginMigrationFactory.buildWithId({ school }); + await em.persistAndFlush([userLoginMigration]); + axiosMock.onPost(/(.*)\/token/).reply(HttpStatus.OK, { id_token: 'idToken', refresh_token: 'refreshToken',