Patch for #4225, with additional checks to prevent overwriting user config if error encountered and allow recovery for malformed configs

[zehata]

This is an extension to #4271, details of which I have copied over.
@leslieyip02 @jloh02 Patch for #4225, for the issue reported on the morning of 16 Dec by LH (https://t.me/NUSMods/13136)

Post-mortem

1. When the app fetches the timetable modules, it populates the lesson indices which allows the migration of the module config to the new schema
   

   nusmods/website/src/reducers/moduleBank.ts

   Lines 47 to 60 in d31261e

   	case SUCCESS_KEY(FETCH_MODULE):
   	return {
   	...state,
   	modules: {
   	...state.modules,
   	[action.payload.moduleCode]: {
   	...action.payload,
   	timestamp: Date.now(),
   	semesterData: map(action.payload.semesterData, (semesterData) => ({
   	...semesterData,
   	timetable: map(semesterData.timetable, (lesson, lessonIndex) => ({
   	...lesson,
   	lessonIndex,
   	})),

2. The issue was caused by
   

   nusmods/website/src/views/AppShell.tsx

   Lines 64 to 66 in d31261e

   	function fetchTimetableModulesImpl(timetable: SemTimetableConfig, semester: Semester) {
   	dispatch(fetchTimetableModulesAction([timetable]))
   	.then(() => dispatch(validateTimetable(semester)))

   
   calling
   

   nusmods/website/src/actions/timetables.ts

   Lines 231 to 239 in d31261e

   	export function validateTimetable(semester: Semester) {
   	return (dispatch: Dispatch, getState: GetState) => {
   	const { timetables, moduleBank } = getState();
   	const { lessons, ta, alreadyMigrated } = migrateTimetableConfigs(
   	timetables.lessons as TimetableConfig | TimetableConfigV1,
   	timetables.ta as TaModulesMap | TaModulesMapV1,
   	moduleBank.modules,
   	);

When validateTimetable(1) is called when the app successfully fetches the module data for semester 1, it calls migrateTimetableConfigs which migrates the configs for all semesters, including semester 2. Since the request for semester 2 has not returned, the lesson indices has not been populated for semester 2 (see point 1). Trying to migrate the config for semester 2 will thus fail.

Fix

This patch fixes this issue by removing the migrateTimetableConfigs function and instead migrates each semester's timetable config as data is fetched for each of the semester.

It adds checks to prevent overwriting the user's config if any error occurs during the migration process.

It also adds code to create a random config to overwrite malformed configs caused by the migration failure.

[vercel]

@zehata is attempting to deploy a commit to the modsbot's projects Team on Vercel.

A member of the Team first needs to authorize it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.62%. Comparing base (988c6fd) to head (2986fbe).
⚠️ Report is 153 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4272      +/-   ##
==========================================
+ Coverage   54.52%   56.62%   +2.10%     
==========================================
  Files         274      297      +23     
  Lines        6076     6924     +848     
  Branches     1455     1671     +216     
==========================================
+ Hits         3313     3921     +608     
- Misses       2763     3003     +240     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
nusmods-export	Ready	Preview, Comment	Dec 20, 2025 8:50pm
nusmods-website	Ready	Preview, Comment	Dec 20, 2025 8:50pm

[zehata]

Also, while doing testing, I realized that the error occurred before writing to state, so users' configs were actually not overwritten. Nonetheless, I think it would make more sense to add checks to explicitly ensure that users' configs are not overwritten when errors are encountered.

[leslieyip02]

Is the Promise.resolve needed? I don't think migrateSemTimetableConfig is async.

[zehata]

No, I am using it to end validation early if an error occurs during migration

[zehata]

Of course there are other ways of achieving the same effect

[leslieyip02]

LGTM! Thanks for the speedy fix!