Skip to content

Commit 48070a1

Browse files
zhoukerrrchrisgzf
authored andcommitted
Change ModuleLessonConfig value to array (#3420)
* feat: update lesson to array * chore: fix lint * fix: update test cases * chore: format lint * chore: update test cases * feat: update timetable redux schema * feat: fix lint * chore: fix lint * chore: add timetable schema migration test * fix: add comment for test version --------- Co-authored-by: Christopher Goh <[email protected]>
1 parent a4f1a75 commit 48070a1

File tree

10 files changed

+188
-90
lines changed

10 files changed

+188
-90
lines changed

website/src/actions/timetables.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ describe('fillTimetableBlanks', () => {
6060
test('do nothing if timetable is already full', () => {
6161
const timetable = {
6262
CS1010S: {
63-
Lecture: '1',
64-
Tutorial: '1',
65-
Recitation: '1',
63+
Lecture: ['1'],
64+
Tutorial: ['1'],
65+
Recitation: ['1'],
6666
},
6767
};
6868

@@ -76,8 +76,8 @@ describe('fillTimetableBlanks', () => {
7676
test('fill missing lessons with randomly generated modules', () => {
7777
const timetable = {
7878
CS1010S: {
79-
Lecture: '1',
80-
Tutorial: '1',
79+
Lecture: ['1'],
80+
Tutorial: ['1'],
8181
},
8282
CS3216: {},
8383
};
@@ -95,9 +95,9 @@ describe('fillTimetableBlanks', () => {
9595
semester,
9696
moduleCode: 'CS1010S',
9797
lessonConfig: {
98-
Lecture: '1',
99-
Tutorial: '1',
100-
Recitation: expect.any(String),
98+
Lecture: ['1'],
99+
Tutorial: ['1'],
100+
Recitation: expect.any(Array),
101101
},
102102
},
103103
});
@@ -108,7 +108,7 @@ describe('fillTimetableBlanks', () => {
108108
semester,
109109
moduleCode: 'CS3216',
110110
lessonConfig: {
111-
Lecture: '1',
111+
Lecture: ['1'],
112112
},
113113
},
114114
});

website/src/reducers/index.test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,16 @@ const exportData: ExportData = {
1010
semester: 1,
1111
timetable: {
1212
CS3216: {
13-
Lecture: '1',
13+
Lecture: ['1'],
1414
},
1515
CS1010S: {
16-
Lecture: '1',
17-
Tutorial: '3',
18-
Recitation: '2',
16+
Lecture: ['1'],
17+
Tutorial: ['3'],
18+
Recitation: ['2'],
1919
},
2020
PC1222: {
21-
Lecture: '1',
22-
Tutorial: '3',
21+
Lecture: ['1'],
22+
Tutorial: ['3'],
2323
},
2424
},
2525
colors: {
@@ -52,16 +52,16 @@ test('reducers should set export data state', () => {
5252
lessons: {
5353
[1]: {
5454
CS3216: {
55-
Lecture: '1',
55+
Lecture: ['1'],
5656
},
5757
CS1010S: {
58-
Lecture: '1',
59-
Tutorial: '3',
60-
Recitation: '2',
58+
Lecture: ['1'],
59+
Tutorial: ['3'],
60+
Recitation: ['2'],
6161
},
6262
PC1222: {
63-
Lecture: '1',
64-
Tutorial: '3',
63+
Lecture: ['1'],
64+
Tutorial: ['3'],
6565
},
6666
},
6767
},

website/src/reducers/timetables.test.ts

Lines changed: 70 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import reducer, { defaultTimetableState, persistConfig } from 'reducers/timetables';
1+
import reducer, { defaultTimetableState, migrateV1toV2, persistConfig } from 'reducers/timetables';
22
import {
33
ADD_MODULE,
44
hideLessonInTimetable,
@@ -125,41 +125,41 @@ describe('lesson reducer', () => {
125125
lessons: {
126126
[1]: {
127127
CS1010S: {
128-
Lecture: '1',
129-
Recitation: '2',
128+
Lecture: ['1'],
129+
Recitation: ['2'],
130130
},
131131
CS3216: {
132-
Lecture: '1',
132+
Lecture: ['1'],
133133
},
134134
},
135135
[2]: {
136136
CS3217: {
137-
Lecture: '1',
137+
Lecture: ['1'],
138138
},
139139
},
140140
},
141141
},
142142
setLessonConfig(1, 'CS1010S', {
143-
Lecture: '2',
144-
Recitation: '3',
145-
Tutorial: '4',
143+
Lecture: ['2'],
144+
Recitation: ['3'],
145+
Tutorial: ['4'],
146146
}),
147147
),
148148
).toMatchObject({
149149
lessons: {
150150
[1]: {
151151
CS1010S: {
152-
Lecture: '2',
153-
Recitation: '3',
154-
Tutorial: '4',
152+
Lecture: ['2'],
153+
Recitation: ['3'],
154+
Tutorial: ['4'],
155155
},
156156
CS3216: {
157-
Lecture: '1',
157+
Lecture: ['1'],
158158
},
159159
},
160160
[2]: {
161161
CS3217: {
162-
Lecture: '1',
162+
Lecture: ['1'],
163163
},
164164
},
165165
},
@@ -172,7 +172,7 @@ describe('stateReconciler', () => {
172172
'2015/2016': {
173173
[1]: {
174174
GET1006: {
175-
Lecture: '1',
175+
Lecture: ['1'],
176176
},
177177
},
178178
},
@@ -181,13 +181,13 @@ describe('stateReconciler', () => {
181181
const oldLessons = {
182182
[1]: {
183183
CS1010S: {
184-
Lecture: '1',
185-
Recitation: '2',
184+
Lecture: ['1'],
185+
Recitation: ['2'],
186186
},
187187
},
188188
[2]: {
189189
CS3217: {
190-
Lecture: '1',
190+
Lecture: ['1'],
191191
},
192192
},
193193
};
@@ -239,3 +239,56 @@ describe('stateReconciler', () => {
239239
});
240240
});
241241
});
242+
243+
describe('redux schema migration', () => {
244+
const reduxDataV1 = {
245+
lessons: {
246+
[1]: {
247+
CS1010S: {
248+
Lecture: '1',
249+
Recitation: '2',
250+
},
251+
},
252+
[2]: {
253+
CS3217: {
254+
Lecture: '1',
255+
},
256+
},
257+
},
258+
colors: {},
259+
hidden: {},
260+
academicYear: '2022/2023',
261+
archive: {},
262+
_persist: {
263+
version: 1,
264+
rehydrated: false,
265+
},
266+
};
267+
268+
const reduxDataV2 = {
269+
lessons: {
270+
[1]: {
271+
CS1010S: {
272+
Lecture: ['1'],
273+
Recitation: ['2'],
274+
},
275+
},
276+
[2]: {
277+
CS3217: {
278+
Lecture: ['1'],
279+
},
280+
},
281+
},
282+
colors: {},
283+
hidden: {},
284+
academicYear: '2022/2023',
285+
archive: {},
286+
_persist: {
287+
version: 1, // version kept the same because the framework does not support it in unit tests
288+
rehydrated: false,
289+
},
290+
};
291+
test('should migrate from V1 to V2', () => {
292+
expect(migrateV1toV2(reduxDataV1)).toEqual(reduxDataV2);
293+
});
294+
});

website/src/reducers/timetables.ts

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { get, omit, values } from 'lodash';
22
import produce from 'immer';
3-
import { createMigrate } from 'redux-persist';
3+
import { createMigrate, PersistedState } from 'redux-persist';
44

55
import { PersistConfig } from 'storage/persistReducer';
66
import { ModuleCode } from 'types/modules';
7-
import { ModuleLessonConfig, SemTimetableConfig } from 'types/timetables';
7+
import { ModuleLessonConfig, SemTimetableConfig, TimetableConfig } from 'types/timetables';
88
import { ColorMapping, TimetablesState } from 'types/reducers';
99

1010
import config from 'config';
@@ -22,6 +22,40 @@ import { getNewColor } from 'utils/colors';
2222
import { SET_EXPORTED_DATA } from 'actions/constants';
2323
import { Actions } from '../types/actions';
2424

25+
// Migration from state V1 -> V2
26+
type TimetableStateV1 = Omit<TimetablesState, 'lessons'> & {
27+
lessons: { [semester: string]: { [moduleCode: string]: { [lessonType: string]: string } } };
28+
};
29+
export function migrateV1toV2(
30+
oldState: TimetableStateV1 & PersistedState,
31+
): TimetablesState & PersistedState {
32+
const newLessons: TimetableConfig = {};
33+
const oldLessons = oldState.lessons;
34+
35+
Object.entries(oldLessons).forEach(([semester, modules]) => {
36+
Object.entries(modules).forEach(([moduleCode, lessons]) => {
37+
const newSemester: { [moduleCode: string]: { [lessonType: string]: string[] } } = {
38+
[moduleCode]: {},
39+
};
40+
41+
Object.entries(lessons).forEach(([lessonType, lessonValue]) => {
42+
const lessonArray = [lessonValue];
43+
newSemester[moduleCode][lessonType] = lessonArray;
44+
});
45+
46+
if (!newLessons[semester]) {
47+
newLessons[semester] = {};
48+
}
49+
Object.assign(newLessons[semester], newSemester);
50+
});
51+
});
52+
53+
return {
54+
...oldState,
55+
lessons: newLessons,
56+
};
57+
}
58+
2559
export const persistConfig = {
2660
/* eslint-disable no-useless-computed-key */
2761
migrate: createMigrate({
@@ -34,9 +68,12 @@ export const persistConfig = {
3468
// eslint-disable-next-line no-underscore-dangle, @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-non-null-asserted-optional-chain
3569
_persist: state?._persist!,
3670
}),
71+
// Same as planner.ts
72+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
73+
[2]: migrateV1toV2 as any,
3774
}),
3875
/* eslint-enable */
39-
version: 1,
76+
version: 2,
4077

4178
// Our own state reconciler archives old timetables if the acad year is different,
4279
// otherwise use the persisted timetable state
@@ -82,7 +119,7 @@ function moduleLessonConfig(
82119
if (!(classNo && lessonType)) return state;
83120
return {
84121
...state,
85-
[lessonType]: classNo,
122+
[lessonType]: [classNo],
86123
};
87124
}
88125
case SET_LESSON_CONFIG:

website/src/types/timetables.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ClassNo, LessonType, ModuleCode, ModuleTitle, RawLesson } from './modul
22

33
// ModuleLessonConfig is a mapping of lessonType to ClassNo for a module.
44
export type ModuleLessonConfig = {
5-
[lessonType: string]: ClassNo;
5+
[lessonType: string]: ClassNo[];
66
};
77

88
// SemTimetableConfig is the timetable data for each semester.

0 commit comments

Comments
 (0)