Skip to content

Commit

Permalink
[Students] Use an HTTP status code instead of throwing an error if st…
Browse files Browse the repository at this point in the history
…udent is not found or user does not have permission to view student's data
  • Loading branch information
valtterikantanen committed Nov 5, 2024
1 parent 807e5fb commit 3a697b9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 38 deletions.
20 changes: 8 additions & 12 deletions services/backend/src/routes/students.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ import { ApplicationError } from '../util/customErrors'

const router = Router()

const filterStudentTags = (student: Awaited<ReturnType<typeof withStudentNumber>>, userId: string) => {
if (!student) return null
return {
...student,
tags: (student.tags ?? []).filter(({ tag }) => !tag.personal_user_id || tag.personal_user_id === userId),
}
}

interface GetStudentsRequest extends Request {
query: {
searchTerm: string
Expand Down Expand Up @@ -59,11 +51,15 @@ router.get('/:studentNumber', async (req: GetStudentRequest, res: Response) => {
} = req

if (!hasFullAccessToStudentData(roles) && !studentsUserCanAccess.includes(studentNumber)) {
throw new ApplicationError('Error finding student', 400)
return res
.status(403)
.json({ error: `User does not have permission to view data for student number ${studentNumber}.` })
}
const student = await withStudentNumber(studentNumber, id)
if (!student) {
return res.status(404).json({ error: `Student not found with student number ${studentNumber}.` })
}
const student = await withStudentNumber(studentNumber)
const filteredTags = filterStudentTags(student, id)
res.json(filteredTags)
res.json(student)
})

export default router
34 changes: 8 additions & 26 deletions services/backend/src/services/students.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
import { Tag, TagStudent } from '../models/kone'
import { EnrollmentState, UnifyStatus } from '../types'
import { splitByEmptySpace } from '../util'
import logger from '../util/logger'

const { sequelize } = dbConnections

Expand Down Expand Up @@ -229,7 +228,7 @@ const formatSharedStudentData = ({
updatedAt,
createdAt,
sis_person_id,
}: Partial<Student>) => {
}: InferAttributes<Student>) => {
const toCourse = ({ grade, credits, credittypecode, is_open, attainment_date, course, isStudyModule }: Credit) => {
course = course.toJSON()

Expand Down Expand Up @@ -272,31 +271,14 @@ const formatSharedStudentData = ({
}
}

const formatStudent = (
studentData: Partial<Student> & {
tags: Array<InferAttributes<TagStudent> & { programme?: Pick<InferAttributes<ProgrammeModule>, 'code' | 'name'> }>
export const withStudentNumber = async (studentNumber: string, userId: string) => {
const student = await byStudentNumber(studentNumber)
if (student == null) {
return null
}
) => {
const formattedData = formatSharedStudentData(studentData)
return {
...formattedData,
tags: studentData.tags,
}
}

const formatStudentWithoutTags = (studentData: Partial<Student>) => {
return formatSharedStudentData(studentData)
}

export const withStudentNumber = async (studentNumber: string) => {
try {
const student = await byStudentNumber(studentNumber)
if (!student) return null
return formatStudent(student)
} catch (error) {
logger.error(`Error when fetching single student`)
logger.error(error)
return null
...formatSharedStudentData(student),
tags: student.tags.filter(({ tag }) => !tag.personal_user_id || tag.personal_user_id === userId),
}
}

Expand Down Expand Up @@ -363,7 +345,7 @@ export const bySearchTermAndStudentNumbers = async (searchterm: string, studentN
}
: { [Op.or]: [nameLike(terms), studentnumberLike(terms)] },
})
).map(formatStudentWithoutTags)
).map(formatSharedStudentData)
}

export const filterStudentnumbersByAccessrights = async (studentnumbers: string[], codes: string[]) =>
Expand Down

0 comments on commit 3a697b9

Please sign in to comment.