Skip to content

feat(website): support multiple saved timetables per semester - #4460

Open
eugenewong22 wants to merge 1 commit into
nusmodifications:masterfrom
eugenewong22:timetable-slots-pr1
Open

feat(website): support multiple saved timetables per semester#4460
eugenewong22 wants to merge 1 commit into
nusmodifications:masterfrom
eugenewong22:timetable-slots-pr1

Conversation

@eugenewong22

@eugenewong22 eugenewong22 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Context

Closes the "save slot" half of #4455: users want to keep multiple timetable arrangements per semester and switch between them without screenshotting before every change. (A follow-up PR adds the side-by-side comparison view — see the issue for the full plan.)

What this does

  • Save slots per semester: a pill-tab row under the semester switcher shows each saved arrangement. Switching tabs swaps the entire arrangement — lessons, colours, hidden modules and TA modules — instantly. Edits always go to the active slot.
  • Create / duplicate: a + menu adds a new blank timetable or duplicates the current one.
  • Rename / delete: a menu with modal dialogs (following ResetTimetable's pattern). Deleting is undoable via the existing undo notification; the last remaining slot can't be deleted.
  • Import as new timetable: shared timetable links now offer importing into a fresh slot, so importing no longer has to overwrite the saved timetable. The existing overwrite Import is unchanged.

Implementation

  • Swap-on-switch: the active timetable stays in the existing TimetablesState.{lessons,colors,hidden,ta} maps, so the ~15 existing timetable actions, all selectors, undo, share and export code are untouched. A new slots: { [semester]: TimetableSlot[] } array stores arrangements; SWITCH_TIMETABLE_SLOT snapshots the live maps into the outgoing slot and loads the target's data (reducers/timetables.ts).
  • Slot data is authoritative for inactive slots only — the active slot's data lives in the live maps and its snapshot is refreshed on switch-away. All reads go through the new getSlotTimetableData selector.
  • Lazy initialization + v3 persist migration: semesters with no slots entry implicitly have one active slot ("Timetable 1"). The redux-persist migration (migrateV2toV3, following the planner's exported-migration pattern) only adds two empty maps — no user data is rewritten.
  • Undo: DELETE_TIMETABLE_SLOT joins actionsToWatch. Switching/adding is deliberately not watched — switching back is lossless, and with limit: 1 it would clobber the single undo step.
  • Module bank: slot modules are pinned against LRU eviction (getPinnedLessonConfigs), and the switch/delete thunks fetch the incoming slot's modules before loading it into the live timetable.
  • Year rollover: unchanged — only the active lessons are archived; slots are reset with the rest of the year's state (documented in the reconciler; archiving all slots would need an archive shape change and is left as a follow-up).

Testing

  • Reducer: slot CRUD, switch round-trip preserves both timetables exactly, delete-active activates the neighbour, no-ops return the same state reference, migration unit tests (reducers/timetables.test.ts).
  • Selectors, thunk ordering (fetch before switch), LRU pinning, undo restore, switcher component behaviour, and an integration test that import-as-new-slot doesn't clobber the saved timetable.
  • pnpm run ci passes (lint, typecheck, 800+ tests, build).
  • Manually verified in Chromium: create/rename/duplicate/delete/switch, share-import both paths, reload persistence, v2→v3 migration with real persisted v2 state, and cross-tab sync with two tabs.
switcher

@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

Add "save slots" per semester (nusmodifications#4455): a tab switcher lets users keep
several timetable arrangements and switch between them instantly,
create blank or duplicated slots, and rename/delete via undoable
actions. Shared timetable links can now import into a new slot
instead of overwriting the saved one.

The active timetable stays in the existing lessons/colors/hidden/ta
maps, so existing timetable actions, selectors, undo, share and
export are unchanged. Inactive arrangements live in a new `slots` map
keyed by semester; switching snapshots the outgoing slot and loads
the target's data. A redux-persist v3 migration adds the new (empty)
maps for existing users, who implicitly keep their current timetable
as "Timetable 1". Slot modules are pinned against module bank LRU
eviction and prefetched before a switch loads them.
@eugenewong22
eugenewong22 force-pushed the timetable-slots-pr1 branch from b3df192 to cca1f2c Compare July 10, 2026 03:14
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.27083% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.79%. Comparing base (988c6fd) to head (cca1f2c).
⚠️ Report is 264 commits behind head on master.

Files with missing lines Patch % Lines
...ite/src/views/timetable/TimetableSlotsSwitcher.tsx 85.45% 8 Missing ⚠️
website/src/actions/moduleBank-lru.ts 66.66% 1 Missing ⚠️
website/src/actions/timetables.ts 96.77% 1 Missing ⚠️
website/src/views/timetable/TimetableContainer.tsx 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4460      +/-   ##
==========================================
+ Coverage   54.52%   59.79%   +5.26%     
==========================================
  Files         274      317      +43     
  Lines        6076     7486    +1410     
  Branches     1455     1846     +391     
==========================================
+ Hits         3313     4476    +1163     
- Misses       2763     3010     +247     

☔ View full report in Codecov by Harness.
📢 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.

@leslieyip02

Copy link
Copy Markdown
Member

@greptile

@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds multiple saved timetables for each semester. The main changes are:

  • Persisted timetable slots with create, duplicate, rename, delete, and switching controls.
  • Slot-aware module-cache pinning and loading before a saved slot becomes active.
  • A shared-timetable import option that creates a separate Imported slot.

Confidence Score: 4/5

  • The import-as-new-timetable undo flow leaves persisted slot state different from the state before import.
  • One confirmed user-visible state restoration bug affects the new import workflow.

Files Needing Attention: - website/src/views/timetable/TimetableContainer.tsx

T-Rex T-Rex Logs

What T-Rex did

  • An environment blocker was encountered during setup: ldd reported 15 missing Chromium dependencies, and the Playwright trace showed Chromium exiting with code 127 before a browser page could open, so no UI proof could be captured.
  • A focused React/Redux Vitest flow loaded a saved timetable, clicked Import as new timetable, and invoked the Undo handler; the Undo operation reproduced an extra slot (slot 2: Imported) and did not revert to the pre-import state, which had slots 0 and 1 with active slot 1.

View all artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix All With AI
### Issue 1
website/src/views/timetable/TimetableContainer.tsx:112-113
**Undo leaves an extra slot**
`addTimetableSlot` is dispatched before the watched `SET_TIMETABLE` import action. Undo therefore restores the state after the slot was created: the imported lessons are removed but the new empty, active `Imported` slot remains. This makes the import notification's `Undo` action fail to restore the prior saved-timetable state; make the combined import atomic for undo, or remove the new slot when its import is undone.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat(website): support multiple saved ti..." | Re-trigger Greptile

Comment on lines +112 to +113
dispatch(addTimetableSlot(semester, { title: 'Imported' }));
importTimetable();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Undo leaves an extra slot
addTimetableSlot is dispatched before the watched SET_TIMETABLE import action. Undo therefore restores the state after the slot was created: the imported lessons are removed but the new empty, active Imported slot remains. This makes the import notification's Undo action fail to restore the prior saved-timetable state; make the combined import atomic for undo, or remove the new slot when its import is undone.

Knowledge Base Used:

Artifacts

Shared timetable Undo flow test

  • Focused React and Redux test that loads a shared timetable, imports it into a new slot, invokes the real notification Undo handler, and compares the resulting slot state with the saved pre-import state — the equality assertion is expected to fail.

Undo retains Imported timetable slot

  • Verbose Vitest output for the shared-timetable flow showing the state before import, after import, and after Undo — Undo leaves active Imported slot 2 while clearing its live lessons.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: website/src/views/timetable/TimetableContainer.tsx
Line: 112-113

Comment:
**Undo leaves an extra slot**
`addTimetableSlot` is dispatched before the watched `SET_TIMETABLE` import action. Undo therefore restores the state after the slot was created: the imported lessons are removed but the new empty, active `Imported` slot remains. This makes the import notification's `Undo` action fail to restore the prior saved-timetable state; make the combined import atomic for undo, or remove the new slot when its import is undone.

**Knowledge Base Used:**
- [Client state and persistence](https://app.greptile.com/nusmods/-/custom-context/knowledge-base/nusmodifications/nusmods/-/docs/client-state-and-persistence.md)
- [Timetable experience](https://app.greptile.com/nusmods/-/custom-context/knowledge-base/nusmodifications/nusmods/-/docs/timetable-experience.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants