BC-8335 - user module refactorings

apps/server/src/modules/idp-console/idp-console.module.ts

@@ -2,6 +2,7 @@ import { ConsoleWriterModule } from '@infra/console';
 import { RabbitMQWrapperModule } from '@infra/rabbitmq';
 import { SchulconnexClientModule } from '@infra/schulconnex-client/schulconnex-client.module';
 import { MikroOrmModule } from '@mikro-orm/nestjs';
+import { AccountModule } from '@modules/account';
 import { defaultMikroOrmOptions } from '@modules/server';
 import { SynchronizationEntity, SynchronizationModule } from '@modules/synchronization';
 import { UserModule } from '@modules/user';
@@ -30,6 +31,7 @@ import { SynchronizationUc } from './uc';
 			// debug: true, // use it for locally debugging of queries
 		}),
 		UserModule,
+		AccountModule,
 		LoggerModule,
 		RabbitMQWrapperModule,
 		ConsoleWriterModule,

apps/server/src/modules/idp-console/uc/synchronization.uc.spec.ts

@@ -13,6 +13,7 @@ import { Test, TestingModule } from '@nestjs/testing';
 import { setupEntities } from '@shared/testing';
 import { createConfigModuleOptions } from '@src/config';
 import { Logger } from '@src/core/logger';
