Patch for #4225

website/src/actions/timetables.test.ts

@@ -4,7 +4,14 @@ import { SemTimetableConfig, LessonWithIndex, TimetableConfigV1 } from 'types/ti
 import lessons from '__mocks__/lessons-array.json';
 import { CS1010A, CS1010S, CS3216 } from '__mocks__/modules';
 
-import { TaModulesMapV1, ModuleBank, TimetablesState } from 'types/reducers';
+import {
+  TaModulesMapV1,
+  ModuleBank,
+  TimetablesState,
+  SemesterColorMap,
+  HiddenModulesMap,
+  ColorMapping,
+} from 'types/reducers';
 import { defaultTimetableState } from 'reducers/timetables';
 import * as actions from './timetables';
 
@@ -193,8 +200,12 @@ describe('fillTimetableBlanks', () => {
   });
 
   test('migrate v1 config', () => {
+    const colors: ColorMapping = {
+      CS1010S: 0,
+      CS3216: 1,
+    };
+    const hiddenModules: ModuleCode[] = [];
     const timetables = {
-      ...initialState,
       lessons: {
         [semester]: {
           CS1010S: {
@@ -207,6 +218,12 @@ describe('fillTimetableBlanks', () => {
           },
         } as TimetableConfigV1,
       },
+      colors: {
+        [semester]: colors,
+      } as SemesterColorMap,
+      hidden: {
+        [semester]: hiddenModules,
+      } as HiddenModulesMap,
       ta: {
         [semester]: {
           CS1010S: [
@@ -223,21 +240,27 @@ describe('fillTimetableBlanks', () => {
     action(dispatch, () => state);
     expect(dispatch).toHaveBeenCalledTimes(1);
     const [[firstAction]] = dispatch.mock.calls;
-    expect(firstAction).toMatchObject({
-      type: 'SET_TIMETABLES',
+
+    const migratedTimetable: SemTimetableConfig = {
+      CS1010S: {
+        Lecture: [0],
+        Recitation: [3],
+        Tutorial: [21],
+      },
+      CS3216: {
+        Lecture: [0],
+      },
+    };
+    const migratedTaModules: ModuleCode[] = ['CS1010S'];
+
+    expect(firstAction).toEqual({
+      type: 'SET_TIMETABLE',
       payload: {
-        lessons: {
-          [semester]: {
-            CS1010S: {
-              Lecture: [0],
-              Tutorial: [21],
-              Recitation: [3],
-            },
-          },
-        },
-        taModules: {
-          [semester]: ['CS1010S'],
-        },
+        semester,
+        timetable: migratedTimetable,
+        colors,
+        hiddenModules,
+        taModules: migratedTaModules,
       },
     });
   });

website/src/actions/timetables.ts

@@ -1,4 +1,4 @@
-import { each, flatMap } from 'lodash';
+import { each, flatMap, get } from 'lodash';
 
 import type {
   ColorIndex,
@@ -18,27 +18,19 @@ import { getModuleCondensed } from 'selectors/moduleBank';
 import {
   getClosestLessonConfig,
   makeLessonIndicesMap,
-  migrateTimetableConfigs,
+  migrateSemTimetableConfig,
   randomModuleLessonConfig,
   validateModuleLessons,
   validateTimetableModules,
 } from 'utils/timetables';
 import { getModuleSemesterData, getModuleTimetable } from 'utils/modules';
 
 // Actions that should not be used directly outside of thunks
-export const SET_TIMETABLES = 'SET_TIMETABLES' as const;
 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 Internal = {
-  setTimetables(lessons: TimetableConfig, taModules: TaModulesMap) {
-    return {
-      type: SET_TIMETABLES,
-      payload: { lessons, taModules },
-    };
-  },
-
   setTimetable(
     semester: Semester,
     timetable: SemTimetableConfig | undefined,
@@ -232,26 +224,41 @@ 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,
-    );
-
-    if (!alreadyMigrated) dispatch(Internal.setTimetables(lessons, ta));
-
-    // Extract the timetable and the modules for the semester
-    const timetable = lessons[semester];
-    if (!timetable) return;
-    const taModules = ta[semester];
+    const timetableConfig = timetables.lessons as TimetableConfig | TimetableConfigV1;
+    const semTimetableConfig = timetableConfig[semester];
+
+    const taTimetableConfig = timetables.ta as TaModulesMap | TaModulesMapV1;
+    const taModulesConfig = get(taTimetableConfig, semester, {});
+
+    const getModuleSemesterTimetable = (moduleCode: ModuleCode) =>
+      moduleBank.modules[moduleCode]
+        ? getModuleTimetable(moduleBank.modules[moduleCode], semester)
+        : [];
+
+    const {
+      migratedSemTimetableConfig: timetable,
+      migratedTaModulesConfig: ta,
+      alreadyMigrated,
+    } = migrateSemTimetableConfig(semTimetableConfig, taModulesConfig, getModuleSemesterTimetable);
+
+    if (!alreadyMigrated)
+      dispatch(
+        Internal.setTimetable(
+          semester,
+          timetable,
+          timetables.colors[semester],
+          timetables.hidden[semester],
+          ta,
+        ),
+      );
 
     // Check that all lessons for each module are valid. If they are not, we update it
     // such that they are
     each(timetable, (lessonConfig: ModuleLessonConfig, moduleCode: ModuleCode) => {
       const module = moduleBank.modules[moduleCode];
       if (!module) return;
 
-      const isTa = taModules?.includes(moduleCode);
+      const isTa = ta?.includes(moduleCode);
 
       const { validatedLessonConfig, valid } = validateModuleLessons(
         semester,

website/src/reducers/timetables.test.ts

@@ -363,15 +363,3 @@ describe('import timetable', () => {
     });
   });
 });
-
-describe('migrate v1 config', () => {
-  test('should migrate config to new format', () => {
-    expect(
-      reducer(initialState, Internal.setTimetables({ [1]: { CS1010S: {} } }, { [1]: ['CS1010S'] })),
-    ).toStrictEqual({
-      ...initialState,
-      lessons: { [1]: { CS1010S: {} } },
-      ta: { [1]: ['CS1010S'] },
-    });
-  });
-});

website/src/reducers/timetables.ts

@@ -24,7 +24,6 @@ import {
   SET_TIMETABLE,
   SHOW_LESSON_IN_TIMETABLE,
   REMOVE_TA_MODULE,
-  SET_TIMETABLES,
 } from 'actions/timetables';
 import { getNewColor } from 'utils/colors';
 import { SET_EXPORTED_DATA } from 'actions/constants';
@@ -250,15 +249,6 @@ function timetables(
   }
 
   switch (action.type) {
-    case SET_TIMETABLES: {
-      const { lessons, taModules } = action.payload;
-      return {
-        ...state,
-        lessons,
-        ta: taModules,
-      };
-    }
-
     case SET_TIMETABLE: {
       const { semester, timetable, colors, hiddenModules, taModules } = action.payload;
 

website/src/utils/timetables.ts

@@ -46,7 +46,6 @@ import {
   ModuleLessonConfigV1,
   SemTimetableConfigV1,
   TaModulesConfigV1,
-  TimetableConfigV1,
   ColoredLesson,
   HoverLesson,
   InteractableLesson,
@@ -56,13 +55,12 @@ import {
   ModuleLessonConfigWithLessons,
   SemTimetableConfig,
   SemTimetableConfigWithLessons,
-  TimetableConfig,
   TimetableDayArrangement,
   TimetableDayFormat,
   TimetableArrangement,
 } from 'types/timetables';
 
-import { TaModulesMapV1, ModuleCodeMap, ModulesMap, TaModulesMap } from 'types/reducers';
+import { ModuleCodeMap, ModulesMap } from 'types/reducers';
 import { ExamClashes } from 'types/views';
 
 import { getTimeAsDate } from './timify';
@@ -1238,75 +1236,6 @@ export function migrateSemTimetableConfig(
   );
 }
 
-/**
- * Checks the current timetable config and migrate it to v2 format if it is not\
- * Migrates all semesters' timetable config in this academic year
- * @param lessons the academic year's timetables
- * @param ta the academic year's TA modules config
- * @param modules modules in the moduleBank state to use for migration
- * @returns
- * - the migrated timetable config
- * - the migrated TA modules config
- * - whether it was previously migrated, to signal to skip dispatch
- */
-export function migrateTimetableConfigs(
-  lessons: TimetableConfig | TimetableConfigV1,
-  ta: TaModulesMap | TaModulesMapV1,
-  modules: ModulesMap,
-): {
-  lessons: TimetableConfig;
-  ta: TaModulesMap;
-  alreadyMigrated: boolean;
-} {
-  const {
-    config: migratedLessons,
-    ta: migratedTa,
-    alreadyMigrated,
-  } = reduce(
-    lessons,
-    (accumulated, semTimetableConfig, semesterString) => {
-      const semester = parseInt(semesterString, 10);
-      const taModulesConfig = get(ta, semester, {});
-
-      const getModuleSemesterTimetable = (moduleCode: ModuleCode) =>
-        modules[moduleCode] ? getModuleTimetable(modules[moduleCode], semester) : [];
-
-      const migrated = migrateSemTimetableConfig(
-        semTimetableConfig,
-        taModulesConfig,
-        getModuleSemesterTimetable,
-      );
-
-      return {
-        config: {
-          ...accumulated.config,
-          [semester]: migrated.migratedSemTimetableConfig,
-        },
-        ta: {
-          ...accumulated.ta,
-          [semester]: migrated.migratedTaModulesConfig,
-        },
-        alreadyMigrated: migrated.alreadyMigrated && accumulated.alreadyMigrated,
-      };
-    },
-    {
-      config: {},
-      ta: {},
-      alreadyMigrated: true,
-    } as {
-      config: TimetableConfig;
-      ta: TaModulesMap;
-      alreadyMigrated: boolean;
-    },
-  );
-
-  return {
-    lessons: migratedLessons,
-    ta: migratedTa,
-    alreadyMigrated,
-  };
-}
-
 /**
  * Based on what lessons are currently in the lesson config, find the classNo that most of the lessons belong to
  * @param lessonIndicesMap {@link LessonIndicesMap|Lesson indices mapping} of the module