fix: includes check + gebruik fetches service laag
This commit is contained in:
		
							parent
							
								
									566bb5a5fb
								
							
						
					
					
						commit
						cb4f6a512d
					
				
					 7 changed files with 38 additions and 36 deletions
				
			
		|  | @ -10,6 +10,10 @@ export function mapToUserDTO(user: User): UserDTO { | ||||||
|     }; |     }; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | export function mapToUsername(user: {username: string}): string { | ||||||
|  |     return user.username; | ||||||
|  | } | ||||||
|  | 
 | ||||||
| export function mapToUser<T extends User>(userData: UserDTO, userInstance: T): T { | export function mapToUser<T extends User>(userData: UserDTO, userInstance: T): T { | ||||||
|     userInstance.username = userData.username; |     userInstance.username = userData.username; | ||||||
|     userInstance.firstName = userData.firstName; |     userInstance.firstName = userData.firstName; | ||||||
|  |  | ||||||
|  | @ -1,7 +1,8 @@ | ||||||
| import {authorize} from "./auth-checks"; | import {authorize} from "./auth-checks"; | ||||||
| import {getAssignment} from "../../../services/assignments"; | import {fetchAssignment, getAssignment} from "../../../services/assignments"; | ||||||
| import {getClass} from "../../../services/classes"; | import {fetchClass, getClass} from "../../../services/classes"; | ||||||
| import {getAllGroups} from "../../../services/groups"; | import {getAllGroups} from "../../../services/groups"; | ||||||
|  | import {mapToUsername} from "../../../interfaces/user"; | ||||||
| 
 | 
 | ||||||
| /** | /** | ||||||
|  * Expects the path to contain the path parameters 'classId' and 'id' (meaning the ID of the assignment). |  * Expects the path to contain the path parameters 'classId' and 'id' (meaning the ID of the assignment). | ||||||
|  | @ -12,15 +13,13 @@ import {getAllGroups} from "../../../services/groups"; | ||||||
| export const onlyAllowIfHasAccessToAssignment = authorize( | export const onlyAllowIfHasAccessToAssignment = authorize( | ||||||
|     async (auth, req) => { |     async (auth, req) => { | ||||||
|         const { classid: classId, id: assignmentId } = req.params as { classid: string, id: number }; |         const { classid: classId, id: assignmentId } = req.params as { classid: string, id: number }; | ||||||
|         const assignment = await getAssignment(classId, assignmentId); |         const assignment = await fetchAssignment(classId, assignmentId); | ||||||
|         if (assignment === null) { |         if (auth.accountType === "teacher") { | ||||||
|             return false; |             const clazz = await fetchClass(assignment.class); | ||||||
|         } else if (auth.accountType === "teacher") { |             return clazz.teachers.map(mapToUsername).includes(auth.username); | ||||||
|             const clazz = await getClass(assignment.class); |  | ||||||
|             return auth.username in clazz!.teachers; |  | ||||||
|         } else { |         } else { | ||||||
|             const groups = await getAllGroups(classId, assignmentId, false); |             const groups = await getAllGroups(classId, assignmentId, false); | ||||||
|             return groups.some(group => auth.username in (group.members as string[])); |             return groups.some(group => group.members.map(mapToUsername).includes(auth.username) ); | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| ); | ); | ||||||
|  |  | ||||||
|  | @ -1,11 +1,12 @@ | ||||||
| import {authorize} from "./auth-checks"; | import {authorize} from "./auth-checks"; | ||||||
| import {AuthenticationInfo} from "../authentication-info"; | import {AuthenticationInfo} from "../authentication-info"; | ||||||
| import {AuthenticatedRequest} from "../authenticated-request"; | import {AuthenticatedRequest} from "../authenticated-request"; | ||||||
| import {getClass} from "../../../services/classes"; | import {fetchClass, getClass} from "../../../services/classes"; | ||||||
|  | import {mapToUsername} from "../../../interfaces/user"; | ||||||
| 
 | 
 | ||||||
| async function teaches(teacherUsername: string, classId: string): Promise<boolean> { | async function teaches(teacherUsername: string, classId: string): Promise<boolean> { | ||||||
|     const clazz = await getClass(classId); |     const clazz = await fetchClass(classId); | ||||||
|     return clazz !== null && teacherUsername in clazz.teachers; |     return clazz.teachers.map(mapToUsername).includes(teacherUsername); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /** | /** | ||||||
|  | @ -41,13 +42,11 @@ export const onlyAllowTeacherOfClass = authorize( | ||||||
| export const onlyAllowIfInClass = authorize( | export const onlyAllowIfInClass = authorize( | ||||||
|     async (auth: AuthenticationInfo, req: AuthenticatedRequest) => { |     async (auth: AuthenticationInfo, req: AuthenticatedRequest) => { | ||||||
|         const classId = req.params.classId ?? req.params.classid ?? req.params.id; |         const classId = req.params.classId ?? req.params.classid ?? req.params.id; | ||||||
|         const clazz = await getClass(classId); |         const clazz = await fetchClass(classId); | ||||||
|         if (clazz === null) { |         if (auth.accountType === "teacher") { | ||||||
|             return false; |             return clazz.teachers.map(mapToUsername).includes(auth.username); | ||||||
|         } else if (auth.accountType === "teacher") { |  | ||||||
|             return auth.username in clazz.teachers; |  | ||||||
|         } |         } | ||||||
|         return auth.username in clazz.students; |         return clazz.students.map(mapToUsername).includes(auth.username); | ||||||
|     } |     } | ||||||
| ); | ); | ||||||
| 
 | 
 | ||||||
|  | @ -57,13 +56,11 @@ export const onlyAllowIfInClass = authorize( | ||||||
| export const onlyAllowOwnClassInBody = authorize( | export const onlyAllowOwnClassInBody = authorize( | ||||||
|     async (auth, req) => { |     async (auth, req) => { | ||||||
|         const classId = (req.body as {class: string})?.class; |         const classId = (req.body as {class: string})?.class; | ||||||
|         const clazz = await getClass(classId); |         const clazz = await fetchClass(classId); | ||||||
| 
 | 
 | ||||||
|         if (clazz === null) { |         if (auth.accountType === "teacher") { | ||||||
|             return false; |             return clazz.teachers.map(mapToUsername).includes(auth.username); | ||||||
|         } else if (auth.accountType === "teacher") { |  | ||||||
|             return auth.username in clazz.teachers; |  | ||||||
|         } |         } | ||||||
|         return auth.username in clazz.students; |         return clazz.students.map(mapToUsername).includes(auth.username); | ||||||
|     } |     } | ||||||
| ); | ); | ||||||
|  |  | ||||||
|  | @ -1,6 +1,7 @@ | ||||||
| import {authorize} from "./auth-checks"; | import {authorize} from "./auth-checks"; | ||||||
| import {getClass} from "../../../services/classes"; | import {fetchClass, getClass} from "../../../services/classes"; | ||||||
| import {getGroup} from "../../../services/groups"; | import {fetchGroup, getGroup} from "../../../services/groups"; | ||||||
|  | import {mapToUsername} from "../../../interfaces/user"; | ||||||
| 
 | 
 | ||||||
| /** | /** | ||||||
|  * Expects the path to contain the path parameters 'classid', 'assignmentid' and 'groupid'. |  * Expects the path to contain the path parameters 'classid', 'assignmentid' and 'groupid'. | ||||||
|  | @ -14,11 +15,11 @@ export const onlyAllowIfHasAccessToGroup = authorize( | ||||||
|             req.params as { classid: string, assignmentid: number, groupid: number }; |             req.params as { classid: string, assignmentid: number, groupid: number }; | ||||||
| 
 | 
 | ||||||
|         if (auth.accountType === "teacher") { |         if (auth.accountType === "teacher") { | ||||||
|             const clazz = await getClass(classId); |             const clazz = await fetchClass(classId); | ||||||
|             return auth.username in clazz!.teachers; |             return clazz.teachers.map(mapToUsername).includes(auth.username); | ||||||
|         } else { // user is student
 |         } else { // user is student
 | ||||||
|             const group = await getGroup(classId, assignmentId, groupId, false); |             const group = await fetchGroup(classId, assignmentId, groupId, false); | ||||||
|             return group === null ? false : auth.username in (group.members as string[]); |             return clazz.students.map(mapToUsername).includes(auth.username); | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| ); | ); | ||||||
|  |  | ||||||
|  | @ -25,6 +25,7 @@ import { QuestionDTO, QuestionId } from '@dwengo-1/common/interfaces/question'; | ||||||
| import { ClassJoinRequestDTO } from '@dwengo-1/common/interfaces/class-join-request'; | import { ClassJoinRequestDTO } from '@dwengo-1/common/interfaces/class-join-request'; | ||||||
| import { ConflictException } from '../exceptions/conflict-exception.js'; | import { ConflictException } from '../exceptions/conflict-exception.js'; | ||||||
| import { Submission } from '../entities/assignments/submission.entity'; | import { Submission } from '../entities/assignments/submission.entity'; | ||||||
|  | import {mapToUsername} from "../interfaces/user"; | ||||||
| 
 | 
 | ||||||
| export async function getAllStudents(full: boolean): Promise<StudentDTO[] | string[]> { | export async function getAllStudents(full: boolean): Promise<StudentDTO[] | string[]> { | ||||||
|     const studentRepository = getStudentRepository(); |     const studentRepository = getStudentRepository(); | ||||||
|  | @ -34,7 +35,7 @@ export async function getAllStudents(full: boolean): Promise<StudentDTO[] | stri | ||||||
|         return users.map(mapToStudentDTO); |         return users.map(mapToStudentDTO); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     return users.map((user) => user.username); |     return users.map(mapToUsername); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| export async function fetchStudent(username: string): Promise<Student> { | export async function fetchStudent(username: string): Promise<Student> { | ||||||
|  |  | ||||||
|  | @ -30,6 +30,7 @@ import { QuestionDTO, QuestionId } from '@dwengo-1/common/interfaces/question'; | ||||||
| import { ClassJoinRequestDTO } from '@dwengo-1/common/interfaces/class-join-request'; | import { ClassJoinRequestDTO } from '@dwengo-1/common/interfaces/class-join-request'; | ||||||
| import { ClassStatus } from '@dwengo-1/common/util/class-join-request'; | import { ClassStatus } from '@dwengo-1/common/util/class-join-request'; | ||||||
| import { ConflictException } from '../exceptions/conflict-exception.js'; | import { ConflictException } from '../exceptions/conflict-exception.js'; | ||||||
|  | import {mapToUsername} from "../interfaces/user"; | ||||||
| 
 | 
 | ||||||
| export async function getAllTeachers(full: boolean): Promise<TeacherDTO[] | string[]> { | export async function getAllTeachers(full: boolean): Promise<TeacherDTO[] | string[]> { | ||||||
|     const teacherRepository: TeacherRepository = getTeacherRepository(); |     const teacherRepository: TeacherRepository = getTeacherRepository(); | ||||||
|  | @ -38,7 +39,7 @@ export async function getAllTeachers(full: boolean): Promise<TeacherDTO[] | stri | ||||||
|     if (full) { |     if (full) { | ||||||
|         return users.map(mapToTeacherDTO); |         return users.map(mapToTeacherDTO); | ||||||
|     } |     } | ||||||
|     return users.map((user) => user.username); |     return users.map(mapToUsername); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| export async function fetchTeacher(username: string): Promise<Teacher> { | export async function fetchTeacher(username: string): Promise<Teacher> { | ||||||
|  |  | ||||||
|  | @ -96,7 +96,7 @@ describe('Teacher controllers', () => { | ||||||
|     }); |     }); | ||||||
| 
 | 
 | ||||||
|     it('Teacher list', async () => { |     it('Teacher list', async () => { | ||||||
|         req = { query: { full: 'true' } }; |         req = { query: { full: 'false' } }; | ||||||
| 
 | 
 | ||||||
|         await getAllTeachersHandler(req as Request, res as Response); |         await getAllTeachersHandler(req as Request, res as Response); | ||||||
| 
 | 
 | ||||||
|  | @ -104,8 +104,7 @@ describe('Teacher controllers', () => { | ||||||
| 
 | 
 | ||||||
|         const result = jsonMock.mock.lastCall?.[0]; |         const result = jsonMock.mock.lastCall?.[0]; | ||||||
| 
 | 
 | ||||||
|         const teacherUsernames = result.teachers.map((s: TeacherDTO) => s.username); |         expect(result.teachers).toContain('testleerkracht1'); | ||||||
|         expect(teacherUsernames).toContain('testleerkracht1'); |  | ||||||
| 
 | 
 | ||||||
|         expect(result.teachers).toHaveLength(5); |         expect(result.teachers).toHaveLength(5); | ||||||
|     }); |     }); | ||||||
|  |  | ||||||
		Reference in a new issue
	
	 Gabriellvl
						Gabriellvl