diff --git a/services/backend/src/app.ts b/services/backend/src/app.ts index ab928dd52..280164d3c 100644 --- a/services/backend/src/app.ts +++ b/services/backend/src/app.ts @@ -4,6 +4,7 @@ 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 logger from './util/logger' import './worker/worker' @@ -13,6 +14,7 @@ initializeDatabaseConnection() dbConnections.on('connect', () => { logger.info('Connected to sis db successfully') + backfillExcludedCourseGroupIds().catch(error => logger.error(error)) }) dbConnections.on('error', () => { diff --git a/services/backend/src/database/migrations_kone/20260929_00_add_course_group_id_to_excluded_courses.js b/services/backend/src/database/migrations_kone/20260929_00_add_course_group_id_to_excluded_courses.js new file mode 100644 index 000000000..6922321ab --- /dev/null +++ b/services/backend/src/database/migrations_kone/20260929_00_add_course_group_id_to_excluded_courses.js @@ -0,0 +1,14 @@ +const { STRING } = require('sequelize') + +// Migration step 1 of 2 +module.exports = { + up: async ({ context: queryInterface }) => { + await queryInterface.addColumn('excluded_courses', 'course_group_id', { + type: STRING, + }) + }, + + down: async ({ context: queryInterface }) => { + await queryInterface.removeColumn('excluded_courses', 'course_group_id') + }, +} diff --git a/services/backend/src/models/kone/excludedCourse.ts b/services/backend/src/models/kone/excludedCourse.ts index a002d05ee..a448c2926 100644 --- a/services/backend/src/models/kone/excludedCourse.ts +++ b/services/backend/src/models/kone/excludedCourse.ts @@ -19,6 +19,9 @@ export class ExcludedCourseModel extends Model implements ExcludedCourse { @Column(DataType.STRING) declare course_code: ExcludedCourse['course_code'] + @Column(DataType.STRING) + declare course_group_id: ExcludedCourse['course_group_id'] + @Column(DataType.STRING) declare curriculum_version: ExcludedCourse['curriculum_version'] diff --git a/services/backend/src/routes/courseExclusions.ts b/services/backend/src/routes/courseExclusions.ts index 558618618..5708d3b3f 100644 --- a/services/backend/src/routes/courseExclusions.ts +++ b/services/backend/src/routes/courseExclusions.ts @@ -7,7 +7,7 @@ import { hasFullAccessToStudentData } from '../util' const router = Router() router.post('/', async (req, res) => { - const { programmeCode, courseCodes, curriculumVersion } = req.body + const { programmeCode, courseGroupIds, curriculumVersion } = req.body const { roles, programmeRights } = req.user const hasFullAccess = hasFullAccessToStudentData(roles) @@ -15,7 +15,7 @@ router.post('/', async (r if (!hasFullAccess && !hasAccessToProgramme) return res.status(403).end() - const result = await addExcludedCourses(courseCodes, curriculumVersion, programmeCode) + const result = await addExcludedCourses(courseGroupIds, curriculumVersion, programmeCode) if (!result) { res.status(400).end() return @@ -24,7 +24,7 @@ router.post('/', async (r }) router.delete('/', async (req, res) => { - const { programmeCode, courseCodes, curriculumVersion } = req.body + const { programmeCode, courseGroupIds, curriculumVersion } = req.body const { roles, programmeRights } = req.user const hasFullAccess = hasFullAccessToStudentData(roles) @@ -32,7 +32,7 @@ router.delete('/', async if (!hasFullAccess && !hasAccessToProgramme) return res.status(403).end() - const result = await removeExcludedCourses(courseCodes, curriculumVersion, programmeCode) + const result = await removeExcludedCourses(courseGroupIds, curriculumVersion, programmeCode) if (!result) { res.status(400).end() return diff --git a/services/backend/src/services/excludedCourses.ts b/services/backend/src/services/excludedCourses.ts index 7a1c86da8..2b645bdab 100644 --- a/services/backend/src/services/excludedCourses.ts +++ b/services/backend/src/services/excludedCourses.ts @@ -1,19 +1,38 @@ import { Op } from 'sequelize' +import { CourseModel } from '../models' import { ExcludedCourseModel } from '../models/kone' +import logger from '../util/logger' -export const addExcludedCourses = async (courseCodes: string[], curriculumVersion: string, programmeCode: string) => { - return ExcludedCourseModel.bulkCreate( - courseCodes.map(courseCode => ({ +export const addExcludedCourses = async ( + courseGroupIds: string[], + curriculumVersion: string, + programmeCode: string +) => { + const existing = await ExcludedCourseModel.findAll({ + attributes: ['course_group_id'], + where: { programme_code: programmeCode, curriculum_version: curriculumVersion, - course_code: courseCode, - })) + course_group_id: { [Op.in]: courseGroupIds }, + }, + raw: true, + }) + const alreadyExcluded = new Set(existing.map(({ course_group_id }) => course_group_id)) + + return ExcludedCourseModel.bulkCreate( + courseGroupIds + .filter(courseGroupId => !alreadyExcluded.has(courseGroupId)) + .map(courseGroupId => ({ + programme_code: programmeCode, + curriculum_version: curriculumVersion, + course_group_id: courseGroupId, + })) ) } export const removeExcludedCourses = async ( - courseCodes: string[], + courseGroupIds: string[], curriculumVersion: string, programmeCode: string ) => { @@ -21,7 +40,46 @@ export const removeExcludedCourses = async ( where: { programme_code: programmeCode, curriculum_version: curriculumVersion, - course_code: { [Op.in]: courseCodes }, + course_group_id: { [Op.in]: courseGroupIds }, }, }) } + +/** + * 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 02f1fe4d5..ccf84e68c 100644 --- a/services/backend/src/services/programmeModules.ts +++ b/services/backend/src/services/programmeModules.ts @@ -47,7 +47,7 @@ export const getCurriculumOptions = async (code: string) => { } } -type ModuleWithChildren = Pick & { +type ModuleWithChildren = Pick & { // TODO: This is supposed to be in the ProgrammeModule degree_programme_type: DegreeProgrammeType | null @@ -90,6 +90,7 @@ const recursivelyGetModuleAndChildren = async (code: string, curriculum_period_i GROUP BY pm.id, c.name, c.code, c.order, c.id ) SELECT id, + group_id, type, module_order, parent_code, @@ -141,7 +142,9 @@ 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_code === module.code) + const foundCourse = excludedCourses.find(course => + course.course_group_id ? course.course_group_id === module.group_id : course.course_code === module.code + ) const visible = { visibility: !foundCourse, id: foundCourse?.id ?? null } return { ...module, label, visible } @@ -180,6 +183,7 @@ const getCoursesAndModulesForProgramme = async (code: string, periodIds: string) return { courses: labelProgrammes(modifiedCourses, excludedCourses).map(mod => ({ + group_id: mod.group_id, code: mod.code, name: mod.name, degree_programme_type: mod.degree_programme_type, diff --git a/services/frontend/src/pages/StudyProgramme/DegreeCoursesTab/DegreeCourseTable/index.tsx b/services/frontend/src/pages/StudyProgramme/DegreeCoursesTab/DegreeCourseTable/index.tsx index 0dfe68edd..76169151b 100644 --- a/services/frontend/src/pages/StudyProgramme/DegreeCoursesTab/DegreeCourseTable/index.tsx +++ b/services/frontend/src/pages/StudyProgramme/DegreeCoursesTab/DegreeCourseTable/index.tsx @@ -93,7 +93,7 @@ export const DegreeCourseTable = ({ const excludeOne = (course: ProgrammeCourse) => { void setExclusion({ - courseCodes: [course.code], + courseGroupIds: [course.group_id], curriculumVersion: version, programmeCode: combinedProgramme === '' ? studyProgramme : combinedProgramme, }) @@ -104,7 +104,7 @@ export const DegreeCourseTable = ({ const module = getModule(moduleCode) const excludeFromProgramme = combinedProgramme === '' ? studyProgramme : combinedProgramme void setExclusion({ - courseCodes: module.courses.filter(course => course.visible.visibility).map(course => course.code), + courseGroupIds: module.courses.filter(course => course.visible.visibility).map(course => course.group_id), curriculumVersion: version, programmeCode: excludeFromProgramme, }) @@ -113,7 +113,7 @@ export const DegreeCourseTable = ({ const removeOne = (course: ProgrammeCourse) => { void removeExclusion({ - courseCodes: [course.code], + courseGroupIds: [course.group_id], curriculumVersion: version, programmeCode: studyProgramme, }) @@ -123,7 +123,7 @@ export const DegreeCourseTable = ({ const deleteAll = (moduleCode: string) => { const module = getModule(moduleCode) void removeExclusion({ - courseCodes: module.courses.map(course => course.code), + courseGroupIds: module.courses.map(course => course.group_id), curriculumVersion: version, programmeCode: studyProgramme, }) diff --git a/services/frontend/src/redux/courseExclusions.ts b/services/frontend/src/redux/courseExclusions.ts index a7575d03f..25cdc12da 100644 --- a/services/frontend/src/redux/courseExclusions.ts +++ b/services/frontend/src/redux/courseExclusions.ts @@ -5,17 +5,17 @@ import type { ExcludedCoursesResBody, ExcludedCoursesReqBody } from '@oodikone/s const courseExclusionsApi = RTKApi.injectEndpoints({ endpoints: builder => ({ setCourseExclusion: builder.mutation({ - query: ({ courseCodes, curriculumVersion, programmeCode }) => ({ + query: ({ courseGroupIds, curriculumVersion, programmeCode }) => ({ url: `/course-exclusions/`, method: 'POST', - body: { courseCodes, curriculumVersion, programmeCode }, + body: { courseGroupIds, curriculumVersion, programmeCode }, }), }), removeCourseExclusion: builder.mutation({ - query: ({ courseCodes, curriculumVersion, programmeCode }) => ({ + query: ({ courseGroupIds, curriculumVersion, programmeCode }) => ({ url: `/course-exclusions/`, method: 'DELETE', - body: { courseCodes, curriculumVersion, programmeCode }, + body: { courseGroupIds, curriculumVersion, programmeCode }, }), }), }), diff --git a/services/shared/models/kone/excludedCourse.ts b/services/shared/models/kone/excludedCourse.ts index 77365027c..7c33809dd 100644 --- a/services/shared/models/kone/excludedCourse.ts +++ b/services/shared/models/kone/excludedCourse.ts @@ -5,7 +5,9 @@ export type ExcludedCourseCreation = Optional