Merge pull request #4623 from UniversityOfHelsinkiCS/type-bottlenecks

Type bottlenecks

.github/workflows/production.yaml

@@ -146,7 +146,7 @@ jobs:
           version: ${{ steps.extract_version.outputs.VERSION }}
 
   report_success:
-    name: 'Report deployment success to oodikone notifications'
+    name: 'Report deployment success to the "oodikone_notifications" channel'
     runs-on: ubuntu-latest
     needs:
       - release_frontend
@@ -158,7 +158,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-      - name: 'Send notification to slack'
+      - name: 'Send notification to Slack'
         uses: UniversityOfHelsinkiCS/slackbot-action@v1.5
         with:
           webhook-url: ${{ secrets.WEBHOOK_URL }}
@@ -167,7 +167,7 @@ jobs:
           deployment-target: production
 
   report_failure:
-    name: 'Report deployment failure to oodikone notifications'
+    name: 'Report deployment failure to the "oodikone_notifications" channel'
     runs-on: ubuntu-latest
     needs:
       - release_frontend
@@ -179,7 +179,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-      - name: 'Send notification to slack'
+      - name: 'Send notification to Slack'
         uses: UniversityOfHelsinkiCS/slackbot-action@v1.5
         with:
           webhook-url: ${{ secrets.WEBHOOK_URL }}

.github/workflows/staging.yaml

@@ -5,16 +5,16 @@ on:
     branches:
       - master
     paths:
-      - '.github/workflows/test.yaml'
-      - '.github/workflows/staging.yaml'
       - '.github/actions/**'
+      - '.github/workflows/staging.yaml'
+      - '.github/workflows/test.yaml'
       - 'cypress/**'
       - 'cypress.config.json'
       - 'services/**'
       - '!services/backend/src/rapodiff/**'
+      - 'updater/sis-updater-worker/src/db/migrations/**'
       - 'docker-compose.ci.yml'
       - 'package*.json'
-      - 'updater/sis-updater-worker/src/db/migrations/**'
 
 jobs:
   test:
@@ -90,7 +90,7 @@ jobs:
           password: ${{ secrets.QUAY_IO_TOKEN }}
 
   report_success:
-    name: 'Report deployment success to oodikone notifications'
+    name: 'Report deployment success to the "oodikone_notifications" channel'
     runs-on: ubuntu-latest
     needs:
       - release_frontend
@@ -100,7 +100,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-      - name: 'Send notification to slack'
+      - name: 'Send notification to Slack'
         uses: UniversityOfHelsinkiCS/slackbot-action@v1.5
         with:
           webhook-url: ${{ secrets.WEBHOOK_URL }}
@@ -109,7 +109,7 @@ jobs:
           deployment-target: staging
 
   report_failure:
-    name: 'Report deployment failure to oodikone notifications'
+    name: 'Report deployment failure to the "oodikone_notifications" channel'
     runs-on: ubuntu-latest
     needs:
       - release_frontend
@@ -119,7 +119,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-      - name: 'Send notification to slack'
+      - name: 'Send notification to Slack'
         uses: UniversityOfHelsinkiCS/slackbot-action@v1.5
         with:
           webhook-url: ${{ secrets.WEBHOOK_URL }}

services/backend/src/routes/population.ts

@@ -96,7 +96,7 @@ router.post('/v2/populationstatistics/courses', async (req: PostPopulationStatis
             : req.body.selectedStudentsByYear[year]
         }
         const newMonths = (upperYearBound - Number(year)) * 12
-        const query = { ...req.body, year, months: newMonths }
+        const query = { ...req.body, year, months: newMonths, selectedStudents: req.body.selectedStudents as string[] }
         const coursestatistics = bottlenecksOf(query, null, encrypted)
         return coursestatistics
       })
