N21-1990 Add preferred name option to migration wizard

apps/server/src/infra/schulconnex-client/response/schulconnex-name-response.ts

@@ -1,9 +1,13 @@
-import { IsString } from 'class-validator';
+import { IsOptional, IsString } from 'class-validator';
 
 export class SchulconnexNameResponse {
 	@IsString()
-	familienname!: string;
+	public familienname!: string;
 
 	@IsString()
-	vorname!: string;
+	public vorname!: string;
+
+	@IsOptional()
+	@IsString()
+	public rufname?: string;
 }

apps/server/src/infra/schulconnex-client/testing/schulconnex-response-factory.ts

@@ -8,6 +8,7 @@ export const schulconnexResponseFactory = Factory.define<SchulconnexResponse>(()
 		person: {
 			name: {
 				vorname: 'Hans',
+				rufname: 'Hansi',
 				familienname: 'Peter',
 			},
 			geburt: {

apps/server/src/modules/provisioning/dto/external-user.dto.ts

@@ -5,6 +5,8 @@ export class ExternalUserDto {
 
 	public firstName?: string;
 
+	public preferredName?: string;
+
 	public lastName?: string;
 
 	public email?: string;
@@ -16,6 +18,7 @@ export class ExternalUserDto {
 	constructor(props: ExternalUserDto) {
 		this.externalId = props.externalId;
 		this.firstName = props.firstName;
+		this.preferredName = props.preferredName;
 		this.lastName = props.lastName;
 		this.email = props.email;
 		this.roles = props.roles;

apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts

@@ -101,6 +101,7 @@ describe(SchulconnexResponseMapper.name, () => {
 				expect(result).toEqual<ExternalUserDto>({
 					externalId: externalUserId,
 					firstName: 'Hans',
+					preferredName: 'Hansi',
 					lastName: 'Peter',
 					email: '[email protected]',
 					roles: [RoleName.ADMINISTRATOR],

apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts

@@ -89,6 +89,7 @@ export class SchulconnexResponseMapper {
 
 		const mapped = new ExternalUserDto({
 			firstName: source.person.name.vorname,
+			preferredName: source.person.name.rufname,
 			lastName: source.person.name.familienname,
 			roles: role ? [role] : [],
 			externalId: source.pid,

apps/server/src/modules/provisioning/strategy/schulconnex/service/schulconnex-user-provisioning.service.spec.ts

@@ -70,6 +70,7 @@ describe(SchulconnexUserProvisioningService.name, () => {
 			const existingUser: UserDO = userDoFactory.withRoles([{ id: 'existingRoleId', name: RoleName.USER }]).buildWithId(
 				{
 					firstName: 'existingFirstName',
+					preferredName: 'existingPreferredName',
 					lastName: 'existingLastName',
 					email: 'existingEmail',
 					schoolId: 'existingSchoolId',
@@ -81,6 +82,7 @@ describe(SchulconnexUserProvisioningService.name, () => {
 			const savedUser: UserDO = userDoFactory.withRoles([{ id: 'roleId', name: RoleName.USER }]).buildWithId(
 				{
 					firstName: 'firstName',
+					preferredName: 'preferredName',
 					lastName: 'lastName',
 					email: 'email',
 					schoolId,
@@ -92,6 +94,7 @@ describe(SchulconnexUserProvisioningService.name, () => {
 			const externalUser: ExternalUserDto = externalUserDtoFactory.build({
 				externalId: 'externalUserId',
 				firstName: 'firstName',
+				preferredName: 'preferredName',
 				lastName: 'lastName',
 				email: 'email',
 				roles: [RoleName.USER],

apps/server/src/modules/provisioning/strategy/schulconnex/service/schulconnex-user-provisioning.service.ts

@@ -7,8 +7,7 @@ import { RoleName } from '@shared/domain/interface';
 import { EntityId } from '@shared/domain/types';
 import CryptoJS from 'crypto-js';
 import { ExternalUserDto } from '../../../dto';
-import { UserRoleUnknownLoggableException } from '../../../loggable';
-import { SchoolMissingLoggableException } from '../../../loggable/school-missing.loggable-exception';
+import { SchoolMissingLoggableException, UserRoleUnknownLoggableException } from '../../../loggable';
 
 @Injectable()
 export class SchulconnexUserProvisioningService {
@@ -78,6 +77,7 @@ export class SchulconnexUserProvisioningService {
 	): UserDO {
 		const user: UserDO = foundUser;
 		user.firstName = externalUser.firstName ?? foundUser.firstName;
+		user.preferredName = externalUser.preferredName ?? foundUser.preferredName;
 		user.lastName = externalUser.lastName ?? foundUser.lastName;
 		user.email = externalUser.email ?? foundUser.email;
 		user.roles = roleRefs ?? foundUser.roles;
@@ -91,6 +91,7 @@ export class SchulconnexUserProvisioningService {
 		const user: UserDO = new UserDO({
 			externalId: externalUser.externalId,
 			firstName: externalUser.firstName ?? '',
+			preferredName: externalUser.preferredName,
 			lastName: externalUser.lastName ?? '',
 			email: externalUser.email ?? '',
 			roles: roleRefs ?? [],

apps/server/src/modules/user-import/controller/dto/populate-import-user.params.ts

@@ -0,0 +1,12 @@
+import { ApiPropertyOptional } from '@nestjs/swagger';
+import { IsBoolean, IsOptional } from 'class-validator';
+
+export class PopulateImportUserParams {
+	@IsOptional()
+	@IsBoolean()
+	@ApiPropertyOptional({
+		description:
+			'Should the users preferred name from the external system be used for auto-matching to existing users?',
+	})
+	public matchByPreferredName?: boolean;
+}

apps/server/src/modules/user-import/controller/import-user.controller.ts

@@ -28,6 +28,7 @@ import {
 	UpdateMatchParams,
 	UserMatchListResponse,
 } from './dto';
+import { PopulateImportUserParams } from './dto/populate-import-user.params';
 
 @ApiTags('UserImport')
 @JwtAuthentication()
@@ -135,8 +136,11 @@ export class ImportUserController {
 	@ApiServiceUnavailableResponse()
 	@ApiBadRequestResponse()
 	@ApiForbiddenResponse()
-	async populateImportUsers(@CurrentUser() currentUser: ICurrentUser): Promise<void> {
-		await this.userImportFetchUc.populateImportUsers(currentUser.userId);
+	async populateImportUsers(
+		@CurrentUser() currentUser: ICurrentUser,
+		@Query() query: PopulateImportUserParams
+	): Promise<void> {
+		await this.userImportFetchUc.populateImportUsers(currentUser.userId, query.matchByPreferredName);
 	}
 
 	@Post('cancel')

apps/server/src/modules/user-import/entity/import-user.entity.ts

@@ -16,6 +16,7 @@ export interface ImportUserProperties {
 	externalId: string;
 	// descriptive properties
 	firstName: string;
+	preferredName?: string;
 	lastName: string;
 	email: string; // TODO VO
 	roleNames?: ImportUserRoleName[];
@@ -43,6 +44,7 @@ export class ImportUser extends BaseEntityWithTimestamps implements EntityWithSc
 		this.ldapDn = props.ldapDn;
 		this.externalId = props.externalId;
 		this.firstName = props.firstName;
+		this.preferredName = props.preferredName;
 		this.lastName = props.lastName;
 		this.email = props.email;
 		if (Array.isArray(props.roleNames) && props.roleNames.length > 0) this.roleNames.push(...props.roleNames);
@@ -81,6 +83,9 @@ export class ImportUser extends BaseEntityWithTimestamps implements EntityWithSc
 	@Property()
 	firstName: string;
 
+	@Property({ nullable: true })
+	preferredName?: string;
+
 	@Property()
 	lastName: string;
 

apps/server/src/modules/user-import/mapper/schulconnex-import-user.mapper.ts

@@ -27,6 +27,7 @@ export class SchulconnexImportUserMapper {
 				ldapDn: `uid=${externalUser.person.name.vorname}.${externalUser.person.name.familienname}.${externalUser.pid},`,
 				externalId: externalUser.pid,
 				firstName: externalUser.person.name.vorname,
+				preferredName: externalUser.person.name.rufname,
 				lastName: externalUser.person.name.familienname,
 				roleNames: ImportUser.isImportUserRole(role) ? [role] : [],
 				email: `${externalUser.person.name.vorname}.${externalUser.person.name.familienname}.${externalUser.pid}@schul-cloud.org`,

apps/server/src/modules/user-import/service/schulconnex-fetch-import-users.service.spec.ts

@@ -70,6 +70,7 @@ describe(SchulconnexFetchImportUsersService.name, () => {
 			ldapDn: `uid=${externalUserData.person.name.vorname}.${externalUserData.person.name.familienname}.${externalUserData.pid},`,
 			externalId: externalUserData.pid,
 			firstName: externalUserData.person.name.vorname,
+			preferredName: externalUserData.person.name.rufname,
 			lastName: externalUserData.person.name.familienname,
 			email: `${externalUserData.person.name.vorname}.${externalUserData.person.name.familienname}.${externalUserData.pid}@schul-cloud.org`,
 			roleNames: [RoleName.ADMINISTRATOR],

apps/server/src/modules/user-import/service/user-import.service.spec.ts

@@ -247,7 +247,48 @@ describe(UserImportService.name, () => {
 				it('should return all users as auto matched', async () => {
 					const { user1, user2, importUser1, importUser2, userLoginMigration } = setup();
 
-					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration);
+					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration, false);
+
+					expect(result).toEqual([
+						{ ...importUser1, user: user1, matchedBy: MatchCreator.AUTO },
+						{ ...importUser2, user: user2, matchedBy: MatchCreator.AUTO },
+					]);
+				});
+			});
+
+			describe('when preferred names are used and 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({
+						school,
+						preferredName: user1.preferredName,
+						lastName: user1.lastName,
+					});
+					const importUser2: ImportUser = importUserFactory.buildWithId({
+						school,
+						firstName: user2.firstName,
+						lastName: user2.lastName,
+					});
+
+					userService.findUserBySchoolAndName.mockResolvedValueOnce([user1]);
+					userService.findUserBySchoolAndName.mockResolvedValueOnce([user2]);
+
+					return {
+						user1,
+						user2,
+						importUser1,
+						importUser2,
+						userLoginMigration,
+					};
+				};
+
+				it('should return users with preferred name matched by preferred name and users without matched by first name', async () => {
+					const { user1, user2, importUser1, importUser2, userLoginMigration } = setup();
+
+					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration, true);
 
 					expect(result).toEqual([
 						{ ...importUser1, user: user1, matchedBy: MatchCreator.AUTO },
@@ -286,7 +327,7 @@ describe(UserImportService.name, () => {
 				it('should return the users without a match', async () => {
 					const { importUser1, importUser2, userLoginMigration } = setup();
 
-					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration);
+					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration, false);
 
 					expect(result).toEqual([importUser1, importUser2]);
 				});
@@ -318,7 +359,7 @@ describe(UserImportService.name, () => {
 				it('should return the users without a match', async () => {
 					const { importUser1, userLoginMigration } = setup();
 
-					const result: ImportUser[] = await service.matchUsers([importUser1], userLoginMigration);
+					const result: ImportUser[] = await service.matchUsers([importUser1], userLoginMigration, false);
 
 					expect(result).toEqual([importUser1]);
 				});
@@ -354,7 +395,7 @@ describe(UserImportService.name, () => {
 				it('should return the users without a match', async () => {
 					const { importUser1, importUser2, userLoginMigration } = setup();
 
-					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration);
+					const result: ImportUser[] = await service.matchUsers([importUser1, importUser2], userLoginMigration, false);
 
 					result.forEach((importUser) => expect(importUser.matchedBy).toBeUndefined());
 				});
@@ -387,7 +428,7 @@ describe(UserImportService.name, () => {
 				it('should return the user without a match', async () => {
 					const { importUser1, userLoginMigration } = setup();
 
-					const result: ImportUser[] = await service.matchUsers([importUser1], userLoginMigration);
+					const result: ImportUser[] = await service.matchUsers([importUser1], userLoginMigration, false);
 
 					result.forEach((importUser) => expect(importUser.matchedBy).toBeUndefined());
 				});

apps/server/src/modules/user-import/service/user-import.service.ts

@@ -7,10 +7,10 @@ import { LegacySchoolDo, UserLoginMigrationDO } from '@shared/domain/domainobjec
 import { SchoolEntity, User } from '@shared/domain/entity';
 import { SchoolFeature } from '@shared/domain/types';
 import { Logger } from '@src/core/logger';
+import { ImportUser, MatchCreator } from '../entity';
 import { UserMigrationCanceledLoggable, UserMigrationIsNotEnabled } from '../loggable';
-import { UserImportConfig } from '../user-import-config';
 import { ImportUserRepo } from '../repo/import-user.repo';
-import { ImportUser, MatchCreator } from '../entity';
+import { UserImportConfig } from '../user-import-config';
 
 @Injectable()
 export class UserImportService {
@@ -45,28 +45,34 @@ export class UserImportService {
 		}
 	}
 
-	public async matchUsers(importUsers: ImportUser[], userLoginMigration: UserLoginMigrationDO): Promise<ImportUser[]> {
+	public async matchUsers(
+		importUsers: ImportUser[],
+		userLoginMigration: UserLoginMigrationDO,
+		matchByPreferredName: boolean
+	): Promise<ImportUser[]> {
 		const importUserMap: Map<string, number> = new Map();
 
-		importUsers.forEach((importUser) => {
-			const key = `${importUser.school.id}_${importUser.firstName}_${importUser.lastName}`;
+		importUsers.forEach((importUser: ImportUser): void => {
+			const firstName: string = this.getFirstNameForMatching(importUser, matchByPreferredName);
+			const key = `${importUser.school.id}_${firstName}_${importUser.lastName}`;
 			const count = importUserMap.get(key) || 0;
 			importUserMap.set(key, count + 1);
 		});
 
 		const matchedImportUsers: ImportUser[] = await Promise.all(
 			importUsers.map(async (importUser: ImportUser): Promise<ImportUser> => {
+				const firstName: string = this.getFirstNameForMatching(importUser, matchByPreferredName);
 				const users: User[] = await this.userService.findUserBySchoolAndName(
 					importUser.school.id,
-					importUser.firstName,
+					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}`;
+				const key = `${importUser.school.id}_${firstName}_${importUser.lastName}`;
 
 				if (users.length === 1 && unmigratedUsers.length === 1 && importUserMap.get(key) === 1) {
 					importUser.user = unmigratedUsers[0];
@@ -80,6 +86,10 @@ export class UserImportService {
 		return matchedImportUsers;
 	}
 
+	private getFirstNameForMatching(importUser: ImportUser, matchByPreferredName: boolean): string {
+		return matchByPreferredName && importUser.preferredName ? importUser.preferredName : importUser.firstName;
+	}
+
 	public async deleteImportUsersBySchool(school: SchoolEntity): Promise<void> {
 		await this.userImportRepo.deleteImportUsersBySchool(school);
 	}

apps/server/src/modules/user-import/uc/user-import-fetch.uc.spec.ts

@@ -147,7 +147,7 @@ describe(UserImportFetchUc.name, () => {
 
 				await uc.populateImportUsers(user.id);
 
-				expect(userImportService.matchUsers).toHaveBeenCalledWith([importUser], userLoginMigration);
+				expect(userImportService.matchUsers).toHaveBeenCalledWith([importUser], userLoginMigration, false);
 			});
 
 			it('should delete all existing imported users of the school', async () => {
@@ -166,6 +166,43 @@ describe(UserImportFetchUc.name, () => {
 				expect(userImportService.saveImportUsers).toHaveBeenCalledWith([importUser]);
 			});
 		});
+
+		describe('when matching users by preferred name', () => {
+			const setup = () => {
+				const systemEntity: SystemEntity = systemEntityFactory.buildWithId();
+				const system: System = systemFactory.build({ id: systemEntity.id });
+				const user: User = userFactory.buildWithId();
+				const importUser: ImportUser = importUserFactory.build({
+					system: systemEntity,
+				});
+				const userLoginMigration: UserLoginMigrationDO = userLoginMigrationDOFactory.build({
+					targetSystemId: system.id,
+				});
+
+				authorizationService.getUserWithPermissions.mockResolvedValueOnce(user);
+				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,
+				};
+			};
+
+			it('should match the users by preferred name', async () => {
+				const { user, importUser, userLoginMigration } = setup();
+
+				await uc.populateImportUsers(user.id, true);
+
+				expect(userImportService.matchUsers).toHaveBeenCalledWith([importUser], userLoginMigration, true);
+			});
+		});
 	});
 
 	describe('when the school has not started the migration', () => {