[Kone] Migration step 2 of 2

- Remove old fields from db
- Not null enforced
- Remove dead code

services/backend/src/app.ts

@@ -4,8 +4,6 @@ import { baseUrl, backendPort } from './config'
 import { initializeDatabaseConnection, dbConnections } from './database/connection'
 import { startCron } from './events'
 import routes from './routes'
-import { backfillExcludedCourseGroupIds } from './services/excludedCourses'
-import { backfillProgressCriteriaGroupIds } from './services/studyProgramme/studyProgrammeCriteria'
 import logger from './util/logger'
 import './worker/worker'
 
@@ -15,8 +13,6 @@ initializeDatabaseConnection()
 
     dbConnections.on('connect', () => {
       logger.info('Connected to sis db successfully')
-      backfillExcludedCourseGroupIds().catch(error => logger.error(error))
-      backfillProgressCriteriaGroupIds().catch(error => logger.error(error))
     })
 
     dbConnections.on('error', () => {

services/backend/src/database/migrations_kone/20260929_02_drop_course_codes_from_kone_tables.js

@@ -0,0 +1,94 @@
+/* eslint-disable no-console */
+const { ARRAY, STRING } = require('sequelize')
+
+const YEARS = ['one', 'two', 'three', 'four', 'five', 'six']
+const UNIQUE_CONSTRAINT = 'excluded_courses_programme_version_group_key'
+
+// Migration step 2 of 2
+// Rows the backfill could not map to a course group id are removed, as they no longer reference a course.
+// NOTE: down only restores the schema, the dropped course codes cannot be restored.
+module.exports = {
+  up: async ({ context: queryInterface }) => {
+    await queryInterface.sequelize.transaction(async transaction => {
+      const [, unmapped] = await queryInterface.sequelize.query(
+        'DELETE FROM excluded_courses WHERE course_group_id IS NULL',
+        { transaction }
+      )
+      console.log(`Deleted ${unmapped.rowCount} excluded courses without a course group id`)
+
+      const [, duplicates] = await queryInterface.sequelize.query(
+        `DELETE FROM excluded_courses a
+          USING excluded_courses b
+          WHERE a.id > b.id
+            AND a.programme_code = b.programme_code
+            AND a.curriculum_version IS NOT DISTINCT FROM b.curriculum_version
+            AND a.course_group_id = b.course_group_id`,
+        { transaction }
+      )
+      console.log(`Deleted ${duplicates.rowCount} duplicate excluded courses`)
+
+      await queryInterface.removeColumn('excluded_courses', 'course_code', { transaction })
+      await queryInterface.changeColumn(
+        'excluded_courses',
+        'course_group_id',
+        { type: STRING, allowNull: false },
+        { transaction }
+      )
+      await queryInterface.addConstraint('excluded_courses', {
+        fields: ['programme_code', 'curriculum_version', 'course_group_id'],
+        type: 'unique',
+        name: UNIQUE_CONSTRAINT,
+        transaction,
+      })
+
+      for (const year of YEARS) {
+        const [[{ count }]] = await queryInterface.sequelize.query(
+          `SELECT COUNT(*) AS count FROM progress_criteria
+            WHERE course_group_ids_year_${year} IS NULL AND cardinality(courses_year_${year}) > 0`,
+          { transaction }
+        )
+        console.log(`Emptied year ${year} of ${count} progress criteria without course group ids`)
+
+        await queryInterface.sequelize.query(
+          `UPDATE progress_criteria SET course_group_ids_year_${year} = '{}' WHERE course_group_ids_year_${year} IS NULL`,
+          { transaction }
+        )
+        await queryInterface.removeColumn('progress_criteria', `courses_year_${year}`, { transaction })
+        await queryInterface.changeColumn(
+          'progress_criteria',
+          `course_group_ids_year_${year}`,
+          { type: ARRAY(STRING), allowNull: false, defaultValue: [] },
+          { transaction }
+        )
+      }
+    })
+  },
+
+  down: async ({ context: queryInterface }) => {
+    await queryInterface.sequelize.transaction(async transaction => {
+      await queryInterface.removeConstraint('excluded_courses', UNIQUE_CONSTRAINT, { transaction })
+      await queryInterface.changeColumn(
+        'excluded_courses',
+        'course_group_id',
+        { type: STRING, allowNull: true },
+        { transaction }
+      )
+      await queryInterface.addColumn('excluded_courses', 'course_code', { type: STRING }, { transaction })
+
+      for (const year of YEARS) {
+        await queryInterface.changeColumn(
+          'progress_criteria',
+          `course_group_ids_year_${year}`,
+          { type: ARRAY(STRING), allowNull: true, defaultValue: null },
+          { transaction }
+        )
+        await queryInterface.addColumn(
+          'progress_criteria',
+          `courses_year_${year}`,
+          { type: ARRAY(STRING) },
+          { transaction }
+        )
+      }
+    })
+  },
+}

services/backend/src/models/kone/excludedCourse.ts

@@ -16,9 +16,6 @@ export class ExcludedCourseModel extends Model implements ExcludedCourse {
   @Column(DataType.STRING)
   declare programme_code: ExcludedCourse['programme_code']
 
-  @Column(DataType.STRING)
-  declare course_code: ExcludedCourse['course_code']
-
   @Column(DataType.STRING)
   declare course_group_id: ExcludedCourse['course_group_id']
 

services/backend/src/models/kone/progressCriteria.ts

@@ -16,24 +16,6 @@ export class ProgressCriteriaModel extends Model implements ProgressCriteria {
   @Column(DataType.STRING)
   declare curriculumVersion: ProgressCriteria['curriculumVersion']
 
-  @Column(DataType.ARRAY(DataType.STRING))
-  declare coursesYearOne: ProgressCriteria['coursesYearOne']
-
-  @Column(DataType.ARRAY(DataType.STRING))
-  declare coursesYearTwo: ProgressCriteria['coursesYearTwo']
-
-  @Column(DataType.ARRAY(DataType.STRING))
-  declare coursesYearThree: ProgressCriteria['coursesYearThree']
-
-  @Column(DataType.ARRAY(DataType.STRING))
-  declare coursesYearFour: ProgressCriteria['coursesYearFour']
-
-  @Column(DataType.ARRAY(DataType.STRING))
-  declare coursesYearFive: ProgressCriteria['coursesYearFive']
-
-  @Column(DataType.ARRAY(DataType.STRING))
-  declare coursesYearSix: ProgressCriteria['coursesYearSix']
-
   @Column(DataType.ARRAY(DataType.STRING))
   declare courseGroupIdsYearOne: ProgressCriteria['courseGroupIdsYearOne']
 

services/backend/src/services/excludedCourses.ts

@@ -1,8 +1,6 @@
 import { Op } from 'sequelize'
 
-import { CourseModel } from '../models'
 import { ExcludedCourseModel } from '../models/kone'
-import logger from '../util/logger'
 
 export const addExcludedCourses = async (
   courseGroupIds: string[],
@@ -44,42 +42,3 @@ export const removeExcludedCourses = async (
     },
   })
 }
-
-/**
- * Fills course_group_id for rows that only have the legacy course_code. Safe to run repeatedly.
- * Rows whose code cannot be found in the course table are left untouched and reported.
- */
-export const backfillExcludedCourseGroupIds = async () => {
-  const legacyRows = await ExcludedCourseModel.findAll({
-    attributes: ['course_code'],
-    where: { course_group_id: null, course_code: { [Op.ne]: null } },
-    group: ['course_code'],
-    raw: true,
-  })
-  if (legacyRows.length === 0) return
-
-  const courseCodes = legacyRows.map(({ course_code }) => course_code!)
-  const courses = await CourseModel.findAll({
-    attributes: ['code', 'groupId'],
-    where: { code: { [Op.in]: courseCodes } },
-    raw: true,
-  })
-  const groupIdByCode = new Map(courses.map(({ code, groupId }) => [code, groupId]))
-
-  let updated = 0
-  for (const courseCode of courseCodes) {
-    const groupId = groupIdByCode.get(courseCode)
-    if (!groupId) continue
-    const [count] = await ExcludedCourseModel.update(
-      { course_group_id: groupId },
-      { where: { course_code: courseCode, course_group_id: null } }
-    )
-    updated += count
-  }
-
-  const unmapped = courseCodes.filter(code => !groupIdByCode.has(code))
-  logger.info(`Backfilled course_group_id for ${updated} excluded courses`)
-  if (unmapped.length > 0) {
-    logger.warn(`Could not find a course group id for excluded course codes: ${unmapped.join(', ')}`)
-  }
-}

services/backend/src/services/programmeModules.ts

@@ -142,9 +142,7 @@ const labelProgrammes = (modules: ModuleWithChildren[], excludedCourses: Exclude
       label: `${module.parent_code}\n${module.parent_name?.fi}`,
       orderNumber: module.module_order,
     }
-    const foundCourse = excludedCourses.find(course =>
-      course.course_group_id ? course.course_group_id === module.group_id : course.course_code === module.code
-    )
+    const foundCourse = excludedCourses.find(course => course.course_group_id === module.group_id)
     const visible = { visibility: !foundCourse, id: foundCourse?.id ?? null }
 
     return { ...module, label, visible }

services/backend/src/services/studyProgramme/criteriaCourseGroupIds.ts

@@ -1,21 +0,0 @@
-export const CRITERIA_YEARS = ['One', 'Two', 'Three', 'Four', 'Five', 'Six'] as const
-
-export type CriteriaYear = (typeof CRITERIA_YEARS)[number]
-
-/**
- * Converts course codes to course group ids, keeping the order and dropping duplicates.
- * Codes that are not in the mapping are returned in `unmapped`.
- */
-export const codesToGroupIds = (codes: readonly string[], groupIdByCode: ReadonlyMap<string, string>) => {
-  const groupIds = new Set<string>()
-  const unmapped: string[] = []
-  for (const code of codes) {
-    const groupId = groupIdByCode.get(code)
-    if (groupId) {
-      groupIds.add(groupId)
-    } else {
-      unmapped.push(code)
-    }
-  }
-  return { groupIds: [...groupIds], unmapped }
-}

services/backend/src/services/studyProgramme/studyProgrammeCriteria.ts

@@ -4,7 +4,6 @@ import { ProgressCriteria } from '@oodikone/shared/types'
 import { CourseModel } from '../../models'
 import { ProgressCriteriaModel } from '../../models/kone'
 import logger from '../../util/logger'
-import { CRITERIA_YEARS, codesToGroupIds } from './criteriaCourseGroupIds'
 
 type CriteriaWithoutCurriculumVersion = Omit<ProgressCriteriaModel, 'curriculumVersion'>
 
@@ -14,16 +13,6 @@ const getCriteriaByStudyProgramme = async (code: string): Promise<CriteriaWithou
     where: { code },
   })
 
