feat: support previous special term when academic year changes#4403
feat: support previous special term when academic year changes#4403ravern wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4403 +/- ##
==========================================
+ Coverage 54.52% 56.63% +2.10%
==========================================
Files 274 317 +43
Lines 6076 7031 +955
Branches 1455 1704 +249
==========================================
+ Hits 3313 3982 +669
- Misses 2763 3049 +286 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function shouldPreferArchiveSemesterData( | ||
| currentSemester: SemesterData | undefined, | ||
| archiveSemester: SemesterData, | ||
| ): boolean { | ||
| const currentHasTimetable = Boolean(currentSemester?.timetable?.length); | ||
| const archiveHasTimetable = Boolean(archiveSemester.timetable?.length); | ||
|
|
||
| if (currentHasTimetable && !archiveHasTimetable) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( | ||
| currentHasTimetable && | ||
| archiveHasTimetable && | ||
| JSON.stringify(currentSemester) === JSON.stringify(archiveSemester) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
shouldPreferArchiveSemesterData only compares the two objects with JSON.stringify when both have timetable entries. When neither has a timetable — a real scenario during the overlap window before ModReg R0 is populated — the function falls through to return true without checking whether the objects are otherwise identical. If the current AY's ST semester entry happens to carry an examDate (added by ModReg R0 planning) while the archive entry doesn't, the function still prefers the archive and silently drops the exam date. A broader equality check (or at minimum checking !currentHasTimetable && !archiveHasTimetable and applying JSON.stringify there too) would prevent this.
Prompt To Fix With AI
This is a comment left during a code review.
Path: website/src/utils/specialTerm.ts
Line: 69-89
Comment:
**Equality check missing for no-timetable case**
`shouldPreferArchiveSemesterData` only compares the two objects with `JSON.stringify` when *both* have timetable entries. When neither has a timetable — a real scenario during the overlap window before ModReg R0 is populated — the function falls through to `return true` without checking whether the objects are otherwise identical. If the current AY's ST semester entry happens to carry an `examDate` (added by ModReg R0 planning) while the archive entry doesn't, the function still prefers the archive and silently drops the exam date. A broader equality check (or at minimum checking `!currentHasTimetable && !archiveHasTimetable` and applying `JSON.stringify` there too) would prevent this.
How can I resolve this? If you propose a fix, please make it concise.| const cachedArchive = getState().moduleBank.moduleArchive[moduleCode]?.[specialTermAcadYear]; | ||
| let archiveModule = cachedArchive; | ||
|
|
||
| if (!archiveModule) { | ||
| try { | ||
| archiveModule = await dispatch<Module>(fetchModuleArchive(moduleCode, specialTermAcadYear)); | ||
| } catch { | ||
| return module; | ||
| } | ||
| } |
There was a problem hiding this comment.
When fetchModuleArchive throws (e.g., a module is genuinely new this AY and doesn't exist in the previous-AY archive), the catch block returns early without recording the failure anywhere. Because moduleBank.moduleArchive[moduleCode]?.[specialTermAcadYear] stays undefined, every subsequent call to fetchModule for the same module will repeat the failing network request. During the overlap period, this affects every new module that hasn't existed in a prior AY — a user browsing several such pages would trigger a fresh 404 each time. Storing a sentinel value (or using the existing request-state tracking in the middleware) to skip the fetch on repeat visits would avoid the repeated failures.
Prompt To Fix With AI
This is a comment left during a code review.
Path: website/src/actions/moduleBank.ts
Line: 62-71
Comment:
**Failed archive fetches are not cached — causes repeated 404s during overlap window**
When `fetchModuleArchive` throws (e.g., a module is genuinely new this AY and doesn't exist in the previous-AY archive), the `catch` block returns early without recording the failure anywhere. Because `moduleBank.moduleArchive[moduleCode]?.[specialTermAcadYear]` stays `undefined`, every subsequent call to `fetchModule` for the same module will repeat the failing network request. During the overlap period, this affects every new module that hasn't existed in a prior AY — a user browsing several such pages would trigger a fresh 404 each time. Storing a sentinel value (or using the existing request-state tracking in the middleware) to skip the fetch on repeat visits would avoid the repeated failures.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Closes #4109