feat(website): support multiple saved timetables per semester

website/src/actions/moduleBank-lru.ts

@@ -1,20 +1,28 @@
-import { flatMap, sortBy, get } from 'lodash-es';
+import { flatMap, sortBy, get, values } from 'lodash-es';
 
-import { ModulesMap } from 'types/reducers';
-import { TimetableConfig } from 'types/timetables';
+import { ModulesMap, TimetablesState } from 'types/reducers';
+import { SemTimetableConfig, TimetableConfig } from 'types/timetables';
 import { ModuleCode } from 'types/modules';
 
 // Module bank utils - exported separately so this does not become exported as an action creator
 
+// All lesson configs that pin their modules in the module bank: the live
+// timetable of every semester, plus every saved slot's snapshot
+export function getPinnedLessonConfigs(timetables: TimetablesState): SemTimetableConfig[] {
+  return [
+    ...values(timetables.lessons),
+    ...flatMap(values(timetables.slots), (slots) => slots.map((slot) => slot.data.lessons)),
+  ];
+}
+
 // Export for testing
-// eslint-disable-next-line import/prefer-default-export
 export function getLRUModules(
   modules: ModulesMap,
-  lessons: TimetableConfig,
+  lessons: TimetableConfig | SemTimetableConfig[],
   currentModule: string,
   toRemove = 1,
 ): ModuleCode[] {
-  // Pull all the modules in all the timetables
+  // Pull all the modules in all the timetables and saved slots
   const timetableModules = new Set(flatMap(lessons, (semester) => Object.keys(semester)));
 
   // Remove the module which is least recently used and which is not in timetable

website/src/actions/moduleBank.test.ts

@@ -88,6 +88,19 @@ test('getLRUModule should return the LRU and non-timetable module', () => {
   expect(resultOfAction).toMatchSnapshot();
 });
 
+test('getLRUModule should not evict modules saved in timetable slots', () => {
+  const modules: any = {
+    ACC1001: { timestamp: 1 }, // in an inactive slot - must not be evicted
+    ACC1002: { timestamp: 2 }, // not referenced anywhere - evictable
+    ACC1003: { timestamp: 3 }, // in the live timetable
+  };
+
+  // Lesson configs may come from live timetables and saved slots alike
+  const lessonConfigs: any = [{ ACC1003: {} }, { ACC1001: {} }];
+
+  expect(getLRUModules(modules, lessonConfigs, 'ACC1004')).toEqual(['ACC1002']);
+});
+
 test('removeLRUModule should return an action', () => {
   const resultOfAction = actions.Internal.removeLRUModule(['ACC1001']);
   expect(resultOfAction).toMatchSnapshot();

website/src/actions/moduleBank.ts

@@ -18,7 +18,7 @@ import {
   REMOVE_LRU_MODULE,
   UPDATE_MODULE_TIMESTAMP,
 } from './constants';
-import { getLRUModules } from './moduleBank-lru';
+import { getLRUModules, getPinnedLessonConfigs } from './moduleBank-lru';
 
 export function fetchModuleList() {
   return requestAction(FETCH_MODULE_LIST, {
@@ -102,7 +102,7 @@ export function fetchModule(moduleCode: ModuleCode) {
 
         const LRUModule = getLRUModules(
           moduleBank.modules,
-          timetables.lessons,
+          getPinnedLessonConfigs(timetables),
           moduleCode,
           overLimitCount,
         );

website/src/actions/timetables.test.ts

@@ -98,6 +98,66 @@ describe(actions.addModule, () => {
   });
 });
 
+describe('timetable slot thunks', () => {
+  // Two slots for semester 1: '0' (inactive, holds CS1010S) and '1' (active, blank)
+  const slotState: any = {
+    moduleBank: { moduleCodes: {}, modules: {} },
+    timetables: {
+      ...initialState,
+      lessons: { [1]: {} },
+      slots: {
+        [1]: [
+          {
+            id: '0',
+            title: 'Timetable 1',
+            data: { lessons: { CS1010S: {} }, colors: { CS1010S: 0 }, hidden: [], ta: [] },
+          },
+          { id: '1', title: 'Timetable 2', data: { lessons: {}, colors: {}, hidden: [], ta: [] } },
+        ],
+      },
+      activeSlot: { [1]: '1' },
+    },
+  };
+
+  // Dispatch mock that executes thunks so nested fetch/validate thunks run
+  const makeDispatch = (state: any) => {
+    const dispatch: any = vi.fn((action) =>
+      typeof action === 'function' ? action(dispatch, () => state) : action,
+    );
+    return dispatch;
+  };
+
+  // The timetable cannot render modules missing from the module bank, so the
+  // incoming slot's modules must be fetched before its data is loaded into
+  // the live timetable
+  const actionTypes = (dispatch: any) =>
+    dispatch.mock.calls.map(([action]: [any]) =>
+      typeof action === 'function' ? 'thunk' : action.type,
+    );
+
+  test('switchTimetableSlot should fetch slot modules before switching', async () => {
+    const dispatch = makeDispatch(slotState);
+    await actions.switchTimetableSlot(1, '0')(dispatch, () => slotState);
+
+    expect(dispatch).toHaveBeenCalledWith(actions.Internal.switchTimetableSlot(1, '0'));
+
+    const calls = actionTypes(dispatch);
+    const switchIndex = calls.indexOf(actions.SWITCH_TIMETABLE_SLOT);
+    expect(calls.slice(0, switchIndex)).toContain('thunk');
+  });
+
+  test('deleteTimetableSlot should fetch neighbouring slot modules before deleting', async () => {
+    const dispatch = makeDispatch(slotState);
+    await actions.deleteTimetableSlot(1, '1')(dispatch, () => slotState);
+
+    expect(dispatch).toHaveBeenCalledWith(actions.Internal.deleteTimetableSlot(1, '1'));
+
+    const calls = actionTypes(dispatch);
+    const deleteIndex = calls.indexOf(actions.DELETE_TIMETABLE_SLOT);
+    expect(calls.slice(0, deleteIndex)).toContain('thunk');
+  });
+});
+
 test('removeLesson should return information to remove module', () => {
   const semester: Semester = 1;
   const moduleCode: ModuleCode = 'CS1010';

website/src/actions/timetables.ts

@@ -43,6 +43,8 @@ export const SET_TIMETABLE = 'SET_TIMETABLE' as const;
 export const ADD_MODULE = 'ADD_MODULE' as const;
 export const SET_HIDDEN_IMPORTED = 'SET_HIDDEN_IMPORTED' as const;
 export const SET_TA_IMPORTED = 'SET_TA_IMPORTED' as const;
+export const SWITCH_TIMETABLE_SLOT = 'SWITCH_TIMETABLE_SLOT' as const;
+export const DELETE_TIMETABLE_SLOT = 'DELETE_TIMETABLE_SLOT' as const;
 export const Internal = {
   setTimetable(
     semester: Semester,
@@ -67,8 +69,78 @@ export const Internal = {
       },
     };
   },
+
+  switchTimetableSlot(semester: Semester, slotId: string) {
+    return {
+      type: SWITCH_TIMETABLE_SLOT,
+      payload: { semester, slotId },
+    };
+  },
+
+  deleteTimetableSlot(semester: Semester, slotId: string) {
+    return {
+      type: DELETE_TIMETABLE_SLOT,
+      payload: { semester, slotId },
+    };
+  },
 };
 
+export const ADD_TIMETABLE_SLOT = 'ADD_TIMETABLE_SLOT' as const;
+export function addTimetableSlot(
+  semester: Semester,
+  options: { title?: string; duplicateCurrent?: boolean } = {},
+) {
+  return {
+    type: ADD_TIMETABLE_SLOT,
+    payload: {
+      semester,
+      title: options.title,
+      duplicateCurrent: options.duplicateCurrent ?? false,
+    },
+  };
+}
+
+export const RENAME_TIMETABLE_SLOT = 'RENAME_TIMETABLE_SLOT' as const;
+export function renameTimetableSlot(semester: Semester, slotId: string, title: string) {
+  return {
+    type: RENAME_TIMETABLE_SLOT,
+    payload: { semester, slotId, title },
+  };
+}
+
+// The timetable cannot render modules that are missing from the module bank,
+// so a slot's modules must be fetched BEFORE its data is loaded into the live
+// timetable. Modules only referenced by a saved slot may have been evicted
+// from the module bank by older versions of the LRU logic.
+export function switchTimetableSlot(semester: Semester, slotId: string) {
+  return (dispatch: Dispatch, getState: GetState) => {
+    const slot = getState().timetables.slots[semester]?.find((s) => s.id === slotId);
+    return dispatch(fetchTimetableModules([slot?.data.lessons ?? {}])).then(() => {
+      dispatch(Internal.switchTimetableSlot(semester, slotId));
+      return dispatch(validateTimetable(semester));
+    });
+  };
+}
+
+export function deleteTimetableSlot(semester: Semester, slotId: string) {
+  return (dispatch: Dispatch, getState: GetState) => {
+    const { slots, activeSlot } = getState().timetables;
+    const semesterSlots = slots[semester] ?? [];
+    const index = semesterSlots.findIndex((slot) => slot.id === slotId);
+    // Deleting the active slot loads the neighbouring slot's timetable, so
+    // fetch the neighbour's modules first
+    const neighbour =
+      activeSlot[semester] === slotId
+        ? (semesterSlots[index + 1] ?? semesterSlots[index - 1])
+        : undefined;
+
+    return dispatch(fetchTimetableModules([neighbour?.data.lessons ?? {}])).then(() => {
+      dispatch(Internal.deleteTimetableSlot(semester, slotId));
+      return dispatch(validateTimetable(semester));
+    });
+  };
+}
+
 export function addModule(semester: Semester, moduleCode: ModuleCode) {
   return (dispatch: Dispatch, getState: GetState) =>
     dispatch(fetchModule(moduleCode)).then(() => {

website/src/reducers/index.test.ts

@@ -2,6 +2,8 @@ import { ExportData } from 'types/export';
 import { VERTICAL } from 'types/reducers';
 import reducers from 'reducers';
 import { setExportedData } from 'actions/export';
+import { addTimetableSlot, Internal } from 'actions/timetables';
+import { undo } from 'actions/undoHistory';
 import modules from '__mocks__/modules/index';
 import { DARK_COLOR_SCHEME, DARK_COLOR_SCHEME_PREFERENCE } from 'types/settings';
 import { SemTimetableConfig, TimetableConfig } from 'types/timetables';
@@ -74,6 +76,8 @@ test('reducers should set export data state', () => {
     },
     hidden: { [1]: ['PC1222'] },
     ta: { [1]: ['CS1010S'] },
+    slots: {},
+    activeSlot: {},
     academicYear: expect.any(String),
     archive: {},
   });
@@ -88,3 +92,17 @@ test('reducers should set export data state', () => {
     showTitle: true,
   });
 });
+
+test('undo should restore a deleted timetable slot', () => {
+  let state = reducers({} as any, { type: 'INIT', payload: null } as any);
+  state = reducers(state, addTimetableSlot(1, { duplicateCurrent: true }));
+  const beforeDelete = state;
+  expect(state.timetables.slots[1]).toHaveLength(2);
+
+  state = reducers(state, Internal.deleteTimetableSlot(1, '0'));
+  expect(state.timetables.slots[1]).toHaveLength(1);
+
+  state = reducers(state, undo());
+  expect(state.timetables.slots[1]).toEqual(beforeDelete.timetables.slots[1]);
+  expect(state.timetables.activeSlot[1]).toBe(beforeDelete.timetables.activeSlot[1]);
+});

website/src/reducers/index.ts

@@ -1,6 +1,6 @@
 import { Reducer } from 'redux';
 
-import { REMOVE_MODULE, SET_TIMETABLE } from 'actions/timetables';
+import { DELETE_TIMETABLE_SLOT, REMOVE_MODULE, SET_TIMETABLE } from 'actions/timetables';
 
 import persistReducer from 'storage/persistReducer';
 import { withReduxStateSync, receiveState } from 'middlewares/state-sync-middleware';
@@ -32,7 +32,10 @@ const planner = persistReducer('planner', plannerReducer, plannerPersistConfig);
 const defaultState = {} as unknown as State;
 const undoReducer = createUndoReducer<State>({
   limit: 1,
-  actionsToWatch: [REMOVE_MODULE, SET_TIMETABLE],
+  // SWITCH/ADD_TIMETABLE_SLOT are deliberately not watched: switching is
+  // losslessly reversible by switching back, and with limit: 1 watching them
+  // would clobber the user's single undo step
+  actionsToWatch: [REMOVE_MODULE, SET_TIMETABLE, DELETE_TIMETABLE_SLOT],
   storedKeyPaths: ['timetables', 'theme.colors'],
 });