Skip to content

Commit

Permalink
BC-8335 - reduce UserModule Dependencies (#5319)
Browse files Browse the repository at this point in the history
* remove AccountModule from UserModule Dependencies
* remove LegacyschoolModule from UserModule Dependencies
  • Loading branch information
Metauriel authored Oct 30, 2024
1 parent 37186aa commit 733ceca
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 126 deletions.
2 changes: 2 additions & 0 deletions apps/server/src/modules/idp-console/idp-console.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -30,6 +31,7 @@ import { SynchronizationUc } from './uc';
// debug: true, // use it for locally debugging of queries
}),
UserModule,
AccountModule,
LoggerModule,
RabbitMQWrapperModule,
ConsoleWriterModule,
Expand Down
30 changes: 25 additions & 5 deletions apps/server/src/modules/idp-console/uc/synchronization.uc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>;

Expand All @@ -45,6 +47,10 @@ describe(SynchronizationUc.name, () => {
provide: UserService,
useValue: createMock<UserService>(),
},
{
provide: AccountService,
useValue: createMock<AccountService>(),
},
{
provide: SchulconnexRestClient,
useValue: createMock<SchulconnexRestClient>(),
Expand All @@ -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();
});
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 7 additions & 3 deletions apps/server/src/modules/idp-console/uc/synchronization.uc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
94 changes: 13 additions & 81 deletions apps/server/src/modules/user/service/user.service.spec.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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>;
Expand Down Expand Up @@ -72,10 +69,6 @@ describe('UserService', () => {
provide: RoleService,
useValue: createMock<RoleService>(),
},
{
provide: AccountService,
useValue: createMock<AccountService>(),
},
{
provide: RegistrationPinService,
useValue: createMock<RegistrationPinService>(),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -946,7 +890,7 @@ describe('UserService', () => {
});
});

describe('findByExternalIdsAndProvidedBySystemId', () => {
describe('findMultipleByExternalIds', () => {
const setup = () => {
const systemId = new ObjectId().toHexString();
const userA = userFactory.buildWithId({ externalId: '111' });
Expand All @@ -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();
Expand Down
22 changes: 0 additions & 22 deletions apps/server/src/modules/user/service/user.service.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down
20 changes: 5 additions & 15 deletions apps/server/src/modules/user/user.module.ts
Original file line number Diff line number Diff line change
@@ -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],
})
Expand Down

0 comments on commit 733ceca

Please sign in to comment.