Skip to content

Commit

Permalink
N21-1541 handle missing users (#4604)
Browse files Browse the repository at this point in the history 
* findNyIdOrNull function, loggable
*null filtering, typeguard

---------

Co-authored-by: Arne Gnisa <[email protected]>
  • Loading branch information
@IgorCapCoder @arnegns
IgorCapCoder and arnegns authored Dec 1, 2023
1 parent 9a07db2 commit ebaaff8
Show file tree
Hide file tree
Showing 10 changed files with 402 additions and 19 deletions.
12 changes: 11 additions & 1 deletion apps/server/src/modules/group/group-api.module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,22 @@ import { RoleModule } from '@modules/role';
import { LegacySchoolModule } from '@modules/legacy-school';
import { SystemModule } from '@modules/system';
import { UserModule } from '@modules/user';
import { LoggerModule } from '@src/core/logger';
import { GroupController } from './controller';
import { GroupModule } from './group.module';
import { GroupUc } from './uc';

@Module({
imports: [GroupModule, ClassModule, UserModule, RoleModule, LegacySchoolModule, AuthorizationModule, SystemModule],
imports: [
GroupModule,
ClassModule,
UserModule,
RoleModule,
LegacySchoolModule,
AuthorizationModule,
SystemModule,
LoggerModule,
],
controllers: [GroupController],
providers: [GroupUc],
})
Expand Down
160 changes: 160 additions & 0 deletions apps/server/src/modules/group/uc/group.uc.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
userDoFactory,
userFactory,
} from '@shared/testing';
import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable';
import { Logger } from '@src/core/logger';
import { SchoolYearQueryType } from '../controller/dto/interface';
import { Group, GroupTypes } from '../domain';
import { UnknownQueryTypeLoggableException } from '../loggable';
Expand All @@ -45,6 +47,7 @@ describe('GroupUc', () => {
let schoolService: DeepMocked<LegacySchoolService>;
let authorizationService: DeepMocked<AuthorizationService>;
let schoolYearService: DeepMocked<SchoolYearService>;
let logger: DeepMocked<Logger>;

beforeAll(async () => {
module = await Test.createTestingModule({
Expand Down Expand Up @@ -82,6 +85,10 @@ describe('GroupUc', () => {
provide: SchoolYearService,
useValue: createMock<SchoolYearService>(),
},
{
provide: Logger,
useValue: createMock<Logger>(),
},
],
}).compile();

Expand All @@ -94,6 +101,7 @@ describe('GroupUc', () => {
schoolService = module.get(LegacySchoolService);
authorizationService = module.get(AuthorizationService);
schoolYearService = module.get(SchoolYearService);
logger = module.get(Logger);

await setupEntities();
});
Expand Down Expand Up @@ -217,6 +225,17 @@ describe('GroupUc', () => {

throw new Error();
});
userService.findByIdOrNull.mockImplementation((userId: string): Promise<UserDO> => {
if (userId === teacherUser.id) {
return Promise.resolve(teacherUserDo);
}

if (userId === studentUser.id) {
return Promise.resolve(studentUserDo);
}

throw new Error();
});
roleService.findById.mockImplementation((roleId: string): Promise<RoleDto> => {
if (roleId === teacherUser.roles[0].id) {
return Promise.resolve(teacherRole);
Expand Down Expand Up @@ -567,6 +586,21 @@ describe('GroupUc', () => {

throw new Error();
});
userService.findByIdOrNull.mockImplementation((userId: string): Promise<UserDO> => {
if (userId === teacherUser.id) {
return Promise.resolve(teacherUserDo);
}

if (userId === studentUser.id) {
return Promise.resolve(studentUserDo);
}

if (userId === adminUser.id) {
return Promise.resolve(adminUserDo);
}

throw new Error();
});
roleService.findById.mockImplementation((roleId: string): Promise<RoleDto> => {
if (roleId === teacherUser.roles[0].id) {
return Promise.resolve(teacherRole);
Expand Down Expand Up @@ -733,6 +767,130 @@ describe('GroupUc', () => {
});
});
});

describe('when class has a user referenced which is not existing', () => {
const setup = () => {
const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId();
const notFoundReferenceId = new ObjectId().toHexString();
const { teacherUser } = UserAndAccountTestFactory.buildTeacher();

const teacherRole: RoleDto = roleDtoFactory.buildWithId({
id: teacherUser.roles[0].id,
name: teacherUser.roles[0].name,
});

const teacherUserDo: UserDO = userDoFactory.buildWithId({
id: teacherUser.id,
lastName: teacherUser.lastName,
roles: [{ id: teacherUser.roles[0].id, name: teacherUser.roles[0].name }],
});

const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId();
const clazz: Class = classFactory.build({
name: 'A',
teacherIds: [teacherUser.id, notFoundReferenceId],
source: 'LDAP',
year: schoolYear.id,
});
const system: SystemDto = new SystemDto({
id: new ObjectId().toHexString(),
displayName: 'External System',
type: 'oauth2',
});
const group: Group = groupFactory.build({
name: 'B',
users: [
{ userId: teacherUser.id, roleId: teacherUser.roles[0].id },
{ userId: notFoundReferenceId, roleId: teacherUser.roles[0].id },
],
externalSource: undefined,
});

schoolService.getSchoolById.mockResolvedValueOnce(school);
authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser);
authorizationService.hasAllPermissions.mockReturnValueOnce(false);
classService.findAllByUserId.mockResolvedValueOnce([clazz]);
groupService.findByUser.mockResolvedValueOnce([group]);
systemService.findById.mockResolvedValue(system);

userService.findById.mockImplementation((userId: string): Promise<UserDO> => {
if (userId === teacherUser.id) {
return Promise.resolve(teacherUserDo);
}

throw new Error();
});
userService.findByIdOrNull.mockImplementation((userId: string): Promise<UserDO | null> => {
if (userId === teacherUser.id) {
return Promise.resolve(teacherUserDo);
}

if (userId === notFoundReferenceId) {
return Promise.resolve(null);
}

throw new Error();
});
roleService.findById.mockImplementation((roleId: string): Promise<RoleDto> => {
if (roleId === teacherUser.roles[0].id) {
return Promise.resolve(teacherRole);
}

throw new Error();
});
schoolYearService.findById.mockResolvedValue(schoolYear);

return {
teacherUser,
clazz,
group,
notFoundReferenceId,
schoolYear,
};
};

it('should return class without missing user', async () => {
const { teacherUser, clazz, group, schoolYear } = setup();

const result = await uc.findAllClasses(teacherUser.id, teacherUser.school.id);

expect(result).toEqual<Page<ClassInfoDto>>({
data: [
{
id: clazz.id,
name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name,
type: ClassRootType.CLASS,
externalSourceName: clazz.source,
teacherNames: [teacherUser.lastName],
schoolYear: schoolYear.name,
isUpgradable: false,
studentCount: 2,
},
{
id: group.id,
name: group.name,
type: ClassRootType.GROUP,
teacherNames: [teacherUser.lastName],
studentCount: 0,
},
],
total: 2,
});
});

it('should log the missing user', async () => {
const { teacherUser, clazz, group, notFoundReferenceId } = setup();

await uc.findAllClasses(teacherUser.id, teacherUser.school.id);

expect(logger.warning).toHaveBeenCalledWith(
new ReferencedEntityNotFoundLoggable(Class.name, clazz.id, UserDO.name, notFoundReferenceId)
);
expect(logger.warning).toHaveBeenCalledWith(
new ReferencedEntityNotFoundLoggable(Group.name, group.id, UserDO.name, notFoundReferenceId)
);
});
});
});

describe('getGroup', () => {
Expand Down Expand Up @@ -817,8 +975,10 @@ describe('GroupUc', () => {
groupService.findById.mockResolvedValueOnce(group);
authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser);
userService.findById.mockResolvedValueOnce(teacherUserDo);
userService.findByIdOrNull.mockResolvedValueOnce(teacherUserDo);
roleService.findById.mockResolvedValueOnce(teacherRole);
userService.findById.mockResolvedValueOnce(studentUserDo);
userService.findByIdOrNull.mockResolvedValueOnce(studentUserDo);
roleService.findById.mockResolvedValueOnce(studentRole);

return {
Expand Down
71 changes: 53 additions & 18 deletions apps/server/src/modules/group/uc/group.uc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { Class } from '@modules/class/domain';
import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school';
import { RoleService } from '@modules/role';
import { RoleDto } from '@modules/role/service/dto/role.dto';
import { LegacySystemService, SystemDto } from '@modules/system';
import { UserService } from '@modules/user';
import { Injectable } from '@nestjs/common';
import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable';
import { LegacySchoolDo, Page, UserDO } from '@shared/domain/domainobject';
import { SchoolYearEntity, User } from '@shared/domain/entity';
import { Permission, SortOrder } from '@shared/domain/interface';
import { EntityId } from '@shared/domain/types';
import { Logger } from '@src/core/logger';
import { LegacySystemService, SystemDto } from '@src/modules/system';
import { SchoolYearQueryType } from '../controller/dto/interface';
import { Group, GroupUser } from '../domain';
import { UnknownQueryTypeLoggableException } from '../loggable';
Expand All @@ -29,7 +31,8 @@ export class GroupUc {
private readonly roleService: RoleService,
private readonly schoolService: LegacySchoolService,
private readonly authorizationService: AuthorizationService,
private readonly schoolYearService: SchoolYearService
private readonly schoolYearService: SchoolYearService,
private readonly logger: Logger
) {}

public async findAllClasses(
Expand Down Expand Up @@ -143,14 +146,14 @@ export class GroupUc {
this.isClassOfQueryType(currentYear, classWithSchoolYear.schoolYear, schoolYearQueryType)
);

const classInfosFromClasses = await this.mapClassInfosFromClasses(filteredClassesForSchoolYear);
const classInfosFromClasses: ClassInfoDto[] = await this.mapClassInfosFromClasses(filteredClassesForSchoolYear);

return classInfosFromClasses;
}

private async addSchoolYearsToClasses(classes: Class[]): Promise<{ clazz: Class; schoolYear?: SchoolYearEntity }[]> {
const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await Promise.all(
classes.map(async (clazz) => {
classes.map(async (clazz: Class) => {
let schoolYear: SchoolYearEntity | undefined;
if (clazz.year) {
schoolYear = await this.schoolYearService.findById(clazz.year);
Expand All @@ -162,6 +165,7 @@ export class GroupUc {
};
})
);

return classesWithSchoolYear;
}

Expand Down Expand Up @@ -193,11 +197,10 @@ export class GroupUc {
private async mapClassInfosFromClasses(
filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[]
): Promise<ClassInfoDto[]> {
const classInfosFromClasses = await Promise.all(
const classInfosFromClasses: ClassInfoDto[] = await Promise.all(
filteredClassesForSchoolYear.map(async (classWithSchoolYear): Promise<ClassInfoDto> => {
const teachers: UserDO[] = await Promise.all(
classWithSchoolYear.clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId))
);
const { teacherIds } = classWithSchoolYear.clazz;
const teachers: UserDO[] = await this.getTeachersByIds(teacherIds, classWithSchoolYear.clazz.id);

const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(
classWithSchoolYear.clazz,
Expand All @@ -208,9 +211,28 @@ export class GroupUc {
return mapped;
})
);

return classInfosFromClasses;
}

private async getTeachersByIds(teacherIds: EntityId[], classId: EntityId): Promise<UserDO[]> {
const teacherPromises: Promise<UserDO | null>[] = teacherIds.map(
async (teacherId: EntityId): Promise<UserDO | null> => {
const teacher: UserDO | null = await this.userService.findByIdOrNull(teacherId);
if (!teacher) {
this.logger.warning(new ReferencedEntityNotFoundLoggable(Class.name, classId, UserDO.name, teacherId));
}
return teacher;
}
);

const teachers: UserDO[] = (await Promise.all(teacherPromises)).filter(
(teacher: UserDO | null): teacher is UserDO => teacher !== null
);

return teachers;
}

private async findGroupsOfTypeClassForSchool(schoolId: EntityId): Promise<ClassInfoDto[]> {
const groupsOfTypeClass: Group[] = await this.groupService.findClassesForSchool(schoolId);

Expand Down Expand Up @@ -271,20 +293,33 @@ export class GroupUc {
}

private async findUsersForGroup(group: Group): Promise<ResolvedGroupUser[]> {
const resolvedGroupUsers: ResolvedGroupUser[] = await Promise.all(
group.users.map(async (groupUser: GroupUser): Promise<ResolvedGroupUser> => {
const user: UserDO = await this.userService.findById(groupUser.userId);
const role: RoleDto = await this.roleService.findById(groupUser.roleId);

const resolvedGroups = new ResolvedGroupUser({
user,
role,
});
const resolvedGroupUsersOrNull: (ResolvedGroupUser | null)[] = await Promise.all(
group.users.map(async (groupUser: GroupUser): Promise<ResolvedGroupUser | null> => {
const user: UserDO | null = await this.userService.findByIdOrNull(groupUser.userId);
let resolvedGroup: ResolvedGroupUser | null = null;

if (!user) {
this.logger.warning(
new ReferencedEntityNotFoundLoggable(Group.name, group.id, UserDO.name, groupUser.userId)
);
} else {
const role: RoleDto = await this.roleService.findById(groupUser.roleId);

resolvedGroup = new ResolvedGroupUser({
user,
role,
});
}

return resolvedGroups;
return resolvedGroup;
})
);

const resolvedGroupUsers: ResolvedGroupUser[] = resolvedGroupUsersOrNull.filter(
(resolvedGroupUser: ResolvedGroupUser | null): resolvedGroupUser is ResolvedGroupUser =>
resolvedGroupUser !== null
);

return resolvedGroupUsers;
}

Expand Down
Loading

0 comments on commit ebaaff8

Please sign in to comment.