Skip to content

Commit

Permalink
exclude migrated users from auto match
Browse files Browse the repository at this point in the history
  • Loading branch information
@MarvinOehlerkingCap
MarvinOehlerkingCap committed Aug 19, 2024
1 parent 5f5cd9a commit 69a0ca0
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 98 deletions.
1 change: 1 addition & 0 deletions apps/server/src/modules/user-import/loggable/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
28 changes: 15 additions & 13 deletions apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
});
Expand All @@ -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);
});
Expand Down
5 changes: 2 additions & 3 deletions apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -38,11 +37,11 @@ export class SchulconnexFetchImportUsersService {
return mappedImportUsers;
}

public async filterAlreadyMigratedUser(importUsers: ImportUser[], systemId: EntityId): Promise<ImportUser[]> {
public async filterAlreadyMigratedUser(importUsers: ImportUser[], system: System): Promise<ImportUser[]> {
const filteredUsers: ImportUser[] = (
await Promise.all(
importUsers.map(async (importUser: ImportUser): Promise<ImportUser | null> => {
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;
})
)
Expand Down
97 changes: 71 additions & 26 deletions apps/server/src/modules/user-import/service/user-import.service.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<ConfigService>;
let importUserRepo: DeepMocked<ImportUserRepo>;
let systemService: DeepMocked<SystemService>;
let userService: DeepMocked<UserService>;
let logger: DeepMocked<Logger>;
let schoolService: DeepMocked<LegacySchoolService>;

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();
Expand All @@ -52,9 +47,7 @@ describe(UserImportService.name, () => {
UserImportService,
{
provide: ConfigService,
useValue: {
get: jest.fn().mockImplementation((key: keyof UserImportConfig) => config[key]),
},
useValue: createMock<ConfigService>(),
},
{
provide: ImportUserRepo,
Expand All @@ -80,20 +73,31 @@ 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);
logger = module.get(Logger);
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', () => {
Expand Down Expand Up @@ -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({
Expand All @@ -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 },
Expand All @@ -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,
Expand All @@ -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]);
});
Expand All @@ -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({
Expand All @@ -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]);
});
Expand All @@ -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,
Expand All @@ -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());
});
Expand Down
14 changes: 9 additions & 5 deletions apps/server/src/modules/user-import/service/user-import.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -44,7 +44,7 @@ export class UserImportService {
}
}

public async matchUsers(importUsers: ImportUser[]): Promise<ImportUser[]> {
public async matchUsers(importUsers: ImportUser[], userLoginMigration: UserLoginMigrationDO): Promise<ImportUser[]> {
const importUserMap: Map<string, number> = new Map();

importUsers.forEach((importUser) => {
Expand All @@ -55,16 +55,20 @@ export class UserImportService {

const matchedImportUsers: ImportUser[] = await Promise.all(
importUsers.map(async (importUser: ImportUser): Promise<ImportUser> => {
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;
}

Expand Down
Loading

0 comments on commit 69a0ca0

Please sign in to comment.