diff --git a/website/src/__mocks__/module-list.json b/website/src/__mocks__/module-code-map.json similarity index 100% rename from website/src/__mocks__/module-list.json rename to website/src/__mocks__/module-code-map.json diff --git a/website/src/__mocks__/moduleList.json b/website/src/__mocks__/moduleList.json new file mode 100644 index 0000000000..cba30628f4 --- /dev/null +++ b/website/src/__mocks__/moduleList.json @@ -0,0 +1,17 @@ +[ + { "moduleCode": "ACC2002", "title": "Managerial Accounting", "semesters": [1, 2] }, + { + "moduleCode": "BFS1001", + "title": "Personal Development & Career Management", + "semesters": [1, 2] + }, + { "moduleCode": "CS1010S", "title": "Programming Methodology", "semesters": [1, 2] }, + { + "moduleCode": "CS3216", + "title": "Software Product Engineering for Digital Markets", + "semesters": [1] + }, + { "moduleCode": "CS4243", "title": "Computer Vision and Pattern Recognition", "semesters": [1] }, + { "moduleCode": "GER1000", "title": "Quantitative Reasoning", "semesters": [1, 2] }, + { "moduleCode": "GES1021", "title": "Natural Heritage of Singapore", "semesters": [1, 2] } +] diff --git a/website/src/__mocks__/modules/index.ts b/website/src/__mocks__/modules/index.ts index 6576a4431b..21e3d5a4bc 100644 --- a/website/src/__mocks__/modules/index.ts +++ b/website/src/__mocks__/modules/index.ts @@ -1,4 +1,4 @@ -import { Module } from 'types/modules'; +import type { Module } from 'types/modules'; import ACC2002_JSON from './ACC2002.json'; import BFS1001_JSON from './BFS1001.json'; diff --git a/website/src/__mocks__/react-feather.tsx b/website/src/__mocks__/react-feather.tsx new file mode 100644 index 0000000000..dc9445cc11 --- /dev/null +++ b/website/src/__mocks__/react-feather.tsx @@ -0,0 +1,9 @@ +import { mapValues } from 'lodash'; +import type { ComponentType } from 'react'; +import * as feather from 'react-feather'; + +module.exports = mapValues(feather, (_component, name) => { + const MockComponent = jest.fn(() =>
); + (MockComponent as ComponentType).displayName = name; + return MockComponent; +}); diff --git a/website/src/actions/export.ts b/website/src/actions/export.ts index 9cab32c20d..8a967f7434 100644 --- a/website/src/actions/export.ts +++ b/website/src/actions/export.ts @@ -2,8 +2,9 @@ import type { Module, Semester } from 'types/modules'; import type { ExportData } from 'types/export'; import type { Dispatch, GetState } from 'types/redux'; import { hydrateSemTimetableWithLessons } from 'utils/timetables'; -import { captureException, retryImport } from 'utils/error'; -import { getSemesterTimetable } from 'selectors/timetables'; +import { captureException } from 'utils/error'; +import retryImport from 'utils/retryImport'; +import { getSemesterTimetableLessons } from 'selectors/timetables'; import { SET_EXPORTED_DATA } from './constants'; function downloadUrl(blob: Blob, filename: string) { @@ -28,11 +29,10 @@ export function downloadAsIcal(semester: Semester) { retryImport(() => import(/* webpackChunkName: "export" */ 'utils/ical')), ]) .then(([ical, icalUtils]) => { - const { - moduleBank: { modules }, - timetables, - } = getState(); - const { timetable } = getSemesterTimetable(semester, timetables); + const state = getState(); + const { modules } = state.moduleBank; + + const timetable = getSemesterTimetableLessons(state)(semester); const timetableWithLessons = hydrateSemTimetableWithLessons(timetable, modules, semester); const events = icalUtils.default(semester, timetableWithLessons, modules); diff --git a/website/src/actions/timetables.ts b/website/src/actions/timetables.ts index 902beea8e2..3378567c52 100644 --- a/website/src/actions/timetables.ts +++ b/website/src/actions/timetables.ts @@ -181,7 +181,7 @@ export function validateTimetable(semester: Semester) { export function fetchTimetableModules(timetables: SemTimetableConfig[]) { return (dispatch: Dispatch, getState: GetState) => { const moduleCodes = new Set(flatMap(timetables, Object.keys)); - const validateModule = getModuleCondensed(getState().moduleBank); + const validateModule = getModuleCondensed(getState()); return Promise.all( Array.from(moduleCodes) diff --git a/website/src/reducers/moduleBank.ts b/website/src/reducers/moduleBank.ts index 3960cb4b84..1242838e97 100644 --- a/website/src/reducers/moduleBank.ts +++ b/website/src/reducers/moduleBank.ts @@ -1,6 +1,10 @@ import produce, { Draft } from 'immer'; import { keyBy, omit, size, zipObject } from 'lodash'; +import type { Actions } from 'types/actions'; +import type { Module } from 'types/modules'; +import type { ModuleBank, ModuleList } from 'types/reducers'; + import { FETCH_ARCHIVE_MODULE, FETCH_MODULE, @@ -10,12 +14,8 @@ import { SET_EXPORTED_DATA, } from 'actions/constants'; import { createMigrate, REHYDRATE } from 'redux-persist'; -import { Module } from 'types/modules'; -import { ModuleBank, ModuleList } from 'types/reducers'; import { SUCCESS_KEY } from 'middlewares/requests-middleware'; -import { Actions } from 'types/actions'; - const defaultModuleBankState: ModuleBank = { moduleList: [], // List of basic modules data (module code, name, semester) modules: {}, // Object of Module code -> Module details diff --git a/website/src/selectors/moduleBank.test.ts b/website/src/selectors/moduleBank.test.ts index f6fdf9d769..b254065bd4 100644 --- a/website/src/selectors/moduleBank.test.ts +++ b/website/src/selectors/moduleBank.test.ts @@ -3,8 +3,10 @@ import { getModuleCondensed } from 'selectors/moduleBank'; describe(getModuleCondensed, () => { test('should return a function that determines if the given module code is valid', () => { const state: any = { - moduleCodes: { - CS1010S: {}, + moduleBank: { + moduleCodes: { + CS1010S: {}, + }, }, }; diff --git a/website/src/selectors/moduleBank.ts b/website/src/selectors/moduleBank.ts index cc57d7087d..7d34d80886 100644 --- a/website/src/selectors/moduleBank.ts +++ b/website/src/selectors/moduleBank.ts @@ -1,19 +1,18 @@ import { createSelector } from 'reselect'; -import { ModuleCode, ModuleCondensed, Semester } from 'types/modules'; -import { ModuleBank, ModuleCodeMap, ModuleSelectListItem } from 'types/reducers'; -import { SemTimetableConfig } from 'types/timetables'; +import type { ModuleCode, ModuleCondensed, Semester } from 'types/modules'; +import type { ModuleCodeMap, ModuleSelectListItem } from 'types/reducers'; +import type { SemTimetableConfig } from 'types/timetables'; +import type { State } from 'types/state'; + import { notNull } from 'types/utils'; -import { State } from 'types/state'; import { isOngoing } from './requests'; import { getRequestModuleCode } from '../actions/constants'; -const moduleCodesSelector = (state: ModuleBank) => state.moduleCodes; - // Returns a getter that returns module condensed given a module code export type ModuleCondensedGetter = (moduleCode: ModuleCode) => ModuleCondensed | undefined; export const getModuleCondensed = createSelector( - moduleCodesSelector, + ({ moduleBank }: State) => moduleBank.moduleCodes, (moduleCodes: ModuleCodeMap): ModuleCondensedGetter => (moduleCode: ModuleCode) => moduleCodes[moduleCode], ); diff --git a/website/src/selectors/timetables.ts b/website/src/selectors/timetables.ts index 69bcf97bbe..75a5d719a9 100644 --- a/website/src/selectors/timetables.ts +++ b/website/src/selectors/timetables.ts @@ -1,10 +1,11 @@ -import { ModuleCode, Semester } from 'types/modules'; +import { createSelector } from 'reselect'; + +import type { ModuleCode, Semester } from 'types/modules'; +import type { State } from 'types/state'; + +import { fetchArchiveRequest } from 'actions/constants'; import config from 'config'; import { isOngoing, isSuccess } from 'selectors/requests'; -import { State } from 'types/state'; -import { fetchArchiveRequest } from 'actions/constants'; -import { ColorMapping, TimetablesState } from 'types/reducers'; -import { SemTimetableConfig } from 'types/timetables'; export function isArchiveLoading(state: State, moduleCode: ModuleCode) { return config.archiveYears.some((year) => @@ -18,14 +19,22 @@ export function availableArchive(state: State, moduleCode: ModuleCode): string[] ); } -// Extract sem timetable and colors for a specific semester from TimetablesState const EMPTY_OBJECT = {}; -export function getSemesterTimetable( - semester: Semester, - state: TimetablesState, -): { timetable: SemTimetableConfig; colors: ColorMapping } { - return { - timetable: state.lessons[semester] || EMPTY_OBJECT, - colors: state.colors[semester] || EMPTY_OBJECT, - }; -} + +/** + * Extract semester timetable lessons for a specific semester. + */ +export const getSemesterTimetableLessons = createSelector( + ({ timetables }: State) => timetables.lessons, + (lessons) => (semester: Semester | null) => + semester === null ? EMPTY_OBJECT : lessons[semester] ?? EMPTY_OBJECT, +); + +/** + * Extract semester timetable colors for a specific semester. + */ +export const getSemesterTimetableColors = createSelector( + ({ timetables }: State) => timetables.colors, + (colors) => (semester: Semester | null) => + semester === null ? EMPTY_OBJECT : colors[semester] ?? EMPTY_OBJECT, +); diff --git a/website/src/test-utils/createHistory.ts b/website/src/test-utils/createHistory.ts index 897d13de6a..bfa51da819 100644 --- a/website/src/test-utils/createHistory.ts +++ b/website/src/test-utils/createHistory.ts @@ -11,17 +11,13 @@ type MatchShape = { isExact?: boolean; }; -// This can also be Location, but no test case use that for now so we leave it -// out for simplicity -type HistoryEntry = string; - // eslint-disable-next-line @typescript-eslint/ban-types export default function createHistory( - initialEntries: HistoryEntry | Readonly = '/', + initialEntries: string | string[] = '/', matchParams: MatchShape = {}, ): RouteComponentProps { const entries = _.castArray(initialEntries); - const history = createMemoryHistory({ initialEntries: entries as any }); + const history = createMemoryHistory({ initialEntries: entries }); const { params = {}, isExact = true } = matchParams; const match: Match = { diff --git a/website/src/test-utils/renderWithRouterMatch.tsx b/website/src/test-utils/renderWithRouterMatch.tsx new file mode 100644 index 0000000000..281b2f6e8a --- /dev/null +++ b/website/src/test-utils/renderWithRouterMatch.tsx @@ -0,0 +1,32 @@ +import { render } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { Route, Router } from 'react-router-dom'; +import createHistory from './createHistory'; + +/** + * `render` `children` in a `Router` and `Route` so that `children` have + * populated route matches when using React Router. + * + * Inspiration: https://spectrum.chat/testing-library/help-react/attempting-to-test-react-router-match~b0550426-f54a-4b76-b402-c7b32204b55e?m=MTU2OTM1MzY4NjUwNw== + */ +export default function renderWithRouterMatch( + children: ReactNode, + { + path = '/', + location, + }: { + path?: string; + location?: Parameters[0]; + }, +) { + const { history } = createHistory(location); + const view = render( + + {children} + , + ); + return { + history, + view, + }; +} diff --git a/website/src/types/reducers.ts b/website/src/types/reducers.ts index e9ab2a4baf..d87e3a9b6c 100644 --- a/website/src/types/reducers.ts +++ b/website/src/types/reducers.ts @@ -177,7 +177,7 @@ export type ModuleCodeMap = { [moduleCode: string]: ModuleCondensed }; export type ModuleArchive = { [moduleCode: string]: { // Mapping acad year to module info - [key: string]: Module; + [acadYear: string]: Module; }; }; diff --git a/website/src/utils/__mocks__/error.ts b/website/src/utils/__mocks__/error.ts index f583c3dd23..fe8f44b778 100644 --- a/website/src/utils/__mocks__/error.ts +++ b/website/src/utils/__mocks__/error.ts @@ -1,3 +1,2 @@ export const captureException = jest.fn(); export const getScriptErrorHandler = jest.fn().mockReturnValue(() => jest.fn()); -export const retryImport = jest.fn().mockResolvedValue(undefined); diff --git a/website/src/utils/error.ts b/website/src/utils/error.ts index 5d6c991ac5..cc77132d07 100644 --- a/website/src/utils/error.ts +++ b/website/src/utils/error.ts @@ -1,6 +1,5 @@ import * as Sentry from '@sentry/browser'; import { each, size } from 'lodash'; -import { retry } from 'utils/promise'; export function captureException(error: Error, extra: { [key: string]: unknown } = {}) { Sentry.withScope((scope) => { @@ -30,15 +29,3 @@ export function getScriptErrorHandler(scriptName: string) { } }; } - -/** - * Wrap an async import() so that it automatically retries in case of a chunk - * load error and when the user is online - */ -export function retryImport(importFactory: () => Promise, retries = 3) { - return retry( - retries, - importFactory, - (error) => error.message.includes('Loading chunk ') && window.navigator.onLine, - ); -} diff --git a/website/src/utils/export.ts b/website/src/utils/export.ts index 1d6dddecea..7b0e97d047 100644 --- a/website/src/utils/export.ts +++ b/website/src/utils/export.ts @@ -1,6 +1,6 @@ import { Semester } from 'types/modules'; import { ExportData } from 'types/export'; -import { getSemesterTimetable } from 'selectors/timetables'; +import { getSemesterTimetableColors } from 'selectors/timetables'; import { State } from 'types/state'; import { SemTimetableConfig } from 'types/timetables'; @@ -9,7 +9,7 @@ export function extractStateForExport( timetable: SemTimetableConfig, state: State, ): ExportData { - const { colors } = getSemesterTimetable(semester, state.timetables); + const colors = getSemesterTimetableColors(state)(semester); const hidden = state.timetables.hidden[semester] || []; return { diff --git a/website/src/utils/retryImport.ts b/website/src/utils/retryImport.ts new file mode 100644 index 0000000000..b7763ce812 --- /dev/null +++ b/website/src/utils/retryImport.ts @@ -0,0 +1,13 @@ +import { retry } from 'utils/promise'; + +/** + * Wrap an async import() so that it automatically retries in case of a chunk + * load error and when the user is online + */ +export default function retryImport(importFactory: () => Promise, retries = 3) { + return retry( + retries, + importFactory, + (error) => error.message.includes('Loading chunk ') && window.navigator.onLine, + ); +} diff --git a/website/src/utils/timetables.test.ts b/website/src/utils/timetables.test.ts index a0f3867ab1..0953a72dea 100644 --- a/website/src/utils/timetables.test.ts +++ b/website/src/utils/timetables.test.ts @@ -16,7 +16,7 @@ import _ from 'lodash'; import { getModuleSemesterData, getModuleTimetable } from 'utils/modules'; import { CS1010S, CS3216, CS4243, PC1222 } from '__mocks__/modules'; -import modulesListJSON from '__mocks__/module-list.json'; +import moduleCodeMapJSON from '__mocks__/module-code-map.json'; import timetable from '__mocks__/sem-timetable.json'; import lessonsArray from '__mocks__/lessons-array.json'; @@ -54,7 +54,7 @@ import { } from './timetables'; // TODO: Fix this later -const modulesList = modulesListJSON as any; +const moduleCodeMap = moduleCodeMapJSON as any; describe(isValidSemester, () => { test('semesters 1-4 are valid', () => { @@ -471,14 +471,14 @@ test('isSameTimetableConfig', () => { describe(validateTimetableModules, () => { test('should leave valid modules untouched', () => { - expect(validateTimetableModules({}, modulesList)).toEqual([{}, []]); + expect(validateTimetableModules({}, moduleCodeMap)).toEqual([{}, []]); expect( validateTimetableModules( { CS1010S: {}, CS2100: {}, }, - modulesList, + moduleCodeMap, ), ).toEqual([{ CS1010S: {}, CS2100: {} }, []]); }); @@ -490,7 +490,7 @@ describe(validateTimetableModules, () => { DEADBEEF: {}, CS2100: {}, }, - modulesList, + moduleCodeMap, ), ).toEqual([{ CS2100: {} }, ['DEADBEEF']]); }); diff --git a/website/src/views/components/LinkModuleCodes.test.tsx b/website/src/views/components/LinkModuleCodes.test.tsx index 7960399e34..02cf24e3f6 100644 --- a/website/src/views/components/LinkModuleCodes.test.tsx +++ b/website/src/views/components/LinkModuleCodes.test.tsx @@ -44,7 +44,9 @@ describe(LinkModuleCodesComponent, () => { function create(content: string, moduleCodes: ModuleCodeMap = {}) { const getModule = getModuleCondensed({ - moduleCodes, + moduleBank: { + moduleCodes, + }, } as any); return mount( diff --git a/website/src/views/components/LinkModuleCodes.tsx b/website/src/views/components/LinkModuleCodes.tsx index 828f30b74c..ebcfa95fff 100644 --- a/website/src/views/components/LinkModuleCodes.tsx +++ b/website/src/views/components/LinkModuleCodes.tsx @@ -44,7 +44,7 @@ export const LinkModuleCodesComponent: React.FC = (props) => { }; const mapStateToProps = connect((state: State) => ({ - getModuleCondensed: getModuleCondensed(state.moduleBank), + getModuleCondensed: getModuleCondensed(state), })); export default mapStateToProps(LinkModuleCodesComponent); diff --git a/website/src/views/components/Tooltip/index.tsx b/website/src/views/components/Tooltip/index.tsx index 0db8b7c30f..f3d7ec1dd9 100644 --- a/website/src/views/components/Tooltip/index.tsx +++ b/website/src/views/components/Tooltip/index.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; -import { retryImport } from 'utils/error'; +import retryImport from 'utils/retryImport'; import { Props, TooltipGroupProps } from './Tooltip'; diff --git a/website/src/views/components/__mocks__/RandomKawaii.tsx b/website/src/views/components/__mocks__/RandomKawaii.tsx new file mode 100644 index 0000000000..89be0c8a5a --- /dev/null +++ b/website/src/views/components/__mocks__/RandomKawaii.tsx @@ -0,0 +1,4 @@ +import type { FC } from 'react'; + +const MockRandomKawaii: FC = jest.fn(() =>
); +export default MockRandomKawaii; diff --git a/website/src/views/components/module-info/ModuleExamClash.tsx b/website/src/views/components/module-info/ModuleExamClash.tsx index 6ac8dba8dd..58ef8f6374 100644 --- a/website/src/views/components/module-info/ModuleExamClash.tsx +++ b/website/src/views/components/module-info/ModuleExamClash.tsx @@ -8,7 +8,7 @@ import { Module, ModuleCode, Semester } from 'types/modules'; import { AlertTriangle } from 'react-feather'; import { getModuleSemesterData } from 'utils/modules'; import { getSemesterModules } from 'utils/timetables'; -import { getSemesterTimetable } from 'selectors/timetables'; +import { getSemesterTimetableLessons } from 'selectors/timetables'; import LinkModuleCodes from 'views/components/LinkModuleCodes'; import { State } from 'types/state'; @@ -60,7 +60,7 @@ export const ModuleExamClashComponent: React.FC = ({ }; export default connect((state: State, ownProps: OwnProps) => { - const { timetable } = getSemesterTimetable(ownProps.semester, state.timetables); + const timetable = getSemesterTimetableLessons(state)(ownProps.semester); const modulesMap = state.moduleBank.modules; return { modules: getSemesterModules(timetable, modulesMap) }; })(React.memo(ModuleExamClashComponent)); diff --git a/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx b/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx index 71457187c9..c6cac66b74 100644 --- a/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx +++ b/website/src/views/contribute/ContributeContainer/ContributeContainer.tsx @@ -316,7 +316,7 @@ const ContributeContainer: FC = ({ modules, beta, ...props }) => { const ConnectedContributeContainer = connect( (state: StoreState) => { - const getModule = getModuleCondensed(state.moduleBank); + const getModule = getModuleCondensed(state); const modules: SemesterModules = mapValues( state.timetables.lessons, (timetable): ModuleCondensed[] => Object.keys(timetable).map(getModule).filter(notNull), diff --git a/website/src/views/contribute/ContributeContainer/index.tsx b/website/src/views/contribute/ContributeContainer/index.tsx index 4a2cb6e819..fe585fba38 100644 --- a/website/src/views/contribute/ContributeContainer/index.tsx +++ b/website/src/views/contribute/ContributeContainer/index.tsx @@ -2,7 +2,7 @@ import Loadable, { LoadingComponentProps } from 'react-loadable'; import LoadingSpinner from 'views/components/LoadingSpinner'; import ApiError from 'views/errors/ApiError'; -import { retryImport } from 'utils/error'; +import retryImport from 'utils/retryImport'; const AsyncContributeContainer = Loadable({ loader: () => diff --git a/website/src/views/layout/Navtabs.test.tsx b/website/src/views/layout/Navtabs.test.tsx index d2bb9e2d9e..4fc7654ad6 100644 --- a/website/src/views/layout/Navtabs.test.tsx +++ b/website/src/views/layout/Navtabs.test.tsx @@ -1,13 +1,67 @@ -import { shallow } from 'enzyme'; +import { render, screen } from '@testing-library/react'; +import configureStore from 'bootstrapping/configure-store'; +import produce from 'immer'; +import React from 'react'; +import { Provider } from 'react-redux'; +import { MemoryRouter } from 'react-router-dom'; +import reducers from 'reducers'; +import { initAction } from 'test-utils/redux'; -import createHistory from 'test-utils/createHistory'; -import { NavtabsComponent } from './Navtabs'; +import Navtabs from './Navtabs'; -describe(NavtabsComponent, () => { - test('renders into nav element', () => { - const navtabs = shallow( - , - ); - expect(navtabs).toMatchSnapshot(); +const relevantStoreContents = { + app: { activeSemester: 1 }, + settings: { beta: false }, +}; + +const initialState = reducers(undefined, initAction()); + +function make(storeOverrides: Partial = {}) { + const { store } = configureStore( + produce(initialState, (draft) => { + draft.app.activeSemester = + storeOverrides.app?.activeSemester ?? relevantStoreContents.app.activeSemester; + draft.settings.beta = storeOverrides.settings?.beta ?? relevantStoreContents.settings.beta; + }), + ); + render( + + + , + + , + ); +} + +describe(Navtabs, () => { + test('should render into nav element', () => { + make(); + expect(screen.getAllByRole('link').map((elem) => elem.textContent)).toMatchInlineSnapshot(` + Array [ + "Today", + "Timetable", + "Modules", + "Venues", + "Settings", + "Contribute", + "Whispers", + ] + `); + }); + + test('should show beta tabs if beta is true', () => { + make({ settings: { beta: true } }); + expect(screen.getAllByRole('link').map((elem) => elem.textContent)).toMatchInlineSnapshot(` + Array [ + "Today", + "Timetable", + "Modules", + "Venues", + "Planner", + "Settings", + "Contribute", + "Whispers", + ] + `); }); }); diff --git a/website/src/views/layout/Navtabs.tsx b/website/src/views/layout/Navtabs.tsx index bde2fb499a..e9ddd0f6ec 100644 --- a/website/src/views/layout/Navtabs.tsx +++ b/website/src/views/layout/Navtabs.tsx @@ -1,27 +1,24 @@ -import * as React from 'react'; -import { connect } from 'react-redux'; -import { NavLink, RouteComponentProps, withRouter } from 'react-router-dom'; +import type { FC } from 'react'; +import { useSelector } from 'react-redux'; +import { NavLink } from 'react-router-dom'; import classnames from 'classnames'; import { BookOpen, Calendar, Clock, Heart, Map, Settings, Star, Trello } from 'react-feather'; -import { Semester } from 'types/modules'; import ExternalLink from 'views/components/ExternalLink'; import { timetablePage } from 'views/routes/paths'; import { preload as preloadToday } from 'views/today/TodayContainer'; import { preload as preloadVenues } from 'views/venues/VenuesContainer'; import { preload as preloadContribute } from 'views/contribute/ContributeContainer'; -import { State } from 'types/state'; +import type { State } from 'types/state'; import styles from './Navtabs.scss'; export const NAVTAB_HEIGHT = 48; -type Props = RouteComponentProps & { - activeSemester: Semester; - beta: boolean; -}; +const Navtabs: FC = () => { + const activeSemester = useSelector(({ app }: State) => app.activeSemester); + const beta = useSelector(({ settings }: State) => settings.beta); -export const NavtabsComponent: React.FC = (props) => { const tabProps = { className: styles.link, activeClassName: styles.linkActive, @@ -33,7 +30,7 @@ export const NavtabsComponent: React.FC = (props) => { Today - + Timetable @@ -48,7 +45,7 @@ export const NavtabsComponent: React.FC = (props) => { Venues - {props.beta && ( + {beta && ( = (props) => { ); }; -const connectedNavtabs = connect((state: State) => ({ - activeSemester: state.app.activeSemester, - beta: !!state.settings.beta, -}))(NavtabsComponent); - -export default withRouter(connectedNavtabs); +export default Navtabs; diff --git a/website/src/views/layout/NoFooter.tsx b/website/src/views/layout/NoFooter.tsx index a110054f8a..7f509f2f3c 100644 --- a/website/src/views/layout/NoFooter.tsx +++ b/website/src/views/layout/NoFooter.tsx @@ -1,10 +1,10 @@ -import * as React from 'react'; +import type { FC } from 'react'; import styles from './NoFooter.scss'; /** * Stupid hack to hide footer using a position fixed element that hides the * footer. */ -const NoFooter: React.FC = () =>
; +const NoFooter: FC = () =>
; export default NoFooter; diff --git a/website/src/views/layout/__snapshots__/Navtabs.test.tsx.snap b/website/src/views/layout/__snapshots__/Navtabs.test.tsx.snap deleted file mode 100644 index f589c1e292..0000000000 --- a/website/src/views/layout/__snapshots__/Navtabs.test.tsx.snap +++ /dev/null @@ -1,105 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`NavtabsComponent renders into nav element 1`] = ` - -`; diff --git a/website/src/views/modules/ModuleArchiveContainer.test.tsx b/website/src/views/modules/ModuleArchiveContainer.test.tsx index dfbbf17919..982d4422b2 100644 --- a/website/src/views/modules/ModuleArchiveContainer.test.tsx +++ b/website/src/views/modules/ModuleArchiveContainer.test.tsx @@ -1,74 +1,106 @@ -import { shallow, ShallowWrapper } from 'enzyme'; -import { Redirect } from 'react-router-dom'; - -import { Module, ModuleCode } from 'types/modules'; -import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; -import LoadingSpinner from 'views/components/LoadingSpinner'; -import createHistory from 'test-utils/createHistory'; -import { waitFor } from 'test-utils/async'; -import ApiError from 'views/errors/ApiError'; +import { screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import axios, { AxiosError, AxiosResponse } from 'axios'; + +import configureStore from 'bootstrapping/configure-store'; +import reducers from 'reducers'; +import { mockDom, mockDomReset } from 'test-utils/mockDom'; +import { initAction } from 'test-utils/redux'; +import renderWithRouterMatch from 'test-utils/renderWithRouterMatch'; import { CS1010S } from '__mocks__/modules'; import { ModuleArchiveContainerComponent } from './ModuleArchiveContainer'; +jest.mock('views/components/RandomKawaii'); jest.mock('utils/error'); -const CANONICAL = '/archive/CS1010S/2019-2020/programming-methodology'; +const cs1010sResponse: AxiosResponse = { + data: CS1010S, + status: 200, + statusText: 'Ok', + headers: {}, + config: {}, +}; -type MakeContainerOptions = { - module: Module | null; - fetchModule: () => Promise; - archiveYear: string; +const notFoundError: Partial = { + response: { + data: undefined, + status: 404, + statusText: 'Not found', + headers: {}, + config: {}, + }, }; -function make(moduleCode: ModuleCode, url: string, options: Partial = {}) { - const props: MakeContainerOptions = { - module: null, - fetchModule: () => Promise.resolve(), - archiveYear: '2019/2020', - ...options, - }; +const someOtherError: Partial = { + response: { + data: undefined, + status: 500, + statusText: 'Test error', + headers: {}, + config: {}, + }, +}; - return shallow( - , - ); -} +const CANONICAL = '/archive/CS1010S/2017-2018/programming-methodology'; + +const initialState = reducers(undefined, initAction()); + +function make(location: string = CANONICAL) { + const { store } = configureStore(initialState); -function assertRedirect(component: ShallowWrapper, redirectTo = CANONICAL) { - expect(component.type()).toEqual(Redirect); - expect(component.props()).toMatchObject({ to: { pathname: redirectTo } }); + return renderWithRouterMatch( + + + , + { + path: '/archive/:moduleCode/:year/:slug?', + location, + }, + ); } describe(ModuleArchiveContainerComponent, () => { - test('should show 404 page when the module code does not exist', () => { - const wrapper = make('CS1234', '/archive/CS1234/2017-2018'); - wrapper.setState({ error: { response: { status: 404 } } }); - expect(wrapper.type()).toEqual(ModuleNotFoundPage); + let mockAxiosRequest: jest.SpiedFunction; + + beforeEach(() => { + mockDom(); + mockAxiosRequest = jest.spyOn(axios, 'request'); + }); + + afterEach(() => { + mockAxiosRequest.mockRestore(); + mockDomReset(); + }); + + test('should show 404 page when the module code does not exist', async () => { + mockAxiosRequest.mockRejectedValue(notFoundError); + make('/archive/CS1234/2017-2018'); + expect(await screen.findByText(/module CS1234 not found/)).toBeInTheDocument(); }); - test('should redirect to canonical URL', () => { - assertRedirect( - make('CS1010S', '/archive/CS1010S', { module: CS1010S, archiveYear: '2017/2018' }), - '/archive/CS1010S/2017-2018/programming-methodology', + test('should redirect to canonical URL', async () => { + mockAxiosRequest.mockResolvedValue(cs1010sResponse); + const { history } = make('/archive/CS1010S/2017-2018'); + await waitFor(() => + expect(history.location.pathname).toBe('/archive/CS1010S/2017-2018/programming-methodology'), ); }); - test('should fetch module', () => { - const fetchModule = jest.fn().mockReturnValue(Promise.resolve()); - const component = make('CS1010S', CANONICAL, { fetchModule }); - expect(component.type()).toEqual(LoadingSpinner); - expect(fetchModule).toBeCalled(); + test('should fetch module', async () => { + mockAxiosRequest.mockResolvedValue(cs1010sResponse); + expect(mockAxiosRequest).not.toBeCalled(); // Sanity check + make(); + expect(screen.getByText(/Loading/i)).toBeInTheDocument(); + // Expect module information to be displayed + expect(await screen.findByText(/This module introduces/)).toBeInTheDocument(); + // Expect component to fetch + expect(mockAxiosRequest).toBeCalled(); }); test('should show error if module fetch failed', async () => { - const fetchModule = jest.fn().mockReturnValue(Promise.reject(new Error('Test error'))); - const component = make('CS1010S', CANONICAL, { fetchModule }); - await waitFor(() => { - component.update(); - return component.type() !== LoadingSpinner; - }); - - expect(component.type()).toEqual(ApiError); - expect(fetchModule).toBeCalled(); + mockAxiosRequest.mockRejectedValue(someOtherError); + make(); + expect(await screen.findByText(/can't load the module information/)).toBeInTheDocument(); }); }); diff --git a/website/src/views/modules/ModuleArchiveContainer.tsx b/website/src/views/modules/ModuleArchiveContainer.tsx index 306a2bc283..4722e8f436 100644 --- a/website/src/views/modules/ModuleArchiveContainer.tsx +++ b/website/src/views/modules/ModuleArchiveContainer.tsx @@ -1,45 +1,36 @@ -import * as React from 'react'; -import { AxiosError } from 'axios'; -import { connect, MapDispatchToPropsNonObject } from 'react-redux'; -import { match as Match, Redirect, RouteComponentProps, withRouter } from 'react-router-dom'; -import deferComponentRender from 'views/hocs/deferComponentRender'; +import { ComponentType, FC, useCallback, useEffect, useRef, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { match as Match, Redirect, useLocation, useRouteMatch } from 'react-router-dom'; import { get } from 'lodash'; -import type { Module, ModuleCode } from 'types/modules'; -import type { State as StoreState } from 'types/state'; +import type { AxiosError } from 'axios'; +import type { ModuleCode } from 'types/modules'; import type { Dispatch } from 'types/redux'; +import type { State } from 'types/state'; import { fetchModuleArchive } from 'actions/moduleBank'; -import { captureException, retryImport } from 'utils/error'; +import { captureException } from 'utils/error'; +import retryImport from 'utils/retryImport'; import ApiError from 'views/errors/ApiError'; import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; import LoadingSpinner from 'views/components/LoadingSpinner'; +import deferComponentRender from 'views/hocs/deferComponentRender'; import { moduleArchive } from 'views/routes/paths'; -import { Props as ModulePageContentProp } from './ModulePageContent'; +import type { Props as ModulePageContentProps } from './ModulePageContent'; type Params = { year: string; moduleCode: string; }; -type OwnProps = RouteComponentProps; - -type DispatchProps = { - fetchModule: () => Promise; -}; - -type Props = OwnProps & - DispatchProps & { - module: Module | null; - moduleCode: ModuleCode; - archiveYear: string; +function getPropsFromMatch(match: Match) { + const { year = '', moduleCode = '' } = match.params; + return { + moduleCode: moduleCode.toUpperCase(), + year: year.replace('-', '/'), }; - -type State = { - ModulePageContent: React.ComponentType | null; - error?: Error; -}; +} /** * Wrapper component for the archive page that handles data fetching and error handling. @@ -48,111 +39,85 @@ type State = { * error handling - the normal page tries to check the archives if this year's * API returns 404, while this page doesn't. */ -export class ModuleArchiveContainerComponent extends React.PureComponent { - state: State = { - ModulePageContent: null, - }; - - componentDidMount() { - this.fetchModule(); - this.fetchPageImport(); - } - - componentDidUpdate(prevProps: Props) { - if ( - prevProps.moduleCode !== this.props.moduleCode || - prevProps.archiveYear !== this.props.archiveYear - ) { - this.fetchModule(); +export const ModuleArchiveContainerComponent: FC = () => { + const [ + ModulePageContent, + setModulePageContent, + ] = useState | null>(null); + const [error, setError] = useState(); + + const dispatch = useDispatch(); + + const handleFetchError = useCallback((fetchError: AxiosError) => { + setError(fetchError); + captureException(fetchError); + }, []); + + useEffect( + () => { + // Try importing ModulePageContent thrice if we're online and + // getting the "Loading chunk x failed." error. + retryImport(() => import(/* webpackChunkName: "module" */ 'views/modules/ModulePageContent')) + .then((module) => setModulePageContent(() => module.default)) + .catch(handleFetchError); + }, + // Only load dynamic import on first mount. + // Assume `handleFetchError` will never change. + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); + + const match = useRouteMatch(); + const { moduleCode, year } = getPropsFromMatch(match); + + // Fetch module on mount and when params are updated + const fetchModuleForDisplay = useCallback(() => { + setError(undefined); + dispatch(fetchModuleArchive(moduleCode, year)).catch(handleFetchError); + }, [dispatch, handleFetchError, moduleCode, year]); + + const previousModuleCode = useRef(); + const previousArchiveYear = useRef(); + useEffect(() => { + if (previousModuleCode.current !== moduleCode || previousArchiveYear.current !== year) { + previousModuleCode.current = moduleCode; + previousArchiveYear.current = year; + fetchModuleForDisplay(); } - } + }, [fetchModuleForDisplay, moduleCode, year]); - fetchModule = () => { - this.setState({ error: undefined }); - this.props.fetchModule().catch(this.handleFetchError); - }; - - fetchPageImport() { - // Try importing ModulePageContent thrice if we're online and - // getting the "Loading chunk x failed." error. - retryImport(() => import(/* webpackChunkName: "module" */ 'views/modules/ModulePageContent')) - .then((module) => this.setState({ ModulePageContent: module.default })) - .catch(this.handleFetchError); - } - - handleFetchError = (error: AxiosError) => { - this.setState({ error }); - captureException(error); - }; + const location = useLocation(); - render() { - const { ModulePageContent, error } = this.state; - const { module, moduleCode, match, location, archiveYear } = this.props; + const module = useSelector(({ moduleBank }: State) => + get(moduleBank.moduleArchive, [moduleCode, year], null), + ); - if (get(error, ['response', 'status'], 200) === 404) { - return ; - } - - // If there is an error but module data can still be found, we assume module has - // been loaded at some point, so we just show that instead - if (error && !module) { - return ; - } + if (get(error, ['response', 'status'], 200) === 404) { + return ; + } - // Redirect to canonical URL - if (module) { - const canonicalUrl = moduleArchive(moduleCode, archiveYear, module.title); - if (match.url !== canonicalUrl) { - return ; - } - } + // If there is an error but module data can still be found, we assume module has + // been loaded at some point, so we just show that instead + if (error && !module) { + return ; + } - if (module && ModulePageContent) { - // Unique key forces component to remount whenever the user moves to - // a new module. This allows the internal state (eg. currently selected - // timetable semester) of to be consistent - return ( - - ); + // Redirect to canonical URL + if (module) { + const canonicalUrl = moduleArchive(moduleCode, year, module.title); + if (match.url !== canonicalUrl) { + return ; } - - return ; } -} -const getPropsFromMatch = (match: Match) => { - const { year = '', moduleCode = '' } = match.params; - return { - moduleCode: moduleCode.toUpperCase(), - year: year.replace('-', '/'), - }; -}; - -const mapStateToProps = ({ moduleBank }: StoreState, ownProps: OwnProps) => { - const { moduleCode, year } = getPropsFromMatch(ownProps.match); - return { - moduleCode, - archiveYear: year, - module: get(moduleBank.moduleArchive, [moduleCode, year], null), - }; -}; + if (module && ModulePageContent) { + // Unique key forces component to remount whenever the user moves to + // a new module. This allows the internal state (eg. currently selected + // timetable semester) of to be consistent + return ; + } -const mapDispatchToProps = (dispatch: Dispatch, ownProps: OwnProps) => { - const { moduleCode, year } = getPropsFromMatch(ownProps.match); - return { - fetchModule: () => dispatch(fetchModuleArchive(moduleCode, year)), - }; + return ; }; -const connectedModuleArchiveContainer = connect( - mapStateToProps, - // Cast required because the version of Dispatch defined by connect does not have the extensions defined - // in our Dispatch - mapDispatchToProps as MapDispatchToPropsNonObject, -)(ModuleArchiveContainerComponent); -const routedModuleArchiveContainer = withRouter(connectedModuleArchiveContainer); -export default deferComponentRender(routedModuleArchiveContainer); +export default deferComponentRender(ModuleArchiveContainerComponent); diff --git a/website/src/views/modules/ModuleFinderContainer/index.tsx b/website/src/views/modules/ModuleFinderContainer/index.tsx index f565abad8c..e2a9beea1d 100644 --- a/website/src/views/modules/ModuleFinderContainer/index.tsx +++ b/website/src/views/modules/ModuleFinderContainer/index.tsx @@ -2,7 +2,7 @@ import Loadable, { LoadingComponentProps } from 'react-loadable'; import LoadingSpinner from 'views/components/LoadingSpinner'; import ApiError from 'views/errors/ApiError'; -import { retryImport } from 'utils/error'; +import retryImport from 'utils/retryImport'; const AsyncModuleFinderContainer = Loadable({ loader: () => diff --git a/website/src/views/modules/ModulePageContainer.test.tsx b/website/src/views/modules/ModulePageContainer.test.tsx index d5587eca4e..a80d9d610d 100644 --- a/website/src/views/modules/ModulePageContainer.test.tsx +++ b/website/src/views/modules/ModulePageContainer.test.tsx @@ -1,71 +1,106 @@ -import { shallow, ShallowWrapper } from 'enzyme'; -import { Redirect } from 'react-router-dom'; - -import { Module, ModuleCode } from 'types/modules'; -import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; -import LoadingSpinner from 'views/components/LoadingSpinner'; -import createHistory from 'test-utils/createHistory'; -import { waitFor } from 'test-utils/async'; -import ApiError from 'views/errors/ApiError'; +import { screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import axios, { AxiosError, AxiosResponse } from 'axios'; + +import configureStore from 'bootstrapping/configure-store'; +import reducers from 'reducers'; +import { mockDom, mockDomReset } from 'test-utils/mockDom'; +import { initAction } from 'test-utils/redux'; +import renderWithRouterMatch from 'test-utils/renderWithRouterMatch'; import { CS1010S } from '__mocks__/modules'; + import { ModulePageContainerComponent } from './ModulePageContainer'; +jest.mock('views/components/RandomKawaii'); jest.mock('utils/error'); -const CANONICAL = '/modules/CS1010S/programming-methodology'; +const cs1010sResponse: AxiosResponse = { + data: CS1010S, + status: 200, + statusText: 'Ok', + headers: {}, + config: {}, +}; -type MakeContainerOptions = { - module: Module | null; - fetchModule: () => Promise; +const notFoundError: Partial = { + response: { + data: undefined, + status: 404, + statusText: 'Not found', + headers: {}, + config: {}, + }, }; -function make(moduleCode: ModuleCode, url: string, options: Partial = {}) { - const props: MakeContainerOptions = { - module: null, - fetchModule: () => Promise.resolve(), - ...options, - }; +const someOtherError: Partial = { + response: { + data: undefined, + status: 500, + statusText: 'Test error', + headers: {}, + config: {}, + }, +}; - return shallow( - , - ); -} +const CANONICAL = '/modules/CS1010S/programming-methodology'; -function assertRedirect(component: ShallowWrapper, redirectTo = CANONICAL) { - expect(component.type()).toEqual(Redirect); - expect(component.props()).toMatchObject({ to: { pathname: redirectTo } }); +const initialState = reducers(undefined, initAction()); + +function make(location: string = CANONICAL) { + const { store } = configureStore(initialState); + + return renderWithRouterMatch( + + + , + { + path: '/modules/:moduleCode/:slug?', + location, + }, + ); } describe(ModulePageContainerComponent, () => { - test('should show 404 page when the module code does not exist', () => { - const wrapper = make('CS1234', '/modules/CS1234'); - wrapper.setState({ error: { response: { status: 404 } } }); - expect(wrapper.type()).toEqual(ModuleNotFoundPage); + let mockAxiosRequest: jest.SpiedFunction; + + beforeEach(() => { + mockDom(); + mockAxiosRequest = jest.spyOn(axios, 'request'); + }); + + afterEach(() => { + mockAxiosRequest.mockRestore(); + mockDomReset(); + }); + + test('should show 404 page when the module code does not exist', async () => { + mockAxiosRequest.mockRejectedValue(notFoundError); + make('/modules/CS1234'); + expect(await screen.findByText(/module CS1234 not found/)).toBeInTheDocument(); }); - test('should redirect to canonical URL', () => { - assertRedirect( - make('CS1010S', '/modules/cs1010s/programming-methodology', { module: CS1010S }), + test('should redirect to canonical URL', async () => { + mockAxiosRequest.mockResolvedValue(cs1010sResponse); + const { history } = make('/modules/CS1010S'); + await waitFor(() => + expect(history.location.pathname).toBe('/modules/CS1010S/programming-methodology'), ); - assertRedirect(make('CS1010S', '/modules/CS1010S', { module: CS1010S })); }); - test('should fetch module', () => { - const fetchModule = jest.fn().mockReturnValue(Promise.resolve()); - const component = make('CS1010S', CANONICAL, { fetchModule }); - expect(component.type()).toEqual(LoadingSpinner); - expect(fetchModule).toBeCalled(); + test('should fetch module', async () => { + mockAxiosRequest.mockResolvedValue(cs1010sResponse); + expect(mockAxiosRequest).not.toBeCalled(); // Sanity check + make(); + expect(screen.getByText(/Loading/i)).toBeInTheDocument(); + // Expect module information to be displayed + expect(await screen.findByText(/This module introduces/)).toBeInTheDocument(); + // Expect component to fetch + expect(mockAxiosRequest).toBeCalled(); }); test('should show error if module fetch failed', async () => { - const fetchModule = jest.fn().mockReturnValue(Promise.reject(new Error('Test error'))); - const component = make('CS1010S', CANONICAL, { fetchModule }); - await waitFor(() => { - component.update(); - return component.type() !== LoadingSpinner; - }); - - expect(component.type()).toEqual(ApiError); - expect(fetchModule).toBeCalled(); + mockAxiosRequest.mockRejectedValue(someOtherError); + make(); + expect(await screen.findByText(/can't load the module information/)).toBeInTheDocument(); }); }); diff --git a/website/src/views/modules/ModulePageContainer.tsx b/website/src/views/modules/ModulePageContainer.tsx index 5ab3396b2c..ddb532f3ff 100644 --- a/website/src/views/modules/ModulePageContainer.tsx +++ b/website/src/views/modules/ModulePageContainer.tsx @@ -1,44 +1,31 @@ -import * as React from 'react'; -import { AxiosError } from 'axios'; -import { connect, MapDispatchToPropsNonObject } from 'react-redux'; -import { match as Match, Redirect, RouteComponentProps, withRouter } from 'react-router-dom'; -import deferComponentRender from 'views/hocs/deferComponentRender'; +import { ComponentType, FC, useCallback, useEffect, useRef, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { match as Match, Redirect, useLocation, useRouteMatch } from 'react-router-dom'; import { get } from 'lodash'; -import type { Module, ModuleCode } from 'types/modules'; +import type { AxiosError } from 'axios'; +import type { ModuleCode } from 'types/modules'; import type { Dispatch } from 'types/redux'; -import type { State as StoreState } from 'types/state'; +import type { State } from 'types/state'; import { fetchModule } from 'actions/moduleBank'; -import { captureException, retryImport } from 'utils/error'; +import { captureException } from 'utils/error'; +import retryImport from 'utils/retryImport'; import ApiError from 'views/errors/ApiError'; import ModuleNotFoundPage from 'views/errors/ModuleNotFoundPage'; import LoadingSpinner from 'views/components/LoadingSpinner'; +import deferComponentRender from 'views/hocs/deferComponentRender'; import { modulePage } from 'views/routes/paths'; -import { Props as ModulePageContentProp } from './ModulePageContent'; +import type { Props as ModulePageContentProps } from './ModulePageContent'; type Params = { moduleCode: string; }; -type OwnProps = RouteComponentProps; - -type DispatchProps = { - fetchModule: () => Promise; -}; - -type Props = OwnProps & - DispatchProps & { - module: Module | null; - moduleCode: ModuleCode; - fetchModule: () => Promise; - }; - -type State = { - ModulePageContent: React.ComponentType | null; - error?: Error; -}; +const getPropsFromMatch = (match: Match) => ({ + moduleCode: (match.params.moduleCode ?? '').toUpperCase(), +}); /** * Wrapper component that loads both module data and the module page component @@ -55,94 +42,79 @@ type State = { * - Loading: Either requests are pending * - Loaded: Both requests are successfully loaded */ -export class ModulePageContainerComponent extends React.PureComponent { - state: State = { - ModulePageContent: null, - }; - - componentDidMount() { - this.fetchModule(); - this.fetchPageImport(); - } - - componentDidUpdate(prevProps: Props) { - if (prevProps.moduleCode !== this.props.moduleCode) { - this.fetchModule(); +export const ModulePageContainerComponent: FC = () => { + const [ + ModulePageContent, + setModulePageContent, + ] = useState | null>(null); + const [error, setError] = useState(); + + const dispatch = useDispatch(); + + const handleFetchError = useCallback((fetchError: AxiosError) => { + setError(fetchError); + captureException(fetchError); + }, []); + + useEffect( + () => { + // Try importing ModulePageContent thrice if we're online and + // getting the "Loading chunk x failed." error. + retryImport(() => import(/* webpackChunkName: "module" */ 'views/modules/ModulePageContent')) + .then((module) => setModulePageContent(() => module.default)) + .catch(handleFetchError); + }, + // Only load dynamic import on first mount. + // Assume `handleFetchError` will never change. + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); + + const match = useRouteMatch(); + const { moduleCode } = getPropsFromMatch(match); + + // Fetch module on mount and when params are updated + const fetchModuleForDisplay = useCallback(() => { + setError(undefined); + dispatch(fetchModule(moduleCode)).catch(handleFetchError); + }, [dispatch, handleFetchError, moduleCode]); + + const previousModuleCode = useRef(); + useEffect(() => { + if (previousModuleCode.current !== moduleCode) { + previousModuleCode.current = moduleCode; + fetchModuleForDisplay(); } - } - - fetchModule = () => { - this.setState({ error: undefined }); - this.props.fetchModule().catch(this.handleFetchError); - }; - - fetchPageImport() { - // Try importing ModulePageContent thrice if we're online and - // getting the "Loading chunk x failed." error. - retryImport(() => import(/* webpackChunkName: "module" */ 'views/modules/ModulePageContent')) - .then((module) => this.setState({ ModulePageContent: module.default })) - .catch(this.handleFetchError); - } + }, [fetchModuleForDisplay, moduleCode]); - handleFetchError = (error: AxiosError) => { - this.setState({ error }); - captureException(error); - }; + const location = useLocation(); - render() { - const { ModulePageContent, error } = this.state; - const { module, moduleCode, match, location } = this.props; + const module = useSelector(({ moduleBank }: State) => moduleBank.modules[moduleCode]); - if (get(error, ['response', 'status'], 200) === 404) { - return ; - } - - // If there is an error but module data can still be found, we assume module has - // been loaded at some point, so we just show that instead - if (error && !module) { - return ; - } - - // Redirect to canonical URL - if (module && match.url !== modulePage(moduleCode, module.title)) { - return ; - } - - if (module && ModulePageContent) { - // Unique key forces component to remount whenever the user moves to - // a new module. This allows the internal state (eg. currently selected - // timetable semester) of to be consistent - return ; - } + if (get(error, ['response', 'status'], 200) === 404) { + return ; + } - return ; + // If there is an error but module data can still be found, we assume module has + // been loaded at some point, so we just show that instead + if (error && !module) { + return ; } -} -const getPropsFromMatch = (match: Match) => ({ - moduleCode: (match.params.moduleCode ?? '').toUpperCase(), -}); + // Redirect to canonical URL + if (module && match.url !== modulePage(moduleCode, module.title)) { + const canonicalUrl = modulePage(moduleCode, module.title); + return ; + } -const mapStateToProps = ({ moduleBank }: StoreState, ownProps: OwnProps) => { - const { moduleCode } = getPropsFromMatch(ownProps.match); - return { - moduleCode, - module: moduleBank.modules[moduleCode], - }; -}; + if (module && ModulePageContent) { + // Unique key forces component to remount whenever the user moves to + // a new module. This allows the internal state (eg. currently selected + // timetable semester) of to be consistent + return ; + } -const mapDispatchToProps = (dispatch: Dispatch, ownProps: OwnProps) => { - const { moduleCode } = getPropsFromMatch(ownProps.match); - return { - fetchModule: () => dispatch(fetchModule(moduleCode)), - }; + return ; }; -const connectedModulePageContainer = connect( - mapStateToProps, - // Cast required because the version of Dispatch defined by connect does not have the extensions defined - // in our Dispatch - mapDispatchToProps as MapDispatchToPropsNonObject, -)(ModulePageContainerComponent); -const routedModulePageContainer = withRouter(connectedModulePageContainer); -export default deferComponentRender(routedModulePageContainer); +export default deferComponentRender(ModulePageContainerComponent); diff --git a/website/src/views/timetable/ShareTimetable.tsx b/website/src/views/timetable/ShareTimetable.tsx index 65a6b943b6..99cd876beb 100644 --- a/website/src/views/timetable/ShareTimetable.tsx +++ b/website/src/views/timetable/ShareTimetable.tsx @@ -13,7 +13,7 @@ import { Copy, Mail, Repeat } from 'react-feather'; import Modal from 'views/components/Modal'; import CloseButton from 'views/components/CloseButton'; import LoadingSpinner from 'views/components/LoadingSpinner'; -import { retryImport } from 'utils/error'; +import retryImport from 'utils/retryImport'; import styles from './ShareTimetable.scss'; diff --git a/website/src/views/timetable/TimetableContainer.test.tsx b/website/src/views/timetable/TimetableContainer.test.tsx index e7c461a58f..5a1bc025c0 100644 --- a/website/src/views/timetable/TimetableContainer.test.tsx +++ b/website/src/views/timetable/TimetableContainer.test.tsx @@ -1,129 +1,172 @@ -import { Redirect } from 'react-router-dom'; -import { shallow, ShallowWrapper } from 'enzyme'; -import { SemTimetableConfig } from 'types/timetables'; -import { ModulesMap } from 'types/reducers'; +import { screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import axios, { AxiosResponse } from 'axios'; +import produce from 'immer'; + +import type { Semester } from 'types/modules'; + +import { FETCH_MODULE, FETCH_MODULE_LIST } from 'actions/constants'; +import { setTimetable } from 'actions/timetables'; +import configureStore from 'bootstrapping/configure-store'; +import config from 'config'; +import { SUCCESS_KEY } from 'middlewares/requests-middleware'; +import reducers from 'reducers'; +import { mockDom, mockDomReset } from 'test-utils/mockDom'; +import { initAction } from 'test-utils/redux'; +import renderWithRouterMatch from 'test-utils/renderWithRouterMatch'; + +import { timetablePage, timetableShare } from 'views/routes/paths'; + +import { BFS1001, CS1010S, CS3216 } from '__mocks__/modules'; +import modulesList from '__mocks__/moduleList.json'; + +import { TimetableContainerComponent } from './TimetableContainer'; + +/** + * A module that exists in our mock `moduleList` but which is also *not* + * pre-loaded into `moduleBank`. Intended to be used by tests that expect + * modules to be fetched. + */ +const moduleCodeThatCanBeLoaded = 'BFS1001'; + +const bfs1001Response: AxiosResponse = { + data: BFS1001, + status: 200, + statusText: 'Ok', + headers: {}, + config: {}, +}; + +const relevantStoreContents = { + app: { + activeSemester: 1, + }, +}; + +const initialState = reducers(undefined, initAction()); + +function make(location: string, storeOverrides: Partial = {}) { + const { store } = configureStore( + produce(initialState, (draft) => { + draft.app.activeSemester = + storeOverrides.app?.activeSemester ?? relevantStoreContents.app.activeSemester; + }), + ); + + // Populate moduleBank moduleList using "succeeded" requests-middleware requests + store.dispatch({ type: SUCCESS_KEY(FETCH_MODULE_LIST), payload: modulesList }); + + return { + store, + ...renderWithRouterMatch( + + + , + { + path: '/timetable/:semester?/:action?', + location, + }, + ), + }; +} -import createHistory from 'test-utils/createHistory'; +describe(TimetableContainerComponent, () => { + let mockAxiosRequest: jest.SpiedFunction; + + beforeEach(() => { + mockDom(); + mockAxiosRequest = jest.spyOn(axios, 'request'); + mockAxiosRequest.mockResolvedValue(bfs1001Response); + }); -import { semesterForTimetablePage, timetablePage, timetableShare } from 'views/routes/paths'; + afterEach(() => { + mockAxiosRequest.mockRestore(); + mockDomReset(); + }); -import { CS1010S, CS3216 } from '__mocks__/modules'; + test('should redirect to activeSemester when semester is empty', async () => { + const semesters: Semester[] = Object.keys(config.shortSemesterNames).map(Number); + expect(semesters.length).toBeGreaterThan(0); // Sanity check: `semesters` cannot be empty + + // Use for-of loop as we `waitFor` must be executed sequentially. + // eslint-disable-next-line no-restricted-syntax + for (const semester of semesters) { + const { history } = make('/timetable', { app: { activeSemester: semester } }); + // eslint-disable-next-line no-await-in-loop + await waitFor(() => expect(history.location.pathname).toBe(timetablePage(semester))); + } + }); -import { QueryParam, TimetableContainerComponent } from './TimetableContainer'; -import TimetableContent from './TimetableContent'; -import LoadingSpinner from '../components/LoadingSpinner'; + test('should redirect to homepage when the URL is invalid', async () => { + function expectRedirectToHomepageFrom(from: string) { + const homepage = timetablePage(relevantStoreContents.app.activeSemester); + const { history } = make(from); + return waitFor(() => expect(history.location.pathname).toBe(homepage)); + } + await expectRedirectToHomepageFrom('/timetable/hello'); + await expectRedirectToHomepageFrom('/timetable/sem-3'); + await expectRedirectToHomepageFrom('/timetable/sem-1/hello'); + await expectRedirectToHomepageFrom('/timetable/2017-2018'); + await expectRedirectToHomepageFrom('/timetable/2017-2018/sem2'); + await expectRedirectToHomepageFrom('/timetable/2017-2018/share'); + await expectRedirectToHomepageFrom('/timetable/2017-2018/v1'); + }); -describe(TimetableContainerComponent, () => { - function create( - path: string, - params: Partial = {}, - timetable: SemTimetableConfig = {}, - modules: ModulesMap = { CS1010S, CS3216 }, - ) { - const router = createHistory(path, { params }); - - const selectSemester = jest.fn(); - const setTimetable = jest.fn(); - const fetchTimetableModules = jest.fn(); - const openNotification = jest.fn(); - const undo = jest.fn(); - const isModuleValid = jest.fn().mockReturnValue(true); - - return { - selectSemester, - setTimetable, - fetchTimetableModules, - isModuleValid, - history: router.history, - - wrapper: shallow( - , - ), - }; - } - - function createWithImport( - importedTimetable: SemTimetableConfig, - modules: ModulesMap = {}, - existingTimetable: SemTimetableConfig = {}, - ) { + test('should eventually display imported timetable if there is one', async () => { const semester = 1; - const path = timetableShare(semester, importedTimetable); - return create(path, { semester: 'sem-1', action: 'share' }, existingTimetable, modules); - } + const importedTimetable = { [moduleCodeThatCanBeLoaded]: { Lecture: '1' } }; + const location = timetableShare(semester, importedTimetable); + make(location); - function expectRedirect(wrapper: ShallowWrapper, to = timetablePage(1)) { - expect(wrapper.type()).toEqual(Redirect); - expect(wrapper.prop('to')).toEqual(to); - } + // Expect spinner when loading modules + expect(screen.getByText(/Loading/)).toBeInTheDocument(); - test('should redirect to activeSemester when semester is empty', () => { - expectRedirect(create('/timetable').wrapper); - }); + // Expect import header to be present + expect(await screen.findByRole('button', { name: 'Import' })).toBeInTheDocument(); - test('should redirect to homepage when the URL is invalid', () => { - expectRedirect(create('/timetable/hello', { semester: 'hello' }).wrapper); - expectRedirect(create('/timetable/sem-3', { semester: 'sem-3' }).wrapper); - expectRedirect(create('/timetable/sem-1/hello', { semester: '1', action: 'hello' }).wrapper); - expectRedirect(create('/timetable/2017-2018', { semester: '2017-2018' }).wrapper); - expectRedirect( - create('/timetable/2017-2018/sem2', { semester: '2017-2018', action: 'sem2' }).wrapper, - ); - expectRedirect( - create('/timetable/2017-2018/share', { semester: '2017-2018', action: 'share' }).wrapper, - ); - expectRedirect( - create('/timetable/2017-2018/v1', { semester: '2017-2018', action: 'v1' }).wrapper, - ); - }); + // Expect imported module info to be displayed + expect(screen.getByText(/Personal Development & Career Management/)).toBeInTheDocument(); - test('should load modules from imported timetable', () => { - const timetable = { CS2105: { Lecture: '1' } }; - const container = createWithImport(timetable); - expect(container.fetchTimetableModules).toBeCalledWith([timetable]); + // Expect correct network calls to be made + expect(mockAxiosRequest).toHaveBeenCalledTimes(1); }); - test('should display spinner when loading modules', () => { - const timetable = { CS2105: { Lecture: '1' } }; - const { wrapper } = createWithImport(timetable); + test('should ignore invalid modules in imported timetable', () => { + const semester = 1; + const importedTimetable = { TRUMP2020: { Lecture: '1' } }; + const location = timetableShare(semester, importedTimetable); + make(location); - expect(wrapper.type()).toEqual(LoadingSpinner); - }); + // Expect nothing to be fetched and the invalid module to be ignored + expect(screen.queryByText(/Loading/)).not.toBeInTheDocument(); + expect(mockAxiosRequest).not.toHaveBeenCalledTimes(1); + expect(screen.queryByText(/TRUMP2020/)).not.toBeInTheDocument(); - test('should display imported timetable', () => { - const timetable = { CS2105: { Lecture: '1' } }; - const { wrapper } = createWithImport(timetable, { CS2105: CS3216 }); - const timetableContentWrapper = wrapper.children(TimetableContent); - expect(timetableContentWrapper.type()).toEqual(TimetableContent); - expect(timetableContentWrapper.props()).toMatchObject({ - timetable, - readOnly: true, - }); + // Expect import header to still be present + expect(screen.getByRole('button', { name: 'Import' })).toBeInTheDocument(); }); test('should display saved timetable when there is no imported timetable', () => { const semester = 1; - const timetable = { CS1010S: { Lecture: '1' } }; - - const { wrapper } = create(timetablePage(semester), { semester: 'sem-1' }, timetable); - const timetableContentWrapper = wrapper.children(TimetableContent); - expect(timetableContentWrapper.type()).toEqual(TimetableContent); - expect(timetableContentWrapper.props()).toMatchObject({ - timetable, - readOnly: false, - }); + const location = timetablePage(semester); + const { store } = make(location); + + // Populate moduleBank using "succeeded" requests-middleware requests + store.dispatch({ type: SUCCESS_KEY(FETCH_MODULE), payload: CS1010S }); + store.dispatch({ type: SUCCESS_KEY(FETCH_MODULE), payload: CS3216 }); + + // Populate mock timetable + const timetable = { CS1010S: { Lecture: '1' }, CS3216: { Lecture: '1' } }; + store.dispatch(setTimetable(semester, timetable)); + + // Expect nothing to be fetched as timetable exists in `moduleBank`. + expect(screen.queryByText(/Loading/)).not.toBeInTheDocument(); + expect(mockAxiosRequest).not.toHaveBeenCalled(); + + // Expect imported module info to be displayed + expect(screen.getByText(/Programming Methodology/)).toBeInTheDocument(); + + // Expect import header not to be present + expect(screen.queryByRole('button', { name: 'Import' })).not.toBeInTheDocument(); }); }); diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index 14c8a89d65..cfc36b78a0 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -1,14 +1,16 @@ -import { PureComponent } from 'react'; -import { connect } from 'react-redux'; -import { Redirect, RouteComponentProps, withRouter } from 'react-router-dom'; +import { FC, useCallback, useEffect, useMemo, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { Redirect, useHistory, useLocation, useParams } from 'react-router-dom'; +import { Repeat } from 'react-feather'; import classnames from 'classnames'; -import { ModuleCode, Semester } from 'types/modules'; -import { SemTimetableConfig } from 'types/timetables'; -import { ColorMapping, ModulesMap, NotificationOptions } from 'types/reducers'; +import type { ModuleCode, Semester } from 'types/modules'; +import type { ColorMapping } from 'types/reducers'; +import type { State } from 'types/state'; +import type { SemTimetableConfig } from 'types/timetables'; import { selectSemester } from 'actions/settings'; -import { getSemesterTimetable } from 'selectors/timetables'; +import { getSemesterTimetableColors, getSemesterTimetableLessons } from 'selectors/timetables'; import { fetchTimetableModules, setTimetable } from 'actions/timetables'; import { openNotification } from 'actions/app'; import { undo } from 'actions/undoHistory'; @@ -17,43 +19,114 @@ import { deserializeTimetable } from 'utils/timetables'; import { fillColorMapping } from 'utils/colors'; import { semesterForTimetablePage, TIMETABLE_SHARE, timetablePage } from 'views/routes/paths'; import deferComponentRender from 'views/hocs/deferComponentRender'; -import { Repeat } from 'react-feather'; import SemesterSwitcher from 'views/components/semester-switcher/SemesterSwitcher'; import LoadingSpinner from 'views/components/LoadingSpinner'; -import ScrollToTop from 'views/components/ScrollToTop'; -import { State as StoreState } from 'types/state'; +import useScrollToTop from 'views/hooks/useScrollToTop'; import TimetableContent from './TimetableContent'; import styles from './TimetableContainer.scss'; -export type QueryParam = { +type Params = { action: string; semester: string; }; -type OwnProps = RouteComponentProps; - -type Props = OwnProps & { - modules: ModulesMap; - semester: Semester | null; - activeSemester: Semester; - timetable: SemTimetableConfig; - colors: ColorMapping; - - isValidModule: (moduleCode: ModuleCode) => boolean; - selectSemester: (semester: Semester) => void; - setTimetable: ( - semester: Semester, - semTimetableConfig: SemTimetableConfig, - colorMapping: ColorMapping, - ) => void; - fetchTimetableModules: (semTimetableConfig: SemTimetableConfig[]) => void; - openNotification: (str: string, notificationOptions: NotificationOptions) => void; - undo: () => void; +/* + * If there is an imported timetable, show a sharing header which asks the user + * if they want to import the shared timetable. + */ +const SharingHeader: FC<{ + semester: Semester; + filledColors: ColorMapping; + importedTimetable: SemTimetableConfig | null; + setImportedTimetable: (timetable: SemTimetableConfig | null) => void; +}> = ({ semester, filledColors, importedTimetable, setImportedTimetable }) => { + const history = useHistory(); + const dispatch = useDispatch(); + + const clearImportedTimetable = useCallback(() => { + if (semester) { + setImportedTimetable(null); + history.push(timetablePage(semester)); // TODO: Check that this works + } + }, [history, semester, setImportedTimetable]); + + const importTimetable = useCallback(() => { + if (!importedTimetable) { + return; + } + dispatch(setTimetable(semester, importedTimetable, filledColors)); + clearImportedTimetable(); + dispatch( + openNotification('Timetable imported', { + timeout: 12000, + overwritable: true, + action: { + text: 'Undo', + handler: () => dispatch(undo) as never, + }, + }), + ); + }, [clearImportedTimetable, dispatch, filledColors, importedTimetable, semester]); + + if (!importedTimetable) { + return null; + } + + return ( +
+ + +
+
+

This timetable was shared with you

+

+ Clicking import will replace your saved timetable with the one below. +

+
+ +
+ + +
+
+
+ ); }; -type State = { - importedTimetable: SemTimetableConfig | null; +const TimetableHeader: FC<{ + semester: Semester; + readOnly?: boolean; +}> = ({ semester, readOnly }) => { + const history = useHistory(); + const dispatch = useDispatch(); + + const handleSelectSemester = useCallback( + (newSemester: Semester) => { + dispatch(selectSemester(newSemester)); + history.push({ + ...history.location, + pathname: timetablePage(newSemester), + }); + }, + [dispatch, history], + ); + + return ( + + ); }; /** @@ -62,191 +135,82 @@ type State = { * - Import timetable data from query string if action is defined * - Create the UI for the user to confirm their actions */ -export class TimetableContainerComponent extends PureComponent { - constructor(props: Props) { - super(props); - - const { semester, match, location } = props; - const importedTimetable = - semester && match.params.action ? deserializeTimetable(location.search) : null; +export const TimetableContainerComponent: FC = () => { + const params = useParams(); - this.state = { - importedTimetable, - }; - } + const semester = semesterForTimetablePage(params.semester); - componentDidMount() { - if (this.state.importedTimetable) { - this.props.fetchTimetableModules([this.state.importedTimetable]); - } - } + const timetable = useSelector(getSemesterTimetableLessons)(semester); + const colors = useSelector(getSemesterTimetableColors)(semester); + const getModule = useSelector(getModuleCondensed); + const modules = useSelector(({ moduleBank }: State) => moduleBank.modules); + const activeSemester = useSelector(({ app }: State) => app.activeSemester); - selectSemester = (semester: Semester) => { - this.props.selectSemester(semester); + const location = useLocation(); + const [importedTimetable, setImportedTimetable] = useState(() => + semester && params.action ? deserializeTimetable(location.search) : null, + ); - this.props.history.push({ - ...this.props.history.location, - pathname: timetablePage(semester), - }); - }; + const dispatch = useDispatch(); + useEffect(() => { + if (importedTimetable) { + dispatch(fetchTimetableModules([importedTimetable])); + } + }, [dispatch, importedTimetable]); - isLoading() { + const isLoading = useMemo(() => { // Check that all modules are fully loaded into the ModuleBank - const { modules, timetable } = this.props; - const { importedTimetable } = this.state; - + const isValidModule = (moduleCode: ModuleCode) => !!getModule(moduleCode); const moduleCodes = new Set(Object.keys(timetable)); if (importedTimetable) { Object.keys(importedTimetable) - .filter(this.props.isValidModule) + .filter(isValidModule) .forEach((moduleCode) => moduleCodes.add(moduleCode)); } - // TODO: Account for loading error return Array.from(moduleCodes).some((moduleCode) => !modules[moduleCode]); - } + }, [getModule, importedTimetable, modules, timetable]); - importTimetable(semester: Semester, timetable: SemTimetableConfig) { - const colors = fillColorMapping(timetable, this.props.colors); - this.props.setTimetable(semester, timetable, colors); - this.clearImportedTimetable(); - - this.props.openNotification('Timetable imported', { - timeout: 12000, - overwritable: true, - action: { - text: 'Undo', - handler: this.props.undo, - }, - }); - } + const displayedTimetable = importedTimetable || timetable; + const filledColors = useMemo(() => fillColorMapping(displayedTimetable, colors), [ + colors, + displayedTimetable, + ]); + const readOnly = displayedTimetable === importedTimetable; - clearImportedTimetable = () => { - const { semester } = this.props; - if (semester) { - this.setState({ importedTimetable: null }, () => - this.props.history.push(timetablePage(semester)), - ); - } - }; - - sharingHeader(semester: Semester, timetable: SemTimetableConfig) { - return ( -
- - -
-
-

This timetable was shared with you

-

- Clicking import will replace your saved timetable with the one below. -

-
- -
- - -
-
-
- ); - } + useScrollToTop(); - timetableHeader(semester: Semester, readOnly?: boolean) { - return ( - - ); + // 1. If the URL doesn't look correct, we'll direct the user to the home page + if (semester == null || (params.action && params.action !== TIMETABLE_SHARE)) { + return ; } - render() { - const { timetable, semester, activeSemester, match } = this.props; - const { importedTimetable } = this.state; - const { action } = match.params; - - // 1. If the URL doesn't look correct, we'll direct the user to the home page - if (semester == null || (action && action !== TIMETABLE_SHARE)) { - return ; - } - - // 2. If we are importing a timetable, check that all imported modules are - // loaded first, and display a spinner if they're not. - if (this.isLoading()) { - return ; - } - - // 3. Construct the color map - const displayedTimetable = importedTimetable || timetable; - const colors = fillColorMapping(displayedTimetable, this.props.colors); - - // 4. If there is an imported timetable, we show the sharing header which - // asks the user if they want to import the shared timetable - const header = importedTimetable ? ( - <> - {this.sharingHeader(semester, importedTimetable)} - {this.timetableHeader(semester, true)} - - ) : ( - this.timetableHeader(semester) - ); - - return ( -
- - - -
- ); + // 2. If we are importing a timetable, check that all imported modules are + // loaded first, and display a spinner if they're not. + if (isLoading) { + return ; } -} - -const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { - const semester = semesterForTimetablePage(ownProps.match.params.semester); - const { timetable, colors } = semester - ? getSemesterTimetable(semester, state.timetables) - : { timetable: {}, colors: {} }; - const getModule = getModuleCondensed(state.moduleBank); - - return { - semester, - timetable, - colors, - isValidModule: (moduleCode: ModuleCode) => !!getModule(moduleCode), - modules: state.moduleBank.modules, - activeSemester: state.app.activeSemester, - }; + + return ( + + + + + } + readOnly={readOnly} + /> + ); }; -// Explicitly declare top level components for React hot reloading to work. -const connectedTimetableContainer = connect(mapStateToProps, { - selectSemester, - setTimetable, - fetchTimetableModules, - openNotification, - undo, -})(TimetableContainerComponent); - -const routedTimetableContainer = withRouter(connectedTimetableContainer); -export default deferComponentRender(routedTimetableContainer); +export default deferComponentRender(TimetableContainerComponent); diff --git a/website/src/views/today/TodayContainer/TodayContainer.tsx b/website/src/views/today/TodayContainer/TodayContainer.tsx index 6623190de2..c9b44a2ea0 100644 --- a/website/src/views/today/TodayContainer/TodayContainer.tsx +++ b/website/src/views/today/TodayContainer/TodayContainer.tsx @@ -28,7 +28,7 @@ import { } from 'utils/timetables'; import { captureException } from 'utils/error'; import Title from 'views/components/Title'; -import { getSemesterTimetable } from 'selectors/timetables'; +import { getSemesterTimetableColors, getSemesterTimetableLessons } from 'selectors/timetables'; import ExternalLink from 'views/components/ExternalLink'; import * as weatherAPI from 'apis/weather'; import config from 'config'; @@ -349,7 +349,8 @@ export const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { const lastDay = addDays(ownProps.currentTime, DAYS); const weekInfo = NUSModerator.academicCalendar.getAcadWeekInfo(lastDay); const semester = semesterNameMap[weekInfo.sem]; - const { timetable, colors } = getSemesterTimetable(semester, state.timetables); + const timetable = getSemesterTimetableLessons(state)(semester); + const colors = getSemesterTimetableColors(state)(semester); const timetableWithLessons = hydrateSemTimetableWithLessons(timetable, modules, semester); return { diff --git a/website/src/views/today/TodayContainer/index.tsx b/website/src/views/today/TodayContainer/index.tsx index 904e105c06..f976d7b3ac 100644 --- a/website/src/views/today/TodayContainer/index.tsx +++ b/website/src/views/today/TodayContainer/index.tsx @@ -2,7 +2,7 @@ import Loadable, { LoadingComponentProps } from 'react-loadable'; import LoadingSpinner from 'views/components/LoadingSpinner'; import ApiError from 'views/errors/ApiError'; -import { retryImport } from 'utils/error'; +import retryImport from 'utils/retryImport'; import EventMapInline from '../EventMapInline'; import EventMap from '../EventMap'; diff --git a/website/src/views/venues/VenueDetails.tsx b/website/src/views/venues/VenueDetails.tsx index 52a41f78b5..3d7a93d01c 100644 --- a/website/src/views/venues/VenueDetails.tsx +++ b/website/src/views/venues/VenueDetails.tsx @@ -1,49 +1,56 @@ -import * as React from 'react'; -import { Link, RouteComponentProps, withRouter } from 'react-router-dom'; +import { FC, memo, useCallback, useMemo } from 'react'; +import { Link, useHistory } from 'react-router-dom'; +import { ChevronLeft, ChevronRight } from 'react-feather'; import classnames from 'classnames'; import { flatMap } from 'lodash'; -import { DayAvailability, TimePeriod, Venue } from 'types/venues'; -import { Lesson } from 'types/timetables'; +import type { DayAvailability, TimePeriod, Venue } from 'types/venues'; +import type { Lesson } from 'types/timetables'; import { colorLessonsByKey } from 'utils/colors'; import { arrangeLessonsForWeek } from 'utils/timetables'; -import { ChevronLeft, ChevronRight } from 'react-feather'; -import Timetable from 'views/timetable/Timetable'; -import makeResponsive from 'views/hocs/makeResponsive'; -import { modulePage, venuePage } from 'views/routes/paths'; import Title from 'views/components/Title'; +import useMediaQuery from 'views/hooks/useMediaQuery'; +import { modulePage, venuePage } from 'views/routes/paths'; +import Timetable from 'views/timetable/Timetable'; import { breakpointDown } from 'utils/css'; import VenueLocation from './VenueLocation'; import styles from './VenueDetails.scss'; -type Props = RouteComponentProps & { - readonly venue: Venue; - readonly previous?: Venue | null; - readonly next?: Venue | null; - readonly availability: DayAvailability[]; - readonly highlightPeriod?: TimePeriod; - - readonly matchBreakpoint: boolean; +type Props = { + venue: Venue; + previous?: Venue | null; + next?: Venue | null; + availability: DayAvailability[]; + highlightPeriod?: TimePeriod; }; -export const VenueDetailsComponent: React.FC = (props) => { - const arrangedLessons = () => { - const lessons: Lesson[] = flatMap(props.availability, (day) => day.classes).map( - (venueLesson) => ({ - ...venueLesson, - title: '', - isModifiable: true, - venue: '', - }), - ); - +const VenueDetailsComponent: FC = ({ + venue, + previous, + next, + availability, + highlightPeriod, +}) => { + const arrangedLessons = useMemo(() => { + const lessons: Lesson[] = flatMap(availability, (day) => day.classes).map((venueLesson) => ({ + ...venueLesson, + title: '', + isModifiable: true, + venue: '', + })); const coloredLessons = colorLessonsByKey(lessons, 'moduleCode'); return arrangeLessonsForWeek(coloredLessons); - }; + }, [availability]); - const { venue, previous, next, matchBreakpoint, history } = props; + const history = useHistory(); + const navigateToLesson = useCallback( + (lesson: Lesson) => history.push(modulePage(lesson.moduleCode, lesson.title)), + [history], + ); + + const narrowViewport = useMediaQuery(breakpointDown('lg')); return ( <> @@ -79,22 +86,16 @@ export const VenueDetailsComponent: React.FC = (props) => {
-
+
{ - history.push(modulePage(lesson.moduleCode, lesson.title)); - }} + lessons={arrangedLessons} + highlightPeriod={highlightPeriod} + isVerticalOrientation={narrowViewport} + onModifyCell={navigateToLesson} />
); }; -const ResponsiveVenueDetails = makeResponsive( - React.memo(VenueDetailsComponent), - breakpointDown('lg'), -); -export default withRouter(ResponsiveVenueDetails); +export default memo(VenueDetailsComponent); diff --git a/website/src/views/venues/VenueLocation/index.tsx b/website/src/views/venues/VenueLocation/index.tsx index e76ec059a0..2b1aa1cc30 100644 --- a/website/src/views/venues/VenueLocation/index.tsx +++ b/website/src/views/venues/VenueLocation/index.tsx @@ -1,4 +1,4 @@ -import { retryImport } from 'utils/error'; +import retryImport from 'utils/retryImport'; import withVenueLocations from 'views/components/map/withVenueLocations'; import { Props } from './VenueLocation'; diff --git a/website/yarn.lock b/website/yarn.lock index 1e05992338..126ec632aa 100644 --- a/website/yarn.lock +++ b/website/yarn.lock @@ -2709,7 +2709,12 @@ dependencies: "@types/node" "*" -"@types/history@*", "@types/history@^4.6.0": +"@types/history@*": + version "4.7.8" + resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" + integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== + +"@types/history@^4.6.0": version "4.7.2" resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.2.tgz#0e670ea254d559241b6eeb3894f8754991e73220" integrity sha512-ui3WwXmjTaY73fOQ3/m3nnajU/Orhi6cEu5rzX+BrAAJxa3eITXZ5ch9suPqtM03OWhAHhPSyBGCN4UKoxO20Q== @@ -2934,9 +2939,9 @@ "@types/react-router" "*" "@types/react-router@*": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.0.3.tgz#855a1606e62de3f4d69ea34fb3c0e50e98e964d5" - integrity sha512-j2Gge5cvxca+5lK9wxovmGPgpVJMwjyu5lTA/Cd6fLGoPq7FXcUE1jFkEdxeyqGGz8VfHYSHCn5Lcn24BzaNKA== + version "5.1.8" + resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.8.tgz#4614e5ba7559657438e17766bb95ef6ed6acc3fa" + integrity sha512-HzOyJb+wFmyEhyfp4D4NYrumi+LQgQL/68HvJO+q6XtuHSDvw6Aqov7sCAhjbNq3bUPgPqbdvjXC5HeB2oEAPg== dependencies: "@types/history" "*" "@types/react" "*" @@ -2948,15 +2953,7 @@ dependencies: "@types/react" "*" -"@types/react@*": - version "16.9.34" - resolved "https://registry.yarnpkg.com/@types/react/-/react-16.9.34.tgz#f7d5e331c468f53affed17a8a4d488cd44ea9349" - integrity sha512-8AJlYMOfPe1KGLKyHpflCg5z46n0b5DbRfqDksxBLBTUpB75ypDBAO9eCUcjNwE6LCUslwTz00yyG/X9gaVtow== - dependencies: - "@types/prop-types" "*" - csstype "^2.2.0" - -"@types/react@17.0.0": +"@types/react@*", "@types/react@17.0.0": version "17.0.0" resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.0.tgz#5af3eb7fad2807092f0046a1302b7823e27919b8" integrity sha512-aj/L7RIMsRlWML3YB6KZiXB3fV2t41+5RBGYF8z+tAKU43Px8C3cYUZsDvf1/+Bm4FK21QWBrDutu8ZJ/70qOw== @@ -5437,11 +5434,6 @@ cssstyle@^2.2.0: dependencies: cssom "~0.3.6" -csstype@^2.2.0: - version "2.6.10" - resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.10.tgz#e63af50e66d7c266edb6b32909cfd0aabe03928b" - integrity sha512-D34BqZU4cIlMCY93rZHbrq9pjTAQJ3U8S8rfBqjwHxkGPThWFjzZDQpgMJY0QViLxth6ZKYiwFBo14RdN44U/w== - csstype@^3.0.2: version "3.0.5" resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.0.5.tgz#7fdec6a28a67ae18647c51668a9ff95bb2fa7bb8"