Skip to content

Commit

Permalink
BC-8106 - teacher discoverability (#5299)
Browse files Browse the repository at this point in the history
find teachers of school
* enable user repo to search by role
* user service function to find users by role and school
* endpoint in school api to find teachers of school

find public teachers of school
* add discoverability as field in nest (already exists in DB/feathers)
* repo functionality to search by discoverability
* add service function to get public teachers, based on configuration
* uc for finding teachers of school gets public teachers instead when user is not part of the school

various
* in order to make api tests with roles work, we had to introduce caching lables for the role cache, used to invalidate the cache in tests.

---------

Co-authored-by: hoeppner.dataport <[email protected]>
  • Loading branch information
Metauriel and hoeppner-dataport authored Oct 22, 2024
1 parent 23f1326 commit 30c64c9
Show file tree
Hide file tree
Showing 23 changed files with 718 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { ApiProperty } from '@nestjs/swagger';
import { PaginationResponse } from '@shared/controller';

export class SchoolUserResponse {
@ApiProperty()
firstName!: string;

@ApiProperty()
lastName!: string;

@ApiProperty()
id!: string;

constructor(props: SchoolUserResponse) {
this.id = props.id;
this.firstName = props.firstName;
this.lastName = props.lastName;
}
}

export class SchoolUserListResponse extends PaginationResponse<SchoolUserResponse[]> {
constructor(data: SchoolUserResponse[], total: number, skip?: number, limit?: number) {
super(total, skip, limit);
this.data = data;
}

@ApiProperty({ type: [SchoolUserResponse] })
data: SchoolUserResponse[];
}
1 change: 1 addition & 0 deletions apps/server/src/modules/school/api/mapper/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './school-systems.response.mapper';
export * from './school.response.mapper';
export * from './school-user.response.mapper';
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { PaginationParams } from '@shared/controller';
import { Page, UserDO } from '@shared/domain/domainobject';
import { SchoolUserListResponse, SchoolUserResponse } from '../dto/response/school-user.response';

export class SchoolUserResponseMapper {
public static mapToResponse(user: UserDO): SchoolUserResponse {
const res = new SchoolUserResponse({
id: user.id || '',
firstName: user.firstName,
lastName: user.lastName,
});

return res;
}

static mapToListResponse(users: Page<UserDO>, pagination?: PaginationParams): SchoolUserListResponse {
const data: SchoolUserResponse[] = users.data.map((user) => this.mapToResponse(user));
const response = new SchoolUserListResponse(data, users.total, pagination?.skip, pagination?.limit);

return response;
}
}
13 changes: 13 additions & 0 deletions apps/server/src/modules/school/api/school.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ import { CurrentUser, ICurrentUser, JwtAuthentication } from '@infra/auth-guard'
import { Body, Controller, ForbiddenException, Get, NotFoundException, Param, Patch, Query } from '@nestjs/common';
import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger';
import { ApiValidationError } from '@shared/common';
import { PaginationParams } from '@shared/controller';
import { SchoolQueryParams, SchoolRemoveSystemUrlParams, SchoolUpdateBodyParams, SchoolUrlParams } from './dto/param';
import { SchoolForExternalInviteResponse, SchoolResponse, SchoolSystemResponse } from './dto/response';
import { SchoolExistsResponse } from './dto/response/school-exists.response';
import { SchoolForLdapLoginResponse } from './dto/response/school-for-ldap-login.response';
import { SchoolUserListResponse } from './dto/response/school-user.response';
import { SchoolUc } from './school.uc';

@ApiTags('School')
Expand Down Expand Up @@ -91,4 +93,15 @@ export class SchoolController {
): Promise<void> {
await this.schoolUc.removeSystemFromSchool(urlParams.schoolId, urlParams.systemId, user.userId);
}

@Get('/:schoolId/teachers')
@JwtAuthentication()
public async getTeachers(
@Param() urlParams: SchoolUrlParams,
@CurrentUser() user: ICurrentUser,
@Query() pagination: PaginationParams
): Promise<SchoolUserListResponse> {
const res = await this.schoolUc.getSchoolTeachers(urlParams.schoolId, user.userId, pagination);
return res;
}
}
47 changes: 44 additions & 3 deletions apps/server/src/modules/school/api/school.uc.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization';
import { Injectable } from '@nestjs/common';
import { Permission, SortOrder } from '@shared/domain/interface';
import { PaginationParams } from '@shared/controller';
import { Page, UserDO } from '@shared/domain/domainobject';
import { User } from '@shared/domain/entity';
import { Permission, RoleName, SortOrder } from '@shared/domain/interface';
import { EntityId } from '@shared/domain/types';
import { UserService } from '@src/modules/user';
import { School, SchoolQuery, SchoolService, SchoolYear, SchoolYearHelper, SchoolYearService } from '../domain';
import { SchoolUpdateBodyParams } from './dto/param';
import {
Expand All @@ -11,15 +15,17 @@ import {
SchoolSystemResponse,
} from './dto/response';
import { SchoolForLdapLoginResponse } from './dto/response/school-for-ldap-login.response';
import { SchoolResponseMapper, SystemResponseMapper } from './mapper';
import { SchoolUserListResponse } from './dto/response/school-user.response';
import { SchoolResponseMapper, SystemResponseMapper, SchoolUserResponseMapper } from './mapper';
import { YearsResponseMapper } from './mapper/years.response.mapper';

@Injectable()
export class SchoolUc {
constructor(
private readonly authorizationService: AuthorizationService,
private readonly schoolService: SchoolService,
private readonly schoolYearService: SchoolYearService
private readonly schoolYearService: SchoolYearService,
private readonly userService: UserService
) {}

public async getSchoolById(schoolId: EntityId, userId: EntityId): Promise<SchoolResponse> {
Expand Down Expand Up @@ -126,4 +132,39 @@ export class SchoolUc {

return dto;
}

public async getSchoolTeachers(
schoolId: EntityId,
userId: EntityId,
pagination?: PaginationParams
): Promise<SchoolUserListResponse> {
const [school, user] = await Promise.all([
this.schoolService.getSchoolById(schoolId),
this.authorizationService.getUserWithPermissions(userId),
]);

this.checkHasPermissionToAccessTeachers(user);

const isUserOfSchool = this.isSchoolInternalUser(user, school);

let result: Page<UserDO>;
if (isUserOfSchool) {
result = await this.userService.findBySchoolRole(schoolId, RoleName.TEACHER, { pagination });
} else {
result = await this.userService.findPublicTeachersBySchool(schoolId, { pagination });
}

const responseDto = SchoolUserResponseMapper.mapToListResponse(result, pagination);
return responseDto;
}

private checkHasPermissionToAccessTeachers(user: User) {
this.authorizationService.checkAllPermissions(user, [Permission.TEACHER_LIST]);
}

private isSchoolInternalUser(user: User, school: School): boolean {
const authContext = AuthorizationContextBuilder.read([Permission.TEACHER_LIST]);
const isUserOfSchool = this.authorizationService.hasPermission(user, school, authContext);
return isUserOfSchool;
}
}
217 changes: 217 additions & 0 deletions apps/server/src/modules/school/api/test/school-users.api.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
import { EntityManager, ObjectId } from '@mikro-orm/mongodb';
import { HttpStatus, INestApplication } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import {
cleanupCollections,
schoolEntityFactory,
TestApiClient,
UserAndAccountTestFactory,
userFactory,
} from '@shared/testing';
import { ServerTestModule } from '@src/modules/server';
import { SchoolUserListResponse } from '../dto/response/school-user.response';

describe('School Controller (API)', () => {
let app: INestApplication;
let em: EntityManager;
let testApiClient: TestApiClient;

beforeAll(async () => {
const moduleFixture = await Test.createTestingModule({
imports: [ServerTestModule],
}).compile();

app = moduleFixture.createNestApplication();
await app.init();
em = app.get(EntityManager);
testApiClient = new TestApiClient(app, 'school');
});

beforeEach(async () => {
await cleanupCollections(em);
await em.clearCache('roles-cache-teacher');
});

afterAll(async () => {
await app.close();
});

describe('get Teachers', () => {
describe('when no user is logged in', () => {
it('should return 401', async () => {
const someId = new ObjectId().toHexString();

const response = await testApiClient.get(`${someId}/teachers`);

expect(response.status).toEqual(HttpStatus.UNAUTHORIZED);
});
});

describe('when schoolId is invalid format', () => {
const setup = async () => {
const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher();

await em.persistAndFlush([teacherAccount, teacherUser]);
em.clear();

const loggedInClient = await testApiClient.login(teacherAccount);

return { loggedInClient };
};

it('should return 400', async () => {
const { loggedInClient } = await setup();

const response = await loggedInClient.get(`/123/teachers`);

expect(response.status).toEqual(HttpStatus.BAD_REQUEST);
expect(response.body).toEqual(
expect.objectContaining({
validationErrors: [{ errors: ['schoolId must be a mongodb id'], field: ['schoolId'] }],
})
);
});
});

describe('when schoolId doesnt exist', () => {
const setup = async () => {
const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher();

await em.persistAndFlush([teacherAccount, teacherUser]);
em.clear();

const loggedInClient = await testApiClient.login(teacherAccount);

return { loggedInClient };
};

it('should return 404', async () => {
const { loggedInClient } = await setup();
const someId = new ObjectId().toHexString();

const response = await loggedInClient.get(`/${someId}/teachers`);

expect(response.status).toEqual(HttpStatus.NOT_FOUND);
});
});

describe('when user is not in the correct school', () => {
const setup = async () => {
const school = schoolEntityFactory.build();
const otherSchool = schoolEntityFactory.build();
const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school: otherSchool });
const teacherRole = teacherUser.roles[0];
const teachersOfSchool = userFactory.buildList(3, { school, roles: [teacherRole] });
const publicTeachersOfSchool = userFactory.buildList(2, { school, roles: [teacherRole], discoverable: true });

await em.persistAndFlush([teacherAccount, teacherUser, ...teachersOfSchool, ...publicTeachersOfSchool]);
em.clear();

const loggedInClient = await testApiClient.login(teacherAccount);

return { loggedInClient, teacherUser, teachersOfSchool, school, publicTeachersOfSchool };
};

it('should return only public teachers', async () => {
const { loggedInClient, school, publicTeachersOfSchool } = await setup();

const response = await loggedInClient.get(`${school.id}/teachers`);
const body = response.body as SchoolUserListResponse;

expect(response.status).toEqual(HttpStatus.OK);
expect(body.total).toEqual(publicTeachersOfSchool.length);
expect(body.data).toEqual(
expect.arrayContaining([
...publicTeachersOfSchool.map((teacher) => {
return {
id: teacher.id,
firstName: teacher.firstName,
lastName: teacher.lastName,
};
}),
])
);
});
});

describe('when user has no permission to view teachers', () => {
const setup = async () => {
const school = schoolEntityFactory.build();
const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent({ school });
const teacherRole = studentUser.roles[0];
const teachersOfSchool = userFactory.buildList(3, { school, roles: [teacherRole] });

await em.persistAndFlush([studentAccount, studentUser, ...teachersOfSchool]);
em.clear();

const loggedInClient = await testApiClient.login(studentAccount);

return { loggedInClient, studentUser, teachersOfSchool, school };
};

it('should return 403', async () => {
const { loggedInClient, school } = await setup();

const response = await loggedInClient.get(`${school.id}/teachers`);

expect(response.status).toEqual(HttpStatus.UNAUTHORIZED);
});
});

describe('when user has permission to view teachers', () => {
const setup = async () => {
const school = schoolEntityFactory.build();
const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school });
const teacherRole = teacherUser.roles[0];
const teachersOfSchool = userFactory.buildList(3, { school, roles: [teacherRole] });

const otherSchool = schoolEntityFactory.build();
const teachersOfOtherSchool = userFactory.buildList(3, { school: otherSchool, roles: [teacherRole] });

await em.persistAndFlush([teacherAccount, teacherUser, ...teachersOfSchool, ...teachersOfOtherSchool]);
em.clear();

const loggedInClient = await testApiClient.login(teacherAccount);

return { loggedInClient, teacherUser, teachersOfSchool, school };
};

it('should return 200 with teachers', async () => {
const { loggedInClient, teacherUser, teachersOfSchool, school } = await setup();

const response = await loggedInClient.get(`${school.id}/teachers`);

const body = response.body as SchoolUserListResponse;

expect(response.status).toEqual(HttpStatus.OK);
expect(body.data).toEqual(
expect.arrayContaining([
expect.objectContaining({
id: teacherUser.id,
firstName: teacherUser.firstName,
lastName: teacherUser.lastName,
}),
...teachersOfSchool.map((teacher) => {
return {
id: teacher.id,
firstName: teacher.firstName,
lastName: teacher.lastName,
};
}),
])
);
});

it('should paginate', async () => {
const { loggedInClient, school } = await setup();

const response = await loggedInClient.get(`${school.id}/teachers`).query({ skip: 1, limit: 1 });
const body = response.body as SchoolUserListResponse;

expect(body.data).toHaveLength(1);
expect(body.total).toEqual(4);
expect(body.skip).toEqual(1);
});
});
});
});
3 changes: 2 additions & 1 deletion apps/server/src/modules/school/school-api.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import { AuthorizationModule } from '@modules/authorization/authorization.module
import { Module } from '@nestjs/common';
import { SchoolController, SchoolUc } from './api';
import { SchoolModule } from './school.module';
import { UserModule } from '../user';

@Module({
imports: [SchoolModule, AuthorizationModule],
imports: [SchoolModule, AuthorizationModule, UserModule],
controllers: [SchoolController],
providers: [SchoolUc],
})
Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/modules/server/admin-api-server.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ const config: AdminApiServerConfig = {
FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED') as boolean,
FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED: Configuration.get('FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED') as boolean,
CTL_TOOLS__PREFERRED_TOOLS_LIMIT: Configuration.get('CTL_TOOLS__PREFERRED_TOOLS_LIMIT') as number,
TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION: Configuration.get(
'TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION'
) as string,
};

export const adminApiServerConfig = () => config;
Loading

0 comments on commit 30c64c9

Please sign in to comment.