@@ -114,13 +114,14 @@ router.post('/v2/populationstatistics/courses', async (req: PostPopulationStatis
     req.body.selectedStudents = req.body.selectedStudents.map(decrypt)
   }
 
-  const result = await bottlenecksOf(req.body, null, encrypted)
-  if (result.error) {
-    Sentry.captureException(new Error(result.error))
-    return res.status(400).json(result)
+  const query = { ...req.body, selectedStudents: req.body.selectedStudents as string[] }
+  try {
+    const result = await bottlenecksOf(query, null, encrypted)
+    return res.json(result)
+  } catch (error: any) {
+    Sentry.captureException(new Error(error.message))
+    return res.status(400).end()
   }
-
-  return res.json(result)
 })
 
 interface PostPopulationStatisticsByTagRequest extends Request {
@@ -135,8 +136,7 @@ router.post(
     const { roles, studentsUserCanAccess } = req.user
     const { tag } = req.body
     if (!tag) {
-      res.status(400).json({ error: 'The body should have a tag defined' })
-      return
+      return res.status(400).json({ error: 'The body should have a tag defined' })
     }
 
     const studentNumbers = await findByTag(tag)
@@ -144,24 +144,22 @@ router.post(
       ? studentNumbers
       : intersection(studentNumbers, studentsUserCanAccess)
 
-    const result = await bottlenecksOf(
-      {
-        year: 1900,
-        studyRights: [],
-        semesters: ['FALL', 'SPRING'],
-        months: 10000,
-        tag,
-      },
-      studentNumberList
-    )
-
-    if (result.error) {
-      Sentry.captureException(new Error(result.error))
-      res.status(400).end()
-      return
+    try {
+      const result = await bottlenecksOf(
+        {
+          year: '1900',
+          studyRights: [],
+          semesters: ['FALL', 'SPRING'],
+          months: 10000,
+          tag,
+        },
+        studentNumberList
+      )
+      return res.json(result)
+    } catch (error: any) {
+      Sentry.captureException(new Error(error.message))
+      return res.status(400).end()
     }
-
-    res.json(result)
   }
 )
 
@@ -190,22 +188,22 @@ router.post(
     const studentNumbers = hasFullAccessToStudentData(roles)
       ? studentnumberlist
       : intersection(studentnumberlist, studentsUserCanAccess)
-    const result = await bottlenecksOf(
-      {
-        year: query?.year ?? 1900,
-        studyRights: query?.studyRights ?? [],
-        semesters: query?.semesters ?? ['FALL', 'SPRING'],
-        months: query?.months ?? 10000,
-      },
-      studentNumbers
-    )
-    if (result.error) {
-      Sentry.captureException(new Error(result.error))
-      res.status(400).json(result)
-      return
-    }
 
-    res.json(result)
+    try {
+      const result = await bottlenecksOf(
+        {
+          year: query?.year ?? 1900,
+          studyRights: query?.studyRights ?? [],
+          semesters: query?.semesters ?? ['FALL', 'SPRING'],
+          months: query?.months ?? 10000,
+        },
+        studentNumbers
+      )
+      return res.json(result)
+    } catch (error: any) {
+      Sentry.captureException(new Error(error.message))
+      return res.status(400).end()
+    }
   }
 )
 

services/backend/src/services/courses/courseStatsCounter.ts

@@ -53,11 +53,59 @@ type DynamicEnrollments = {
   }
 }
 
