Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N21-2147 Exclude migrated users from migration wizard auto match #5190

Merged
merged 4 commits into from
Aug 23, 2024
Merged

N21-2147 Exclude migrated users from migration wizard auto match #5190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69a0ca0
exclude migrated users from auto match
MarvinOehlerkingCap Aug 19, 2024
4750449
fix test
MarvinOehlerkingCap Aug 19, 2024
7ade13b
Merge branch 'main' into N21-2147-fix-auto-matching
IgorCapCoder Aug 22, 2024
0036ad5
Merge branch 'main' into N21-2147-fix-auto-matching
MarvinOehlerkingCap Aug 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions apps/server/src/modules/user-import/controller/api-test/import-user-populate.api.spec.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading