From b05b11aa06c526803b2cbd0ba4d35a69eb6da2e2 Mon Sep 17 00:00:00 2001 From: Valtteri Kantanen Date: Mon, 14 Oct 2024 17:38:44 +0300 Subject: [PATCH] Don't fetch enrollments with state other than `ENROLLED` or `REJECTED` (#4292) Rejected enrollments are only used in Programme courses / By semester and Language center view --- .../src/services/completedCoursesSearch.ts | 7 ++--- .../backend/src/services/courses/index.ts | 9 ++++-- .../src/services/openUni/openUniSearches.ts | 3 +- .../src/services/populations/bottlenecksOf.ts | 14 +++++---- .../getStudentsIncludeCoursesBetween.ts | 2 +- .../src/services/populations/shared.ts | 29 +++++++++++-------- services/backend/src/services/students.ts | 12 ++++++-- services/backend/src/types/enrollmentState.ts | 6 ---- .../StudentTable/CoursesTab/index.jsx | 6 ++-- 9 files changed, 48 insertions(+), 40 deletions(-) diff --git a/services/backend/src/services/completedCoursesSearch.ts b/services/backend/src/services/completedCoursesSearch.ts index a20a826a08..9ae1b82b58 100644 --- a/services/backend/src/services/completedCoursesSearch.ts +++ b/services/backend/src/services/completedCoursesSearch.ts @@ -15,7 +15,6 @@ interface StudentCredit { interface StudentEnrollment { date: Date courseCode: string - state: EnrollmentState substitution: string | null } @@ -95,9 +94,9 @@ const getCredits = async ( } const getEnrollments = async (courses: Courses, fullCourseCodes: string[], studentNumbers: string[]) => { - const enrollments: Array> = + const enrollments: Array> = await Enrollment.findAll({ - attributes: ['course_code', 'enrollment_date_time', 'studentnumber', 'state'], + attributes: ['course_code', 'enrollment_date_time', 'studentnumber'], where: { course_code: { [Op.in]: fullCourseCodes, @@ -117,7 +116,6 @@ const getEnrollments = async (courses: Courses, fullCourseCodes: string[], stude substitution: originalCode ? enrollment.course_code : null, studentNumber: enrollment.studentnumber, date: enrollment.enrollment_date_time, - state: enrollment.state, } }) return formattedEnrollments @@ -201,7 +199,6 @@ export const getCompletedCourses = async (studentNumbers: string[], courseCodes: studentCredits[enrollment.studentNumber].enrollments.push({ date: enrollment.date, courseCode: enrollment.courseCode, - state: enrollment.state, substitution: enrollment.substitution, }) }) diff --git a/services/backend/src/services/courses/index.ts b/services/backend/src/services/courses/index.ts index 3750683143..1406b4ee5a 100644 --- a/services/backend/src/services/courses/index.ts +++ b/services/backend/src/services/courses/index.ts @@ -2,7 +2,7 @@ import crypto from 'crypto' import { Op } from 'sequelize' import { Course, Credit, Enrollment, Organization, SISStudyRightElement } from '../../models' -import { Name, Unification } from '../../types' +import { EnrollmentState, Name, Unification } from '../../types' import { isOpenUniCourseCode } from '../../util' import { getSortRank } from '../../util/sortRank' import { CourseYearlyStatsCounter } from './courseYearlyStatsCounter' @@ -317,7 +317,12 @@ export const getCourseYearlyStats = async ( }) const enrollments = await Enrollment.findAll({ attributes: ['studentnumber'], - where: { course_code: { [Op.in]: courseCodes } }, + where: { + course_code: { + [Op.in]: courseCodes, + }, + state: EnrollmentState.ENROLLED, + }, }) const studentNumbers = {} diff --git a/services/backend/src/services/openUni/openUniSearches.ts b/services/backend/src/services/openUni/openUniSearches.ts index 5b278e4a12..4f8f6d86ca 100644 --- a/services/backend/src/services/openUni/openUniSearches.ts +++ b/services/backend/src/services/openUni/openUniSearches.ts @@ -2,7 +2,7 @@ import { InferAttributes, Op, WhereOptions } from 'sequelize' import { Course, Credit, Enrollment, Student, SISStudyRight } from '../../models' import { OpenUniPopulationSearch } from '../../models/kone' -import { ExtentCode } from '../../types' +import { EnrollmentState, ExtentCode } from '../../types' import { formatCourseInfo, formatOpenCredits, formatOpenEnrollments, formatStudentInfo } from './format' export const getCredits = async (courseCodes: string[], startdate: Date) => @@ -47,6 +47,7 @@ export const getEnrollments = async (courseCodes: string[], startDate: Date, end [Op.gte]: startDate, }, }, + state: EnrollmentState.ENROLLED, }, }) ).map(formatOpenEnrollments) diff --git a/services/backend/src/services/populations/bottlenecksOf.ts b/services/backend/src/services/populations/bottlenecksOf.ts index c17ac482b2..850eeb1f3e 100644 --- a/services/backend/src/services/populations/bottlenecksOf.ts +++ b/services/backend/src/services/populations/bottlenecksOf.ts @@ -11,7 +11,8 @@ import { parseCreditInfo, parseQueryParams, Query, - QueryResult, + CoursesQueryResult, + EnrollmentsQueryResult, } from './shared' import { getStudentNumbersWithAllStudyRightElements } from './studentNumbersWithAllElements' @@ -20,7 +21,7 @@ const getStudentsAndCourses = async ( selectedStudents: string[] | undefined, studentNumbers: string[] | null, courseCodes: string[] | undefined -) => { +): Promise<[number, CoursesQueryResult, EnrollmentsQueryResult]> => { if (!studentNumbers) { const { months, studyRights, startDate, endDate, exchangeStudents, nondegreeStudents, transferredStudents } = params const studentnumbers = @@ -89,10 +90,10 @@ export const bottlenecksOf = async (query: Query, studentNumbers: string[] | nul // To fix failed and enrolled, no grade filter options some not so clean and nice solutions were added // Get the data with actual 1. courses and filtered students. 2. all students by year, if provided. - const [[allStudents, courses, courseEnrollements], [, allCourses]] = (await Promise.all([ + const [[allStudents, courses, courseEnrollements], [, allCourses]] = await Promise.all([ getStudentsAndCourses(params, query.selectedStudents, studentNumbers, query.courses), getStudentsAndCourses(params, allStudentsByYears, null, query.courses), - ])) as [[number, QueryResult, QueryResult], [number, QueryResult]] + ]) // Get the substitution codes for the fetch data by selected students const substitutionCodes = Object.entries(courses).reduce( @@ -112,7 +113,8 @@ export const bottlenecksOf = async (query: Query, studentNumbers: string[] | nul const stats = {} as Record const startYear = parseInt(query.year, 10) - let coursesToLoop = courses.concat(substitutionCourses) + let coursesToLoop: Array = + courses.concat(substitutionCourses) const courseCodes = coursesToLoop.map(course => course.code) // This fixes a problem when "Enrolled, no grade" is chosen. The SQL query for fetching @@ -146,7 +148,7 @@ export const bottlenecksOf = async (query: Query, studentNumbers: string[] | nul } }) } - if (course.credits) { + if ('credits' in course) { course.credits.forEach(credit => { const { studentnumber, passingGrade, improvedGrade, failingGrade, grade, date } = parseCreditInfo(credit) if ((query?.selectedStudents && query?.selectedStudents.includes(studentnumber)) || !query?.selectedStudents) { diff --git a/services/backend/src/services/populations/getStudentsIncludeCoursesBetween.ts b/services/backend/src/services/populations/getStudentsIncludeCoursesBetween.ts index f2a690b165..d07879ed0f 100644 --- a/services/backend/src/services/populations/getStudentsIncludeCoursesBetween.ts +++ b/services/backend/src/services/populations/getStudentsIncludeCoursesBetween.ts @@ -85,7 +85,7 @@ const getEnrollments = async (studentNumbers: string[], attainmentDateFrom: mome studentnumber: { [Op.in]: studentNumbers, }, - state: [EnrollmentState.ENROLLED, EnrollmentState.CONFIRMED], + state: EnrollmentState.ENROLLED, }, raw: true, }) diff --git a/services/backend/src/services/populations/shared.ts b/services/backend/src/services/populations/shared.ts index c87e7d201b..be18e1120d 100644 --- a/services/backend/src/services/populations/shared.ts +++ b/services/backend/src/services/populations/shared.ts @@ -6,7 +6,7 @@ import { Op, QueryTypes } from 'sequelize' import { dbConnections } from '../../database/connection' import { Course, Credit, Enrollment, SISStudyRight, SISStudyRightElement, Student, Studyplan } from '../../models' import { Tag, TagStudent } from '../../models/kone' -import { Criteria, DegreeProgrammeType, Name, ParsedCourse } from '../../types' +import { Criteria, DegreeProgrammeType, EnrollmentState, Name, ParsedCourse } from '../../types' import { SemesterStart } from '../../util/semester' import { hasTransferredFromOrToProgramme } from '../studyProgramme/studyProgrammeHelpers' @@ -480,21 +480,26 @@ const getSubstitutions = async (codes: string[]) => { return substitutions } -export type QueryResult = Array<{ +export type EnrollmentsQueryResult = Array<{ code: string name: Name - coursetypecode: string substitutions: string[] main_course_code: string - course_type: Name - credits: Array> enrollments: Array> | null }> +type CreditPick = Pick + +export type CoursesQueryResult = Array< + EnrollmentsQueryResult[number] & { + credits: Array + } +> + // This duplicate code is added here to ensure that we get the enrollments in cases no credits found for the selected students export const findCourseEnrollments = async (studentNumbers: string[], beforeDate: Date, courses: string[] = []) => { const courseCodes = courses.length === 0 ? ['DUMMY'] : await getSubstitutions(courses) - const result: QueryResult = await sequelize.query( + const result: EnrollmentsQueryResult = await sequelize.query( ` SELECT DISTINCT ON (course.id) course.code, @@ -512,7 +517,7 @@ export const findCourseEnrollments = async (studentNumbers: string[], beforeDate 'enrollment_date_time', enrollment_date_time )) AS data FROM enrollment - WHERE enrollment.studentnumber IN (:studentnumbers) AND enrollment.enrollment_date_time < :beforeDate + WHERE enrollment.studentnumber IN (:studentnumbers) AND enrollment.enrollment_date_time < :beforeDate AND enrollment.state = :enrollmentState GROUP BY enrollment.course_id ) enrollment ON enrollment.course_id = course.id WHERE :skipCourseCodeFilter OR course.code IN (:courseCodes) @@ -523,6 +528,7 @@ export const findCourseEnrollments = async (studentNumbers: string[], beforeDate beforeDate, courseCodes, skipCourseCodeFilter: courses.length === 0, + enrollmentState: EnrollmentState.ENROLLED, }, type: QueryTypes.SELECT, } @@ -532,7 +538,7 @@ export const findCourseEnrollments = async (studentNumbers: string[], beforeDate export const findCourses = async (studentNumbers: string[], beforeDate: Date, courses: string[] = []) => { const courseCodes = courses.length === 0 ? ['DUMMY'] : await getSubstitutions(courses) - const result: QueryResult = await sequelize.query( + const result: CoursesQueryResult = await sequelize.query( ` SELECT DISTINCT ON (course.id) course.code, @@ -565,7 +571,7 @@ export const findCourses = async (studentNumbers: string[], beforeDate: Date, co 'enrollment_date_time', enrollment_date_time )) AS data FROM enrollment - WHERE enrollment.studentnumber IN (:studentnumbers) AND enrollment.enrollment_date_time < :beforeDate + WHERE enrollment.studentnumber IN (:studentnumbers) AND enrollment.enrollment_date_time < :beforeDate AND enrollment.state = :enrollmentState GROUP BY enrollment.course_id ) enrollment ON enrollment.course_id = course.id WHERE :skipCourseCodeFilter OR course.code IN (:courseCodes) @@ -576,6 +582,7 @@ export const findCourses = async (studentNumbers: string[], beforeDate: Date, co beforeDate, courseCodes, skipCourseCodeFilter: courses.length === 0, + enrollmentState: EnrollmentState.ENROLLED, }, type: QueryTypes.SELECT, } @@ -583,9 +590,7 @@ export const findCourses = async (studentNumbers: string[], beforeDate: Date, co return result } -export const parseCreditInfo = ( - credit: Pick -) => { +export const parseCreditInfo = (credit: CreditPick) => { return { studentnumber: credit.student_studentnumber, grade: credit.grade, diff --git a/services/backend/src/services/students.ts b/services/backend/src/services/students.ts index 3e32be99bd..a285be92e0 100644 --- a/services/backend/src/services/students.ts +++ b/services/backend/src/services/students.ts @@ -12,7 +12,7 @@ import { SISStudyRightElement, } from '../models' import { Tag, TagStudent } from '../models/kone' -import { UnifyStatus } from '../types' +import { EnrollmentState, UnifyStatus } from '../types' import { splitByEmptySpace } from '../util' import logger from '../util/logger' @@ -182,11 +182,17 @@ export const findByCourseAndSemesters = async ( AND e.course_code IN (:coursecodes) AND e.semestercode BETWEEN ${fromSemester} AND ${toSemester} AND e.enrollment_date_time >= '2021-05-31' - AND e.state IN ('ENROLLED', 'CONFIRMED') + AND e.state = :enrollmentState ); `, { - replacements: { coursecodes, minYearCode: from, maxYearCode: to, isOpen: unifyStatus }, + replacements: { + coursecodes, + minYearCode: from, + maxYearCode: to, + isOpen: unifyStatus, + enrollmentState: EnrollmentState.ENROLLED, + }, type: QueryTypes.SELECT, raw: true, } diff --git a/services/backend/src/types/enrollmentState.ts b/services/backend/src/types/enrollmentState.ts index 1585d19300..d4288f1fd2 100644 --- a/services/backend/src/types/enrollmentState.ts +++ b/services/backend/src/types/enrollmentState.ts @@ -1,10 +1,4 @@ export enum EnrollmentState { - ABORTED_BY_STUDENT = 'ABORTED_BY_STUDENT', - ABORTED_BY_TEACHER = 'ABORTED_BY_TEACHER', - CONFIRMED = 'CONFIRMED', // ? Does this state actually exist anymore? ENROLLED = 'ENROLLED', - INVALID = 'INVALID', - NOT_ENROLLED = 'NOT_ENROLLED', - PROCESSING = 'PROCESSING', REJECTED = 'REJECTED', } diff --git a/services/frontend/src/components/PopulationStudents/StudentTable/CoursesTab/index.jsx b/services/frontend/src/components/PopulationStudents/StudentTable/CoursesTab/index.jsx index 8f59ba713e..e6b935f3e6 100644 --- a/services/frontend/src/components/PopulationStudents/StudentTable/CoursesTab/index.jsx +++ b/services/frontend/src/components/PopulationStudents/StudentTable/CoursesTab/index.jsx @@ -229,7 +229,7 @@ const CoursesTable = ({ curriculum, includeSubstitutions, populationCourses, stu if (!student.enrollments) { return false } - return student.enrollments.some(enrollment => enrollment.course_code === code && enrollment.state === 'ENROLLED') + return student.enrollments.some(enrollment => enrollment.course_code === code) } const getCompletionDate = (student, code) => { @@ -238,9 +238,7 @@ const CoursesTable = ({ curriculum, includeSubstitutions, populationCourses, stu } const getEnrollmentDate = (student, code) => { - const enrollment = student.enrollments.find( - enrollment => enrollment.course_code === code && enrollment.state === 'ENROLLED' - ) + const enrollment = student.enrollments.find(enrollment => enrollment.course_code === code) return enrollment.enrollment_date_time }