-const getGroupIdByCode = async (codes: string[]) => {
-  if (codes.length === 0) return new Map<string, string>()
-  const courses: Array<Pick<CourseModel, 'code' | 'groupId'>> = await CourseModel.findAll({
-    attributes: ['code', 'groupId'],
-    where: { code: { [Op.in]: codes } },
-    raw: true,
-  })
-  return new Map(courses.map(({ code, groupId }) => [code, groupId]))
-}
-
 /** Construct course_group_id => substitutionGroups object */
 const getSubstitutions = async (courseGroupIds: string[]) => {
   const courses: Array<Pick<CourseModel, 'groupId' | 'substitutionGroups' | 'isPrimary'>> = await CourseModel.findAll({
@@ -49,28 +38,13 @@ const getSubstitutions = async (courseGroupIds: string[]) => {
   )
 }
 
-/**
- * TODO: Remove the fallback to coursesYear* (course codes) once all rows have courseGroupIdsYear*
- * populated and the old columns are dropped.
- */
 const formatCriteria = async (criteria: CriteriaWithoutCurriculumVersion | null) => {
-  const legacyCodes = CRITERIA_YEARS.flatMap(year =>
-    criteria && !criteria[`courseGroupIdsYear${year}`] ? (criteria[`coursesYear${year}`] ?? []) : []
-  )
-  const groupIdByCode = await getGroupIdByCode(legacyCodes)
-
-  const getYear = (year: (typeof CRITERIA_YEARS)[number]) => {
-    const courseGroupIds = criteria?.[`courseGroupIdsYear${year}`]
-    if (courseGroupIds) return courseGroupIds
-    return codesToGroupIds(criteria?.[`coursesYear${year}`] ?? [], groupIdByCode).groupIds
-  }
-
-  const yearOne = getYear('One')
-  const yearTwo = getYear('Two')
-  const yearThree = getYear('Three')
-  const yearFour = getYear('Four')
-  const yearFive = getYear('Five')
-  const yearSix = getYear('Six')
+  const yearOne = criteria?.courseGroupIdsYearOne ?? []
+  const yearTwo = criteria?.courseGroupIdsYearTwo ?? []
+  const yearThree = criteria?.courseGroupIdsYearThree ?? []
+  const yearFour = criteria?.courseGroupIdsYearFour ?? []
+  const yearFive = criteria?.courseGroupIdsYearFive ?? []
+  const yearSix = criteria?.courseGroupIdsYearSix ?? []
   const courseGroupIds = [...yearOne, ...yearTwo, ...yearThree, ...yearFour, ...yearFive, ...yearSix]
 
   const formattedCriteria: ProgressCriteria = {
@@ -197,39 +171,3 @@ export const getCriteria = async (studyProgramme: string) => {
 
   return await formatCriteria(studyProgrammeCriteria)
 }
-
-/**
- * Fills courseGroupIdsYear* for rows that only have the legacy coursesYear* (course codes). Safe to run repeatedly.
- * A year whose codes cannot all be found in the course table is left untouched and reported.
- */
-export const backfillProgressCriteriaGroupIds = async () => {
-  const rows = await ProgressCriteriaModel.findAll()
-  const rowsToBackfill = rows.filter(row =>
-    CRITERIA_YEARS.some(year => !row[`courseGroupIdsYear${year}`] && row[`coursesYear${year}`])
-  )
-  if (rowsToBackfill.length === 0) return
-
-  const allCodes = rowsToBackfill.flatMap(row => CRITERIA_YEARS.flatMap(year => row[`coursesYear${year}`] ?? []))
-  const groupIdByCode = await getGroupIdByCode([...new Set(allCodes)])
-
-  let updatedYears = 0
-  for (const row of rowsToBackfill) {
-    const updates: Partial<Record<`courseGroupIdsYear${(typeof CRITERIA_YEARS)[number]}`, string[]>> = {}
-    for (const year of CRITERIA_YEARS) {
-      const codes = row[`coursesYear${year}`]
-      if (row[`courseGroupIdsYear${year}`] || !codes) continue
-
-      const { groupIds, unmapped } = codesToGroupIds(codes, groupIdByCode)
-      if (unmapped.length > 0) {
-        logger.warn(
-          `Could not find a course group id for progress criteria codes of ${row.code} (${year}): ${unmapped.join(', ')}`
-        )
-        continue
-      }
-      updates[`courseGroupIdsYear${year}`] = groupIds
-      updatedYears++
-    }
-    if (Object.keys(updates).length > 0) await row.update(updates)
-  }
-  logger.info(`Backfilled course group ids for ${updatedYears} progress criteria years`)
-}

services/shared/models/kone/excludedCourse.ts

@@ -5,9 +5,7 @@ export type ExcludedCourseCreation = Optional<ExcludedCourse, 'id' | 'createdAt'
 export type ExcludedCourse = {
   id: number
   programme_code: string
-  // TODO: Remove course_code once all rows have been migrated to course_group_id
-  course_code: string | null
-  course_group_id: string | null
+  course_group_id: string
   curriculum_version: string
   createdAt: Date
   updatedAt: Date

services/shared/models/kone/progressCriteria.ts

@@ -5,19 +5,12 @@ export type ProgressCriteriaCreation = Optional<ProgressCriteria, 'curriculumVer
 export type ProgressCriteria = {
   code: string
   curriculumVersion: string
-  // TODO: Remove coursesYear* once all rows have been migrated to courseGroupIdsYear*
-  coursesYearOne: string[]
-  coursesYearTwo: string[]
-  coursesYearThree: string[]
-  coursesYearFour: string[]
-  coursesYearFive: string[]
-  coursesYearSix: string[]
-  courseGroupIdsYearOne: string[] | null
-  courseGroupIdsYearTwo: string[] | null
-  courseGroupIdsYearThree: string[] | null
-  courseGroupIdsYearFour: string[] | null
-  courseGroupIdsYearFive: string[] | null
-  courseGroupIdsYearSix: string[] | null
+  courseGroupIdsYearOne: string[]
+  courseGroupIdsYearTwo: string[]
+  courseGroupIdsYearThree: string[]
+  courseGroupIdsYearFour: string[]
+  courseGroupIdsYearFive: string[]
+  courseGroupIdsYearSix: string[]
   creditsYearOne: number
   creditsYearTwo: number
   creditsYearThree: number