From ca16291e37cdd6b72ec8f30cd627fbefabf98883 Mon Sep 17 00:00:00 2001 From: joonas-a Date: Tue, 29 Sep 2026 17:05:31 +0300 Subject: [PATCH] [Kone] Migration step 2 of 2 - Remove old fields from db - Not null enforced - Remove dead code --- services/backend/src/app.ts | 4 - ...9_02_drop_course_codes_from_kone_tables.js | 94 +++++++++++++++++++ .../backend/src/models/kone/excludedCourse.ts | 3 - .../src/models/kone/progressCriteria.ts | 18 ---- .../backend/src/services/excludedCourses.ts | 41 -------- .../backend/src/services/programmeModules.ts | 4 +- .../studyProgramme/criteriaCourseGroupIds.ts | 21 ----- .../studyProgramme/studyProgrammeCriteria.ts | 74 ++------------- services/shared/models/kone/excludedCourse.ts | 4 +- .../shared/models/kone/progressCriteria.ts | 19 ++-- 10 files changed, 108 insertions(+), 174 deletions(-) create mode 100644 services/backend/src/database/migrations_kone/20260929_02_drop_course_codes_from_kone_tables.js delete mode 100644 services/backend/src/services/studyProgramme/criteriaCourseGroupIds.ts diff --git a/services/backend/src/app.ts b/services/backend/src/app.ts index dfc84a687..ab928dd52 100644 --- a/services/backend/src/app.ts +++ b/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', () => { diff --git a/services/backend/src/database/migrations_kone/20260929_02_drop_course_codes_from_kone_tables.js b/services/backend/src/database/migrations_kone/20260929_02_drop_course_codes_from_kone_tables.js new file mode 100644 index 000000000..0d8d5f973 --- /dev/null +++ b/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 } + ) + } + }) + }, +} diff --git a/services/backend/src/models/kone/excludedCourse.ts b/services/backend/src/models/kone/excludedCourse.ts index a448c2926..3498bd369 100644 --- a/services/backend/src/models/kone/excludedCourse.ts +++ b/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'] diff --git a/services/backend/src/models/kone/progressCriteria.ts b/services/backend/src/models/kone/progressCriteria.ts index 03e29c310..145d2c427 100644 --- a/services/backend/src/models/kone/progressCriteria.ts +++ b/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'] diff --git a/services/backend/src/services/excludedCourses.ts b/services/backend/src/services/excludedCourses.ts index 2b645bdab..a637b266f 100644 --- a/services/backend/src/services/excludedCourses.ts +++ b/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(', ')}`) - } -} diff --git a/services/backend/src/services/programmeModules.ts b/services/backend/src/services/programmeModules.ts index ccf84e68c..fe3fbbe8b 100644 --- a/services/backend/src/services/programmeModules.ts +++ b/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 } diff --git a/services/backend/src/services/studyProgramme/criteriaCourseGroupIds.ts b/services/backend/src/services/studyProgramme/criteriaCourseGroupIds.ts deleted file mode 100644 index b07c9c1b3..000000000 --- a/services/backend/src/services/studyProgramme/criteriaCourseGroupIds.ts +++ /dev/null @@ -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) => { - const groupIds = new Set() - 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 } -} diff --git a/services/backend/src/services/studyProgramme/studyProgrammeCriteria.ts b/services/backend/src/services/studyProgramme/studyProgrammeCriteria.ts index b6c908b4d..b10456c77 100644 --- a/services/backend/src/services/studyProgramme/studyProgrammeCriteria.ts +++ b/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 @@ -14,16 +13,6 @@ const getCriteriaByStudyProgramme = async (code: string): Promise { - if (codes.length === 0) return new Map() - const courses: Array> = 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> = 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> = {} - 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`) -} diff --git a/services/shared/models/kone/excludedCourse.ts b/services/shared/models/kone/excludedCourse.ts index 7c33809dd..352d986f7 100644 --- a/services/shared/models/kone/excludedCourse.ts +++ b/services/shared/models/kone/excludedCourse.ts @@ -5,9 +5,7 @@ export type ExcludedCourseCreation = Optional