+export type CourseStatistics = {
+  stats: {
+    students: number
+    passed: number
+    failed: number
+    failedMany: number
+    retryPassed: number
+    attempts: number
+    improvedPassedGrade: number
+    percentage: number | undefined
+    passedOfPopulation: number | undefined
+    triedOfPopulation: number | undefined
+    perStudent: number | undefined
+    passingSemesters: Record<string, number>
+    passingSemestersCumulative?: Record<string, number>
+    totalStudents?: number
+    totalEnrolledNoGrade?: number
+    percentageWithEnrollments?: number
+  }
+  students: {
+    all: Record<string, boolean>
+    passed: Record<string, boolean>
+    failed: Record<string, boolean>
+    retryPassed: Record<string, boolean>
+    failedMany: Record<string, boolean>
+    improvedPassedGrade: Record<string, boolean>
+    markedToSemester: Record<string, boolean>
+    enrolledNoGrade: Record<string, boolean>
+  }
+  course: {
+    code: string
+    name: Name
+    disciplines: object
+    coursetypes: Record<string, Name>
+    substitutions: string[]
+  }
+  grades: {
+    [grade: string]: {
+      count: number
+      status: {
+        passingGrade: boolean
+        improvedGrade: boolean
+        failingGrade: boolean
+      }
+    }
+  }
+}
+
 export class CourseStatsCounter {
   private studentsInTotal: number
   private course: {
     code: string
-    name: string
+    name: Name
     disciplines: object
     coursetypes: Record<string, Name>
     substitutions: string[]
@@ -102,7 +150,7 @@ export class CourseStatsCounter {
     }
   }
 
-  constructor(code: string, name: string, studentsInTotal: number) {
+  constructor(code: string, name: Name, studentsInTotal: number) {
     this.studentsInTotal = studentsInTotal
     this.course = {
       code,
@@ -289,6 +337,6 @@ export class CourseStatsCounter {
     stats.totalEnrolledNoGrade = lengthOf(filteredEnrolledNoGrade)
     stats.percentageWithEnrollments = percentageOf(stats.passed, stats.totalStudents)
 
-    return { stats, students, course: this.course, grades: this.grades }
+    return { stats, students, course: this.course, grades: this.grades } as CourseStatistics
   }
 }

services/backend/src/services/populations/bottlenecksOf.ts

@@ -1,15 +1,29 @@
-const { keyBy } = require('lodash')
+import { keyBy } from 'lodash'
 
-const { getPassingSemester } = require('../../util/semester')
-const { CourseStatsCounter } = require('../courses/courseStatsCounter')
-const { encrypt } = require('../encrypt')
-const { dateMonthsFromNow, findCourses, findCourseEnrollments, parseCreditInfo, parseQueryParams } = require('./shared')
-const { getStudentNumbersWithAllStudyRightElements } = require('./studentNumbersWithAllElements')
+import { Name } from '../../types'
+import { getPassingSemester } from '../../util/semester'
+import { CourseStatistics, CourseStatsCounter } from '../courses/courseStatsCounter'
+import { encrypt } from '../encrypt'
+import {
+  dateMonthsFromNow,
+  findCourses,
+  findCourseEnrollments,
+  Params,
+  parseCreditInfo,
+  parseQueryParams,
+  Query,
+  QueryResult,
+} from './shared'
+import { getStudentNumbersWithAllStudyRightElements } from './studentNumbersWithAllElements'
 
-const getStudentsAndCourses = async (params, selectedStudents, studentNumbers, courseCodes) => {
+const getStudentsAndCourses = async (
+  params: Params,
+  selectedStudents: string[] | undefined,
+  studentNumbers: string[] | null,
+  courseCodes: string[] | undefined
+) => {
   if (!studentNumbers) {
-    const { months, studyRights, startDate, endDate, exchangeStudents, nondegreeStudents, transferredStudents, tag } =
-      params
+    const { months, studyRights, startDate, endDate, exchangeStudents, nondegreeStudents, transferredStudents } = params
     const studentnumbers =
       selectedStudents ||
       (await getStudentNumbersWithAllStudyRightElements({
@@ -19,9 +33,6 @@ const getStudentsAndCourses = async (params, selectedStudents, studentNumbers, c
         exchangeStudents,
         nondegreeStudents,
         transferredOutStudents: transferredStudents,
-        tag,
-        transferredToStudents: true,
-        graduatedStudents: true,
       }))
 
     const allStudents = studentnumbers.length
@@ -48,8 +59,15 @@ const getStudentsAndCourses = async (params, selectedStudents, studentNumbers, c
   return [allStudents, courses, courseEnrollments]
 }
 
-const bottlenecksOf = async (query, studentNumbers, encryptData = false) => {
-  const encryptStudentNumbers = bottlenecks => {
+type Bottlenecks = {
+  disciplines: Record<string, string>
+  coursetypes: Record<string, Name>
+  coursestatistics: CourseStatistics[]
+  allStudents: number
+}
+
+export const bottlenecksOf = async (query: Query, studentNumbers: string[] | null, encryptData: boolean = false) => {
+  const encryptStudentNumbers = (bottlenecks: Bottlenecks) => {
     for (const course of Object.keys(bottlenecks.coursestatistics)) {
       const encryptedStudentStats = {}
       for (const data of Object.keys(bottlenecks.coursestatistics[course].students)) {
@@ -67,22 +85,22 @@ const bottlenecksOf = async (query, studentNumbers, encryptData = false) => {
   const params = parseQueryParams(query)
   const allStudentsByYears = query?.selectedStudentsByYear
     ? Object.keys(query.selectedStudentsByYear).reduce(
-        (res, year) => [...res, ...query.selectedStudentsByYear[year]],
-        []
+        (res, year) => [...res, ...query.selectedStudentsByYear![year]],
+        [] as string[]
       )
     : []
 
   // 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(
     (codes, [, course]) => [...codes, ...(course?.substitutions || [])],
-    []
+    [] as string[]
   )
   const codes = Object.keys(keyBy(courses, 'code')).map(code => code)
   // Filter substitution courses for fetched courses -> by this we avoid the situation in which there
@@ -90,12 +108,14 @@ const bottlenecksOf = async (query, studentNumbers, encryptData = false) => {
   const substitutionCourses = allCourses.filter(
     course => substitutionCodes.includes(course.code) && !codes.includes(course.code)
   )
-  const bottlenecks = {
+  const bottlenecks: Bottlenecks = {
     disciplines: {},
     coursetypes: {},
+    coursestatistics: [],
+    allStudents: 0,
   }
 
-  const stats = {}
+  const stats = {} as Record<string, CourseStatsCounter>
   const startYear = parseInt(query.year, 10)
   let coursesToLoop = courses.concat(substitutionCourses)
   const courseCodes = coursesToLoop.map(course => course.code)
@@ -110,7 +130,6 @@ const bottlenecksOf = async (query, studentNumbers, encryptData = false) => {
 
   const coursesByCode = keyBy(coursesToLoop, 'code')
   for (const course of coursesToLoop) {
-    const { course_type } = course
     let mainCourse = course
 
     if (course.main_course_code && course.main_course_code !== course.code) {
@@ -123,15 +142,16 @@ const bottlenecksOf = async (query, studentNumbers, encryptData = false) => {
     if (!stats[mainCourse.code]) {
       stats[mainCourse.code] = new CourseStatsCounter(mainCourse.code, mainCourse.name, allStudents)
     }
+
     const coursestats = stats[mainCourse.code]
-    coursestats.addCourseType(course_type.coursetypecode, course_type.name)
+    coursestats.addCourseType(course.coursetypecode, course.name)
     coursestats.addCourseSubstitutions(course.substitutions)
-    bottlenecks.coursetypes[course_type.coursetypecode] = course_type.name
+    bottlenecks.coursetypes[course.coursetypecode] = course.name
     if (course.enrollments) {
       course.enrollments.forEach(({ studentnumber, state, enrollment_date_time }) => {
         if ((query?.selectedStudents && query?.selectedStudents.includes(studentnumber)) || !query?.selectedStudents) {
           const semester = getPassingSemester(startYear, enrollment_date_time)
-          coursestats.markEnrollment(studentnumber, state, semester, enrollment_date_time)
+          coursestats.markEnrollment(studentnumber, state, semester)
         }
       })
     }
@@ -157,7 +177,3 @@ const bottlenecksOf = async (query, studentNumbers, encryptData = false) => {
 
   return bottlenecks
 }
-
-module.exports = {
-  bottlenecksOf,
-}

services/backend/src/services/populations/optimizedStatisticsOf.ts

@@ -8,6 +8,7 @@ import {
   formatStudentsForApi,
   getOptionsForStudents,
   parseQueryParams,
+  Query as FormattedQueryParams,
 } from './shared'
 import { getStudentNumbersWithAllStudyRightElements } from './studentNumbersWithAllElements'
 
@@ -34,7 +35,7 @@ export const optimizedStatisticsOf = async (query: Query, studentNumberList?: st
   }
 
   const { studyRights, startDate, months, endDate, exchangeStudents, nondegreeStudents, transferredStudents, tag } =
-    parseQueryParams(formattedQueryParams)
+    parseQueryParams(formattedQueryParams as FormattedQueryParams)
 
   const studentNumbers =
     studentNumberList ||

services/backend/src/services/populations/shared.ts

@@ -5,7 +5,7 @@ import { Op, QueryTypes } from 'sequelize'
 
 import { dbConnections } from '../../database/connection'
 import { Course, Credit, Enrollment, SISStudyRight, SISStudyRightElement, Student, Studyplan } from '../../models'
-import { TagStudent } from '../../models/kone'
+import { Tag, TagStudent } from '../../models/kone'
 import { Criteria, DegreeProgrammeType, Name, ParsedCourse } from '../../types'
 import { SemesterStart } from '../../util/semester'
 import { hasTransferredFromOrToProgramme } from '../studyProgramme/studyProgrammeHelpers'
@@ -286,7 +286,31 @@ export const dateMonthsFromNow = (date: string, months: number) => {
   return new Date(moment(date).add(months, 'months').format('YYYY-MM-DD'))
 }
 
-export const parseQueryParams = query => {
+export type Query = {
+  year: string
+  months: number
+  semesters: string[]
+  studyRights: string | string[]
+  selectedStudents?: string[]
+  selectedStudentsByYear?: Record<string, string[]>
+  studentStatuses?: string[]
+  years?: string[]
+  tag?: string
+  courses?: string[]
+}
+
+export type Params = {
+  startDate: string
+  endDate: string
+  exchangeStudents: boolean
+  nondegreeStudents: boolean
+  transferredStudents: boolean
+  studyRights: string[]
+  months: number
+  tag?: Tag
+}
+
+export const parseQueryParams = (query: Query) => {
   const { semesters, studentStatuses, studyRights, months, year, tag } = query
   const hasFall = semesters.includes('FALL')
   const hasSpring = semesters.includes('SPRING')
@@ -313,7 +337,7 @@ export const parseQueryParams = query => {
     startDate,
     endDate,
     tag,
-  }
+  } as Params
 }
 
 export const getOptionsForStudents = async (
@@ -462,10 +486,21 @@ const getCourseCodes = async (courses: string[]) => {
   return courseCodes
 }
 
+export type QueryResult = Array<{
+  code: string
+  name: Name
+  coursetypecode: string
+  substitutions: string[]
+  main_course_code: string
+  course_type: Name
+  credits: Array<Pick<Credit, 'grade' | 'student_studentnumber' | 'attainment_date' | 'credittypecode' | 'course_code'>>
+  enrollments: Array<Pick<Enrollment, 'studentnumber' | 'state' | 'enrollment_date_time'>> | null
+}>
+
 // 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 = await getCourseCodes(courses)
-  const res = await sequelize.query(
+  const result: QueryResult = await sequelize.query(
     `
       SELECT DISTINCT ON (course.id)
         course.code,
@@ -502,12 +537,12 @@ export const findCourseEnrollments = async (studentNumbers: string[], beforeDate
       type: QueryTypes.SELECT,
     }
   )
-  return res
+  return result
 }
 
 export const findCourses = async (studentNumbers: string[], beforeDate: Date, courses: string[] = []) => {
   const courseCodes = await getCourseCodes(courses)
-  const res = await sequelize.query(
+  const result: QueryResult = await sequelize.query(
     `
       SELECT DISTINCT ON (course.id)
         course.code,
@@ -559,15 +594,18 @@ export const findCourses = async (studentNumbers: string[], beforeDate: Date, co
       type: QueryTypes.SELECT,
     }
   )
-
-  return res
+  return result
 }
 
-export const parseCreditInfo = (credit: Credit) => ({
-  studentnumber: credit.student_studentnumber,
-  grade: credit.grade,
-  passingGrade: Credit.passed(credit),
-  failingGrade: Credit.failed(credit),
-  improvedGrade: Credit.improved(credit),
-  date: credit.attainment_date,
-})
+export const parseCreditInfo = (
+  credit: Pick<Credit, 'grade' | 'student_studentnumber' | 'attainment_date' | 'credittypecode' | 'course_code'>
+) => {
+  return {
+    studentnumber: credit.student_studentnumber,
+    grade: credit.grade,
+    passingGrade: Credit.passed(credit),
+    failingGrade: Credit.failed(credit),
+    improvedGrade: Credit.improved(credit),
+    date: credit.attainment_date,
+  }
+}