+import { AccountService } from '@src/modules/account';
 import { ObjectId } from 'bson';
 import {
 	FailedUpdateLastSyncedAtLoggableException,
@@ -25,6 +26,7 @@ describe(SynchronizationUc.name, () => {
 	let module: TestingModule;
 	let uc: SynchronizationUc;
 	let userService: DeepMocked<UserService>;
+	let accountService: DeepMocked<AccountService>;
 	let synchronizationService: DeepMocked<SynchronizationService>;
 	let schulconnexRestClient: DeepMocked<SchulconnexRestClient>;
 
@@ -45,6 +47,10 @@ describe(SynchronizationUc.name, () => {
 					provide: UserService,
 					useValue: createMock<UserService>(),
 				},
+				{
+					provide: AccountService,
+					useValue: createMock<AccountService>(),
+				},
 				{
 					provide: SchulconnexRestClient,
 					useValue: createMock<SchulconnexRestClient>(),
@@ -55,6 +61,7 @@ describe(SynchronizationUc.name, () => {
 		uc = module.get(SynchronizationUc);
 		synchronizationService = module.get(SynchronizationService);
 		userService = module.get(UserService);
+		accountService = module.get(AccountService);
 		schulconnexRestClient = module.get(SchulconnexRestClient);
 		await setupEntities();
 	});
@@ -354,26 +361,39 @@ describe(SynchronizationUc.name, () => {
 				const systemId = new ObjectId().toHexString();
 				const userAId = new ObjectId().toHexString();
 				const userBId = new ObjectId().toHexString();
-				const usersToCheck = [userAId, userBId];
+				const userCId = new ObjectId().toHexString();
+				const userDId = new ObjectId().toHexString();
+				const usersToCheck = [userAId, userBId, userCId, userDId];
+				const usersFound = [userAId, userBId, userCId];
 				const usersToSync = [userAId, userBId];
 				const userSyncCount = 2;
 
-				userService.findByExternalIdsAndProvidedBySystemId.mockResolvedValueOnce(usersToSync);
+				userService.findMultipleByExternalIds.mockResolvedValueOnce(usersFound);
+				accountService.findByUserIdsAndSystemId.mockResolvedValueOnce(usersToSync);
 
 				return {
 					systemId,
 					userSyncCount,
 					usersToCheck,
+					usersFound,
 					usersToSync,
 				};
 			};
 
-			it('should call the userService.findByExternalIdsAndProvidedBySystemId to get array of users to sync', async () => {
+			it('should call the userService.findMultipleByExternalIds to get array of users to sync', async () => {
 				const { systemId, usersToCheck } = setup();
 
 				await uc.updateLastSyncedAt(usersToCheck, systemId);
 
-				expect(userService.findByExternalIdsAndProvidedBySystemId).toHaveBeenCalledWith(usersToCheck, systemId);
+				expect(userService.findMultipleByExternalIds).toHaveBeenCalledWith(usersToCheck);
+			});
+
+			it('should call the accountService.findByUserIdsAndSystemId confirm users', async () => {
+				const { systemId, usersToCheck, usersFound } = setup();
+
+				await uc.updateLastSyncedAt(usersToCheck, systemId);
+
+				expect(accountService.findByUserIdsAndSystemId).toHaveBeenCalledWith(usersFound, systemId);
 			});
 
 			it('should call the userService.updateLastSyncedAt to update users', async () => {
@@ -401,7 +421,7 @@ describe(SynchronizationUc.name, () => {
 				const usersToCheck = [userAId, userBId];
 				const usersToSync = [userAId, userBId];
 
-				userService.findByExternalIdsAndProvidedBySystemId.mockResolvedValueOnce(usersToSync);
+				userService.findMultipleByExternalIds.mockResolvedValueOnce(usersToSync);
 
 				const error = new Error('testError');
 				const expectedError = new FailedUpdateLastSyncedAtLoggableException(systemId);

apps/server/src/modules/idp-console/uc/synchronization.uc.ts

@@ -5,6 +5,7 @@ import { Injectable } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { Logger } from '@src/core/logger';
 import { ErrorLogMessage } from '@src/core/logger/types';
+import { AccountService } from '@src/modules/account';
 import { SynchronizationConfig } from '../interface';
 import { StartSynchronizationLoggable, SucessSynchronizationLoggable } from './loggable';
 import {
@@ -20,6 +21,7 @@ export class SynchronizationUc {
 		private readonly schulconnexRestClient: SchulconnexRestClient,
 		private readonly synchronizationService: SynchronizationService,
 		private readonly userService: UserService,
+		private readonly accountService: AccountService,
 		private readonly logger: Logger
 	) {
 		this.logger.setContext(SynchronizationUc.name);
@@ -70,11 +72,13 @@ export class SynchronizationUc {
 
 	public async updateLastSyncedAt(usersToCheck: string[], systemId: string): Promise<number> {
 		try {
-			const usersToSync = await this.userService.findByExternalIdsAndProvidedBySystemId(usersToCheck, systemId);
+			const foundUsers = await this.userService.findMultipleByExternalIds(usersToCheck);
 
-			await this.userService.updateLastSyncedAt(usersToSync);
+			const verifiedUsers = await this.accountService.findByUserIdsAndSystemId(foundUsers, systemId);
 
-			return usersToSync.length;
+			await this.userService.updateLastSyncedAt(verifiedUsers);
+
+			return verifiedUsers.length;
 		} catch {
 			throw new FailedUpdateLastSyncedAtLoggableException(systemId);
 		}

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

@@ -1,9 +1,7 @@
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
-import { ICurrentUser } from '@infra/auth-guard';
 import { CalendarService } from '@infra/calendar';
 import { EntityManager, MikroORM } from '@mikro-orm/core';
 import { ObjectId } from '@mikro-orm/mongodb';
-import { Account, AccountService } from '@modules/account';
 import {
 	DataDeletedEvent,
 	DeletionErrorLoggableException,
@@ -41,7 +39,6 @@ describe('UserService', () => {
 	let userDORepo: DeepMocked<UserDORepo>;
 	let config: DeepMocked<ConfigService>;
 	let roleService: DeepMocked<RoleService>;
-	let accountService: DeepMocked<AccountService>;
 	let registrationPinService: DeepMocked<RegistrationPinService>;
 	let calendarService: DeepMocked<CalendarService>;
 	let eventBus: DeepMocked<EventBus>;
@@ -72,10 +69,6 @@ describe('UserService', () => {
 					provide: RoleService,
 					useValue: createMock<RoleService>(),
 				},
-				{
-					provide: AccountService,
-					useValue: createMock<AccountService>(),
-				},
 				{
 					provide: RegistrationPinService,
 					useValue: createMock<RegistrationPinService>(),
@@ -106,7 +99,6 @@ describe('UserService', () => {
 		userDORepo = module.get(UserDORepo);
 		config = module.get(ConfigService);
 		roleService = module.get(RoleService);
-		accountService = module.get(AccountService);
 		registrationPinService = module.get(RegistrationPinService);
 		eventBus = module.get(EventBus);
 		calendarService = module.get(CalendarService);
@@ -261,54 +253,6 @@ describe('UserService', () => {
 		});
 	});
 
-	describe('getResolvedUser is called', () => {
-		describe('when a resolved user is requested', () => {
-			const setup = () => {
-				const systemId = 'systemId';
-				const role: Role = roleFactory.buildWithId({
-					name: RoleName.STUDENT,
-					permissions: [Permission.DASHBOARD_VIEW],
-				});
-				const user: UserDO = userDoFactory.buildWithId({ roles: [role] });
-				const account: Account = new Account({
-					id: 'accountId',
-					systemId,
-					username: 'username',
-					createdAt: new Date(),
-					updatedAt: new Date(),
-					activated: true,
-				});
-
-				userDORepo.findById.mockResolvedValue(user);
-				accountService.findByUserIdOrFail.mockResolvedValue(account);
-
-				return {
-					userId: user.id as string,
-					user,
-					account,
-					role,
-					systemId,
-				};
-			};
-
-			it('should return the current user', async () => {
-				const { userId, user, account, role, systemId } = setup();
-
-				const result = await service.getResolvedUser(userId);
-
-				expect(result).toEqual<ICurrentUser>({
-					userId,
-					systemId,
-					schoolId: user.schoolId,
-					accountId: account.id,
-					roles: [role.id],
-					isExternalUser: false,
-					support: false,
-				});
-			});
-		});
-	});
-
 	describe('getDisplayName', () => {
 		let role: Role;
 
@@ -946,7 +890,7 @@ describe('UserService', () => {
 		});
 	});
 
-	describe('findByExternalIdsAndProvidedBySystemId', () => {
+	describe('findMultipleByExternalIds', () => {
 		const setup = () => {
 			const systemId = new ObjectId().toHexString();
 			const userA = userFactory.buildWithId({ externalId: '111' });
@@ -962,39 +906,27 @@ describe('UserService', () => {
 			};
 		};
 
-		describe('when find users By externalIds and systemId', () => {
-			it('should call findMultipleByExternalIds in userService with externalIds', async () => {
-				const { externalIds, foundUsers, systemId } = setup();
-
-				jest.spyOn(service, 'findMultipleByExternalIds').mockResolvedValueOnce(foundUsers);
+		it('should call findMultipleByExternalIds in userService with externalIds', async () => {
+			const { externalIds, foundUsers } = setup();
 
-				await service.findByExternalIdsAndProvidedBySystemId(externalIds, systemId);
-
-				expect(service.findMultipleByExternalIds).toHaveBeenCalledWith(externalIds);
-			});
+			userRepo.findByExternalIds.mockResolvedValueOnce(foundUsers);
 
-			it('should call accountService.findByUserIdsAndSystemId with foundUsers and systemId', async () => {
-				const { externalIds, foundUsers, systemId } = setup();
+			await service.findMultipleByExternalIds(externalIds);
 
-				jest.spyOn(service, 'findMultipleByExternalIds').mockResolvedValueOnce(foundUsers);
-
-				await service.findByExternalIdsAndProvidedBySystemId(externalIds, systemId);
-
-				expect(accountService.findByUserIdsAndSystemId).toHaveBeenCalledWith(foundUsers, systemId);
-			});
+			expect(userRepo.findByExternalIds).toHaveBeenCalledWith(externalIds);
+		});
 
-			it('should return array with verified Users', async () => {
-				const { externalIds, foundUsers, systemId } = setup();
+		it('should return array with verified Users', async () => {
+			const { externalIds, foundUsers } = setup();
 
-				jest.spyOn(service, 'findMultipleByExternalIds').mockResolvedValueOnce(foundUsers);
-				jest.spyOn(accountService, 'findByUserIdsAndSystemId').mockResolvedValueOnce(foundUsers);
+			userRepo.findByExternalIds.mockResolvedValueOnce(foundUsers);
 
-				const result = await service.findByExternalIdsAndProvidedBySystemId(externalIds, systemId);
+			const result = await service.findMultipleByExternalIds(externalIds);
 
-				expect(result).toEqual(foundUsers);
-			});
+			expect(result).toEqual(foundUsers);
 		});
 	});
+
 	describe('findUnsynchronizedUserIds', () => {
 		const setup = () => {
 			const currentDate = new Date();

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

@@ -1,8 +1,4 @@
 import { MikroORM, UseRequestContext } from '@mikro-orm/core';
-import { Account, AccountService } from '@modules/account';
-// invalid import
-import { OauthCurrentUser } from '@modules/authentication/interface';
-import { CurrentUserMapper } from '@modules/authentication/mapper';
 import {
 	DataDeletedEvent,
 	DataDeletionDomainOperationLoggable,
@@ -44,7 +40,6 @@ export class UserService implements DeletionService, IEventHandler<UserDeletedEv
 		private readonly userDORepo: UserDORepo,
 		private readonly configService: ConfigService<UserConfig, true>,
 		private readonly roleService: RoleService,
-		private readonly accountService: AccountService,
 		private readonly registrationPinService: RegistrationPinService,
 		private readonly calendarService: CalendarService,
 		private readonly logger: Logger,
@@ -84,15 +79,6 @@ export class UserService implements DeletionService, IEventHandler<UserDeletedEv
 		return userDto;
 	}
 
-	async getResolvedUser(userId: EntityId): Promise<OauthCurrentUser> {
-		const user: UserDO = await this.findById(userId);
-		const account: Account = await this.accountService.findByUserIdOrFail(userId);
-
-		const resolvedUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser(account.id, user, account.systemId);
-
-		return resolvedUser;
-	}
-
 	async findById(id: string): Promise<UserDO> {
 		const userDO = await this.userDORepo.findById(id, true);
 
@@ -261,14 +247,6 @@ export class UserService implements DeletionService, IEventHandler<UserDeletedEv
 		return users;
 	}
 
-	public async findByExternalIdsAndProvidedBySystemId(externalIds: string[], systemId: string): Promise<string[]> {
-		const foundUsers = await this.findMultipleByExternalIds(externalIds);
-
-		const verifiedUsers = await this.accountService.findByUserIdsAndSystemId(foundUsers, systemId);
-
-		return verifiedUsers;
-	}
-
 	public async findMultipleByExternalIds(externalIds: string[]): Promise<string[]> {
 		return this.userRepo.findByExternalIds(externalIds);
 	}

apps/server/src/modules/user/user.module.ts

@@ -1,25 +1,15 @@
-import { AccountModule } from '@modules/account';
-import { LegacySchoolModule } from '@modules/legacy-school';
+import { CalendarModule } from '@infra/calendar';
+import { RegistrationPinModule } from '@modules/registration-pin';
 import { RoleModule } from '@modules/role';
-import { forwardRef, Module } from '@nestjs/common';
+import { Module } from '@nestjs/common';
+import { CqrsModule } from '@nestjs/cqrs';
 import { UserRepo } from '@shared/repo';
 import { UserDORepo } from '@shared/repo/user/user-do.repo';
 import { LoggerModule } from '@src/core/logger';
-import { CqrsModule } from '@nestjs/cqrs';
-import { RegistrationPinModule } from '@modules/registration-pin';
-import { CalendarModule } from '@infra/calendar';
 import { UserService } from './service';
 
 @Module({
-	imports: [
-		forwardRef(() => LegacySchoolModule),
-		RoleModule,
-		AccountModule,
-		LoggerModule,
-		CqrsModule,
-		RegistrationPinModule,
-		CalendarModule,
-	],
+	imports: [RoleModule, LoggerModule, CqrsModule, RegistrationPinModule, CalendarModule],
 	providers: [UserRepo, UserDORepo, UserService],
 	exports: [UserService, UserRepo],
 })