Skip to content

Commit

Permalink
N21-2147 Exclude migrated users from migration wizard auto match (#5190)
Browse files Browse the repository at this point in the history
  • Loading branch information
@MarvinOehlerkingCap
MarvinOehlerkingCap authored Aug 23, 2024
1 parent 7c42c84 commit aae9a5b
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 101 deletions.
20 changes: 17 additions & 3 deletions apps/server/src/modules/user-import/controller/api-test/import-user-populate.api.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 };
};
Expand Down Expand Up @@ -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<OauthTokenResponse>(HttpStatus.OK, {
id_token: 'idToken',
refresh_token: 'refreshToken',
Expand Down
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 @@ -79,8 +79,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 @@ -143,26 +143,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 @@ -171,25 +172,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
Loading

0 comments on commit aae9a5b

Please sign in to comment.