Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(timetable): restore customizable timetables #3483

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions website/src/actions/__snapshots__/timetables.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ exports[`cancelModifyLesson should not have payload 1`] = `
exports[`changeLesson should return updated information to change lesson 1`] = `
{
"payload": {
"activeLesson": "1",
"classNo": "1",
"lessonType": "Recitation",
"moduleCode": "CS1010S",
Expand Down
26 changes: 14 additions & 12 deletions website/src/actions/timetables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ test('modifyLesson should return lesson payload', () => {
test('changeLesson should return updated information to change lesson', () => {
const semester: Semester = 1;
const lesson: Lesson = lessons[1];
expect(actions.changeLesson(semester, lesson)).toMatchSnapshot();
expect(actions.changeLesson(semester, lesson, lesson.classNo)).toMatchSnapshot();
});

test('cancelModifyLesson should not have payload', () => {
Expand All @@ -53,19 +53,21 @@ describe('fillTimetableBlanks', () => {
const moduleBank = { modules: { CS1010S, CS3216 } };
const timetablesState = (semester: Semester, timetable: SemTimetableConfig) => ({
lessons: { [semester]: timetable },
customisedModules: { [semester]: [] },
});
const semester = 1;
const action = actions.validateTimetable(semester);

test('do nothing if timetable is already full', () => {
const timetable = {
const timetable: SemTimetableConfig = {
CS1010S: {
Lecture: '1',
Tutorial: '1',
Recitation: '1',
Lecture: ['1'],
Tutorial: ['1'],
Recitation: ['1'],
},
};

// TODO(zwliew): Correctly type all the `state: any` declarations in this function and the rest of the codebase.
const state: any = { timetables: timetablesState(semester, timetable), moduleBank };
const dispatch = jest.fn();
action(dispatch, () => state);
Expand All @@ -74,10 +76,10 @@ describe('fillTimetableBlanks', () => {
});

test('fill missing lessons with randomly generated modules', () => {
const timetable = {
const timetable: SemTimetableConfig = {
CS1010S: {
Lecture: '1',
Tutorial: '1',
Lecture: ['1'],
Tutorial: ['1'],
},
CS3216: {},
};
Expand All @@ -95,9 +97,9 @@ describe('fillTimetableBlanks', () => {
semester,
moduleCode: 'CS1010S',
lessonConfig: {
Lecture: '1',
Tutorial: '1',
Recitation: expect.any(String),
Lecture: ['1'],
Tutorial: ['1'],
Recitation: expect.any(Array),
},
},
});
Expand All @@ -108,7 +110,7 @@ describe('fillTimetableBlanks', () => {
semester,
moduleCode: 'CS3216',
lessonConfig: {
Lecture: '1',
Lecture: ['1'],
},
},
});
Expand Down
78 changes: 76 additions & 2 deletions website/src/actions/timetables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,86 @@
};
}

export const CUSTOMISE_MODULE = 'CUSTOMISE_LESSON' as const;
export function customiseLesson(semester: Semester, moduleCode: ModuleCode) {
return {

Check warning on line 95 in website/src/actions/timetables.ts

View check run for this annotation

Codecov / codecov/patch

website/src/actions/timetables.ts#L94-L95

Added lines #L94 - L95 were not covered by tests
type: CUSTOMISE_MODULE,
payload: {
semester,
moduleCode,
},
};
}

export const CHANGE_LESSON = 'CHANGE_LESSON' as const;
export function setLesson(
semester: Semester,
moduleCode: ModuleCode,
lessonType: LessonType,
classNo: ClassNo,
activeLesson: ClassNo,
) {
return {
type: CHANGE_LESSON,
payload: {
semester,
moduleCode,
lessonType,
classNo,
activeLesson,
},
};
}

export const ADD_CUSTOM_MODULE = 'ADD_CUSTOM_MODULE' as const;
export function addCustomModule(semester: Semester, moduleCode: ModuleCode) {
return {

Check warning on line 126 in website/src/actions/timetables.ts

View check run for this annotation

Codecov / codecov/patch

website/src/actions/timetables.ts#L125-L126

Added lines #L125 - L126 were not covered by tests
type: ADD_CUSTOM_MODULE,
payload: {
semester,
moduleCode,
},
};
}

export const REMOVE_CUSTOM_MODULE = 'REMOVE_CUSTOM_MODULE' as const;
export function removeCustomModule(semester: Semester, moduleCode: ModuleCode) {
return {

Check warning on line 137 in website/src/actions/timetables.ts

View check run for this annotation

Codecov / codecov/patch

website/src/actions/timetables.ts#L136-L137

Added lines #L136 - L137 were not covered by tests
type: REMOVE_CUSTOM_MODULE,
payload: {
semester,
moduleCode,
},
};
}

export const ADD_LESSON = 'ADD_LESSON' as const;
export function addLesson(
semester: Semester,
moduleCode: ModuleCode,
lessonType: LessonType,
classNo: ClassNo,
) {
return {

Check warning on line 153 in website/src/actions/timetables.ts

View check run for this annotation

Codecov / codecov/patch

website/src/actions/timetables.ts#L152-L153

Added lines #L152 - L153 were not covered by tests
type: ADD_LESSON,
payload: {
semester,
moduleCode,
lessonType,
classNo,
},
};
}

export const REMOVE_LESSON = 'REMOVE_LESSON' as const;
export function removeLesson(
semester: Semester,
moduleCode: ModuleCode,
lessonType: LessonType,
classNo: ClassNo,
) {
return {

Check warning on line 171 in website/src/actions/timetables.ts

View check run for this annotation

Codecov / codecov/patch

website/src/actions/timetables.ts#L170-L171

Added lines #L170 - L171 were not covered by tests
type: REMOVE_LESSON,
payload: {
semester,
moduleCode,
Expand All @@ -108,8 +179,8 @@
};
}

export function changeLesson(semester: Semester, lesson: Lesson) {
return setLesson(semester, lesson.moduleCode, lesson.lessonType, lesson.classNo);
export function changeLesson(semester: Semester, lesson: Lesson, activeLesson: ClassNo) {
return setLesson(semester, lesson.moduleCode, lesson.lessonType, lesson.classNo, activeLesson);
}

export const SET_LESSON_CONFIG = 'SET_LESSON_CONFIG' as const;
Expand Down Expand Up @@ -165,6 +236,9 @@
const module = moduleBank.modules[moduleCode];
if (!module) return;

// Do not validate customised modules.
if (timetables.customisedModules[semester]?.includes(moduleCode)) return;

const [validatedLessonConfig, changedLessonTypes] = validateModuleLessons(
semester,
lessonConfig,
Expand Down
3 changes: 2 additions & 1 deletion website/src/reducers/app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const appInitialState: AppState = {
isFeedbackModalOpen: false,
promptRefresh: false,
notifications: [],
customiseModule: '',
};
const appHasSemesterTwoState: AppState = { ...appInitialState, activeSemester: anotherSemester };
const appHasActiveLessonState: AppState = { ...appInitialState, activeLesson: lesson };
Expand Down Expand Up @@ -55,7 +56,7 @@ test('app should set active lesson', () => {
});

test('app should accept lesson change and unset active lesson', () => {
const action = changeLesson(semester, lesson);
const action = changeLesson(semester, lesson, lesson.classNo);
const nextState: AppState = reducer(appInitialState, action);

expect(nextState).toEqual(appInitialState);
Expand Down
13 changes: 12 additions & 1 deletion website/src/reducers/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
import config from 'config';

import { forceRefreshPrompt } from 'utils/debug';
import { MODIFY_LESSON, CHANGE_LESSON, CANCEL_MODIFY_LESSON } from 'actions/timetables';
import {
MODIFY_LESSON,
CHANGE_LESSON,
CANCEL_MODIFY_LESSON,
CUSTOMISE_MODULE,
} from 'actions/timetables';
import { SELECT_SEMESTER } from 'actions/settings';
import {
OPEN_NOTIFICATION,
Expand All @@ -18,6 +23,7 @@
activeSemester: config.semester,
// The lesson being modified on the timetable.
activeLesson: null,
customiseModule: '',
isOnline: navigator.onLine,
isFeedbackModalOpen: false,
promptRefresh: forceRefreshPrompt(),
Expand All @@ -37,6 +43,11 @@
...state,
activeLesson: action.payload.activeLesson,
};
case CUSTOMISE_MODULE:
return {

Check warning on line 47 in website/src/reducers/app.ts

View check run for this annotation

Codecov / codecov/patch

website/src/reducers/app.ts#L46-L47

Added lines #L46 - L47 were not covered by tests
...state,
customiseModule: action.payload.moduleCode,
};
case CANCEL_MODIFY_LESSON:
case CHANGE_LESSON:
return {
Expand Down
25 changes: 13 additions & 12 deletions website/src/reducers/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ const exportData: ExportData = {
semester: 1,
timetable: {
CS3216: {
Lecture: '1',
Lecture: ['1'],
},
CS1010S: {
Lecture: '1',
Tutorial: '3',
Recitation: '2',
Lecture: ['1'],
Tutorial: ['3'],
Recitation: ['2'],
},
PC1222: {
Lecture: '1',
Tutorial: '3',
Lecture: ['1'],
Tutorial: ['3'],
},
},
colors: {
Expand Down Expand Up @@ -52,16 +52,16 @@ test('reducers should set export data state', () => {
lessons: {
[1]: {
CS3216: {
Lecture: '1',
Lecture: ['1'],
},
CS1010S: {
Lecture: '1',
Tutorial: '3',
Recitation: '2',
Lecture: ['1'],
Tutorial: ['3'],
Recitation: ['2'],
},
PC1222: {
Lecture: '1',
Tutorial: '3',
Lecture: ['1'],
Tutorial: ['3'],
},
},
},
Expand All @@ -72,6 +72,7 @@ test('reducers should set export data state', () => {
PC1222: 2,
},
},
customisedModules: {},
hidden: { [1]: ['PC1222'] },
academicYear: expect.any(String),
archive: {},
Expand Down
Loading