Patch for #4225, with additional checks to prevent overwriting user config if error encountered and allow recovery for malformed configs
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
-
When the app fetches the timetable modules, it populates the lesson indices which allows the migration of the module config to the new schema https://github.com/nusmodifications/nusmods/blob/d31261e060e45bcd79b07d251f0ffe2403905dee/website/src/reducers/moduleBank.ts#L47-L60
-
The issue was caused by https://github.com/nusmodifications/nusmods/blob/d31261e060e45bcd79b07d251f0ffe2403905dee/website/src/views/AppShell.tsx#L64-L66 calling https://github.com/nusmodifications/nusmods/blob/d31261e060e45bcd79b07d251f0ffe2403905dee/website/src/actions/timetables.ts#L231-L239
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.
@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 Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 56.62%. Comparing base (988c6fd) to head (2986fbe).
:warning: 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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
The latest updates on your projects. Learn more about Vercel for GitHub.
| Project | Deployment | Review | Updated (UTC) |
|---|---|---|---|
| nusmods-export | Preview, Comment | Dec 20, 2025 8:50pm | |
| nusmods-website | Preview, Comment | Dec 20, 2025 8:50pm |
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.