From 66eafd0ae5fdaf49d1f6e2b547d1718e21c51509 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Tue, 25 Aug 2026 22:04:44 +0800 Subject: [PATCH 1/4] style(lesson-plan): rename javascript -> typescript converted files --- .../app/bundles/course/lesson-plan/{constants.js => constants.ts} | 0 .../containers/ColumnVisibilityDropdown/{index.jsx => index.tsx} | 0 .../containers/EventFormDialog/{index.jsx => index.tsx} | 0 .../containers/LessonPlanFilter/{index.jsx => index.tsx} | 0 .../{EnterEditModeButton.jsx => EnterEditModeButton.tsx} | 0 .../LessonPlanLayout/{NewEventButton.jsx => NewEventButton.tsx} | 0 .../{NewMilestoneButton.jsx => NewMilestoneButton.tsx} | 0 .../__test__/{NewEventButton.test.jsx => NewEventButton.test.tsx} | 0 .../{NewMilestoneButton.test.jsx => NewMilestoneButton.test.tsx} | 0 .../LessonPlanLayout/__test__/{index.test.jsx => index.test.tsx} | 0 .../containers/LessonPlanLayout/{index.jsx => index.tsx} | 0 .../containers/MilestoneFormDialog/{index.jsx => index.tsx} | 0 .../pages/LessonPlanEdit/ItemRow/{DateCell.jsx => DateCell.tsx} | 0 .../ItemRow/{PublishedCell.jsx => PublishedCell.tsx} | 0 .../pages/LessonPlanEdit/ItemRow/{index.jsx => index.tsx} | 0 .../pages/LessonPlanEdit/{MilestoneRow.jsx => MilestoneRow.tsx} | 0 .../__test__/{ItemRow.test.jsx => ItemRow.test.tsx} | 0 .../__test__/{MilestoneRow.test.jsx => MilestoneRow.test.tsx} | 0 .../LessonPlanEdit/__test__/{index.test.jsx => index.test.tsx} | 0 .../lesson-plan/pages/LessonPlanEdit/{index.jsx => index.tsx} | 0 .../LessonPlanItem/{AdminTools.jsx => AdminTools.tsx} | 0 .../__test__/{AdminTools.test.jsx => AdminTools.test.tsx} | 0 .../{MilestoneAdminTools.jsx => MilestoneAdminTools.tsx} | 0 ...{MilestoneAdminTools.test.jsx => MilestoneAdminTools.test.tsx} | 0 .../bundles/course/lesson-plan/reducers/{flags.js => flags.ts} | 0 .../course/lesson-plan/reducers/{lessonPlan.js => lessonPlan.ts} | 0 .../bundles/course/lesson-plan/reducers/{utils.js => utils.ts} | 0 27 files changed, 0 insertions(+), 0 deletions(-) rename client/app/bundles/course/lesson-plan/{constants.js => constants.ts} (100%) rename client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/EventFormDialog/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/{EnterEditModeButton.jsx => EnterEditModeButton.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/{NewEventButton.jsx => NewEventButton.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/{NewMilestoneButton.jsx => NewMilestoneButton.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/{NewEventButton.test.jsx => NewEventButton.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/{NewMilestoneButton.test.jsx => NewMilestoneButton.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/{index.test.jsx => index.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/{DateCell.jsx => DateCell.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/{PublishedCell.jsx => PublishedCell.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/{MilestoneRow.jsx => MilestoneRow.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/{ItemRow.test.jsx => ItemRow.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/{MilestoneRow.test.jsx => MilestoneRow.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/{index.test.jsx => index.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/{index.jsx => index.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/{AdminTools.jsx => AdminTools.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/{AdminTools.test.jsx => AdminTools.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanShow/{MilestoneAdminTools.jsx => MilestoneAdminTools.tsx} (100%) rename client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/{MilestoneAdminTools.test.jsx => MilestoneAdminTools.test.tsx} (100%) rename client/app/bundles/course/lesson-plan/reducers/{flags.js => flags.ts} (100%) rename client/app/bundles/course/lesson-plan/reducers/{lessonPlan.js => lessonPlan.ts} (100%) rename client/app/bundles/course/lesson-plan/reducers/{utils.js => utils.ts} (100%) diff --git a/client/app/bundles/course/lesson-plan/constants.js b/client/app/bundles/course/lesson-plan/constants.ts similarity index 100% rename from client/app/bundles/course/lesson-plan/constants.js rename to client/app/bundles/course/lesson-plan/constants.ts diff --git a/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.jsx b/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.jsx rename to client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.jsx b/client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.jsx rename to client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/index.test.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/index.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/index.test.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/index.test.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.jsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.jsx rename to client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.tsx diff --git a/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.jsx b/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.jsx rename to client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/AdminTools.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/AdminTools.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/AdminTools.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/AdminTools.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.tsx diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.jsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.tsx similarity index 100% rename from client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.jsx rename to client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.tsx diff --git a/client/app/bundles/course/lesson-plan/reducers/flags.js b/client/app/bundles/course/lesson-plan/reducers/flags.ts similarity index 100% rename from client/app/bundles/course/lesson-plan/reducers/flags.js rename to client/app/bundles/course/lesson-plan/reducers/flags.ts diff --git a/client/app/bundles/course/lesson-plan/reducers/lessonPlan.js b/client/app/bundles/course/lesson-plan/reducers/lessonPlan.ts similarity index 100% rename from client/app/bundles/course/lesson-plan/reducers/lessonPlan.js rename to client/app/bundles/course/lesson-plan/reducers/lessonPlan.ts diff --git a/client/app/bundles/course/lesson-plan/reducers/utils.js b/client/app/bundles/course/lesson-plan/reducers/utils.ts similarity index 100% rename from client/app/bundles/course/lesson-plan/reducers/utils.js rename to client/app/bundles/course/lesson-plan/reducers/utils.ts From 80df4d9c24f50fe935f9b092d29578f90dbb3f9a Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Wed, 26 Aug 2026 10:36:25 +0800 Subject: [PATCH 2/4] fix(lesson-plan): add debouncing for LessonPlanEdit page - convert LessonPlanEdit and subpages to typescript - add debouncing / request batching logic to prevent duplicate requests / personalized timeline updates --- .../bundles/course/lesson-plan/operations.ts | 44 +-- .../pages/LessonPlanEdit/ItemRow/DateCell.tsx | 97 +++---- .../LessonPlanEdit/ItemRow/PublishedCell.tsx | 19 +- .../pages/LessonPlanEdit/ItemRow/index.tsx | 256 +++++++++++------- .../pages/LessonPlanEdit/MilestoneRow.tsx | 184 +++++++++---- .../LessonPlanEdit/__test__/ItemRow.test.tsx | 196 +++++++++++--- .../__test__/MilestoneRow.test.tsx | 109 +++++++- .../LessonPlanEdit/__test__/index.test.tsx | 50 ++-- .../pages/LessonPlanEdit/index.tsx | 133 +++++---- .../app/bundles/course/lesson-plan/types.ts | 146 ++++++++++ 10 files changed, 850 insertions(+), 384 deletions(-) create mode 100644 client/app/bundles/course/lesson-plan/types.ts diff --git a/client/app/bundles/course/lesson-plan/operations.ts b/client/app/bundles/course/lesson-plan/operations.ts index 7c33ec50908..dfa68ff4587 100644 --- a/client/app/bundles/course/lesson-plan/operations.ts +++ b/client/app/bundles/course/lesson-plan/operations.ts @@ -55,29 +55,21 @@ export function createMilestone( }; } -export function updateMilestone( - id, - values, - successMessage, - failureMessage, - setError, -): Operation { +/** + * Reports whether the save succeeded and leaves the notification to the caller: + * a row that has already queued a newer edit suppresses the message so the user + * gets one verdict, from the last edit, rather than two. + */ +export function updateMilestone(id, values, setError): Operation { return async (dispatch) => { dispatch({ type: actionTypes.MILESTONE_UPDATE_REQUEST }); return CourseAPI.lessonPlan .updateMilestone(id, { lesson_plan_milestone: values }) .then((response) => { - dispatch({ - type: actionTypes.MILESTONE_UPDATE_SUCCESS, - milestoneId: id, - milestone: response.data, - }); - dispatch(actions.hideMilestoneForm()); - setNotification(successMessage)(dispatch); + dispatch(actions.milestoneUpdated(response.data)); + return true; }) .catch((error) => { - dispatch({ type: actionTypes.MILESTONE_UPDATE_FAILURE }); - setNotification(failureMessage)(dispatch); if (error?.response?.data?.errors && setError) { setReactHookFormError(setError, error.response.data.errors); } @@ -104,27 +96,17 @@ export function deleteMilestone(id, successMessage, failureMessage): Operation { }; } -export function updateItem( - id, - values, - successMessage, - failureMessage, -): Operation { +/** See `updateMilestone` for why the notification is the caller's. */ +export function updateItem(id, values): Operation { return async (dispatch) => { dispatch({ type: actionTypes.ITEM_UPDATE_REQUEST }); return CourseAPI.lessonPlan .updateItem(id, { item: values }) .then(() => { - dispatch({ - type: actionTypes.ITEM_UPDATE_SUCCESS, - item: { id, ...values }, - }); - setNotification(successMessage)(dispatch); + dispatch(actions.itemUpdated({ id, ...values })); + return true; }) - .catch(() => { - dispatch({ type: actionTypes.ITEM_UPDATE_FAILURE }); - setNotification(failureMessage)(dispatch); - }); + .catch(() => false); }; } diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.tsx index 687f5b612fb..5546f63a0de 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/DateCell.tsx @@ -1,76 +1,63 @@ -import { Component } from 'react'; -import PropTypes from 'prop-types'; - import DateTimePicker from 'lib/components/core/fields/DateTimePicker'; import moment from 'lib/moment'; -const sameDate = (a, b) => - (!a && !b) || (a && b && moment(a).isSame(b, 'minute')); -const datePropType = PropTypes.oneOfType([ - PropTypes.string, - PropTypes.instanceOf(Date), -]); +import { + LessonPlanDate, + LessonPlanItemUpdate, + LessonPlanItemUpdateField, +} from '../../../types'; + +interface DateCellProps { + fieldName: Exclude; + fieldValue?: LessonPlanDate; + startAt: LessonPlanDate; + bonusEndAt?: LessonPlanDate; + endAt?: LessonPlanDate; + updateItem: (payload: LessonPlanItemUpdate) => void; +} + +/** + * Renders one datetime field of a lesson plan item. Changes are handed to + * `updateItem`, which is responsible for coalescing and sending them — this + * component deliberately does not talk to the server itself. + */ +const DateCell = (props: DateCellProps): JSX.Element => { + const { fieldName, fieldValue, startAt, bonusEndAt, endAt, updateItem } = + props; -class DateCell extends Component { /** - * Updates a date value for a lesson plan item if the date has changed. - * If it is start_at that is shifted, shift existing end dates by the same amount. + * Reports a new value for this field. If it is start_at that is shifted, the + * existing end dates are shifted by the same amount. */ - updateItemDate = (_, newDate) => { - const { - fieldValue: oldDate, - fieldName, - updateItem, - startAt, - endAt, - bonusEndAt, - } = this.props; - - if (sameDate(oldDate, newDate)) { - return; - } + const updateItemDate = (_, newDate: Date | null): void => { + const payload: LessonPlanItemUpdate = { + [fieldName]: newDate ? moment(newDate).toISOString() : null, + }; - const payload = { [fieldName]: moment(newDate).toISOString() }; - if (startAt && fieldName === 'start_at') { + if (startAt && fieldName === 'start_at' && newDate) { const timeShift = moment.duration(moment(newDate).diff(moment(startAt))); if (endAt) { - const shiftedDate = moment(endAt); - shiftedDate.add(timeShift); - payload.end_at = shiftedDate.toISOString(); + payload.end_at = moment(endAt).add(timeShift).toISOString(); } if (bonusEndAt) { - const shiftedDate = moment(bonusEndAt); - shiftedDate.add(timeShift); - payload.bonus_end_at = shiftedDate.toISOString(); + payload.bonus_end_at = moment(bonusEndAt).add(timeShift).toISOString(); } } + updateItem(payload); }; - render() { - const { fieldName, fieldValue } = this.props; - - return ( - - - - ); - } -} - -DateCell.propTypes = { - fieldValue: datePropType, - fieldName: PropTypes.string.isRequired, - startAt: datePropType.isRequired, - endAt: datePropType, - bonusEndAt: datePropType, - updateItem: PropTypes.func.isRequired, + return ( + + + + ); }; export default DateCell; diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.tsx index 99e56529e9a..7ab5b4e4598 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/PublishedCell.tsx @@ -1,5 +1,5 @@ +import { ChangeEvent } from 'react'; import { Switch } from '@mui/material'; -import PropTypes from 'prop-types'; const styles = { toggle: { @@ -7,13 +7,21 @@ const styles = { }, }; -const PublishedCell = (props) => { - const { published, onToggle } = props; +interface PublishedCellProps { + published: boolean; + onToggle: (event: ChangeEvent, isToggled: boolean) => void; + disabled?: boolean; +} + +const PublishedCell = (props: PublishedCellProps): JSX.Element => { + const { published, onToggle, disabled } = props; + return ( @@ -21,9 +29,4 @@ const PublishedCell = (props) => { ); }; -PublishedCell.propTypes = { - published: PropTypes.bool.isRequired, - onToggle: PropTypes.func.isRequired, -}; - export default PublishedCell; diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx index c8e764e108d..8a5b77261cc 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx @@ -1,13 +1,21 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; +import { useRef, useState } from 'react'; +import { defineMessages } from 'react-intl'; import Link from 'lib/components/core/Link'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; +import { FIELD_LONG_DEBOUNCE_DELAY_MS } from 'lib/constants/sharedConstants'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import { useDebounce } from 'lib/hooks/useDebounce'; +import useTranslation from 'lib/hooks/useTranslation'; +import moment from 'lib/moment'; -import { fields } from '../../../constants'; import TranslatedItemType from '../../../containers/TranslatedItemType'; import { updateItem } from '../../../operations'; +import { + LessonPlanDate, + LessonPlanItemUpdate, + LessonPlanItemUpdateField, +} from '../../../types'; import DateCell from './DateCell'; import PublishedCell from './PublishedCell'; @@ -23,100 +31,166 @@ const translations = defineMessages({ }, }); -const datePropType = PropTypes.oneOfType([ - PropTypes.string, - PropTypes.instanceOf(Date), -]); +type ItemValue = LessonPlanItemUpdate[LessonPlanItemUpdateField]; -class ItemRow extends Component { - updateItem = (payload) => { - const { id, title, dispatch } = this.props; - const successMessage = ( - - ); - const failureMessage = ( - - ); - dispatch(updateItem(id, payload, successMessage, failureMessage)); - }; +// `start_at` is required server-side, so an empty field is a transient state +// while the instructor retypes the date rather than an edit worth sending. It is +// dropped here, leaving the item untouched until the field is valid again. +const REQUIRED_FIELDS: LessonPlanItemUpdateField[] = ['start_at']; + +const sameValue = ( + a: ItemValue | LessonPlanDate | undefined, + b: ItemValue | LessonPlanDate | undefined, +): boolean => { + if (typeof a === 'boolean' || typeof b === 'boolean') return a === b; + return Boolean((!a && !b) || (a && b && moment(a).isSame(b, 'minute'))); +}; + +interface ItemRowProps { + id: number; + type: string; + title: string; + startAt: LessonPlanDate; + bonusEndAt?: LessonPlanDate; + endAt?: LessonPlanDate; + published: boolean; + itemPath?: string; +} - updatePublished = (_, isToggled) => this.updateItem({ published: isToggled }); +const ItemRow = (props: ItemRowProps): JSX.Element | null => { + const { id, type, title, startAt, bonusEndAt, endAt, published, itemPath } = + props; - render() { - const { - type, - title, - startAt, - bonusEndAt, - endAt, - published, - visibility, - columnsVisible, - itemPath, - } = this.props; - - const isHidden = !visibility[type]; - if (isHidden) { - return null; + const dispatch = useAppDispatch(); + const { t } = useTranslation(); + const visibility = useAppSelector( + (state) => state.lessonPlan.lessonPlan.visibilityByType, + ); + const columnsVisible = useAppSelector( + (state) => state.lessonPlan.flags.editPageColumnsVisible, + ); + + // Changes queued but not yet sent. Merged so that editing several fields of the + // same row results in one request rather than one per field. + const pendingRef = useRef({}); + // The latest value we know for each field, whether or not it has been saved yet. + // Guards against re-sending a value the user has already queued. + const latestValuesRef = useRef({}); + // At most one request per row may be in flight. + const inFlightRef = useRef(false); + const [saving, setSaving] = useState(false); + + const flush = (context: SaveContext): void => { + if (inFlightRef.current) return; + + const payload = pendingRef.current; + pendingRef.current = {}; + + if (Object.keys(payload).length === 0) { + setSaving(false); + return; } - const dateProps = { - startAt, - bonusEndAt, - endAt, - updateItem: this.updateItem, + const successMessage = context.t(translations.updateSuccess, { + title: context.title, + }); + const failureMessage = context.t(translations.updateFailed, { + title: context.title, + }); + + inFlightRef.current = true; + context + .dispatch(updateItem(context.id, payload, successMessage, failureMessage)) + .finally(() => { + inFlightRef.current = false; + if (Object.keys(pendingRef.current).length > 0) { + flush(context); + } else { + setSaving(false); + } + }); + }; + + const debouncedFlush = useDebounce(flush, FIELD_LONG_DEBOUNCE_DELAY_MS, []); + + const queueUpdate = (payload: LessonPlanItemUpdate): void => { + const savedValues: Record = { + start_at: startAt, + bonus_end_at: bonusEndAt, + end_at: endAt, + published, }; - return ( - - {columnsVisible[fields.ITEM_TYPE] ? ( - - - - ) : null} - - {title} - - {columnsVisible[fields.START_AT] ? ( - - ) : null} - {columnsVisible[fields.BONUS_END_AT] ? ( - - ) : null} - {columnsVisible[fields.END_AT] ? ( - - ) : null} - {columnsVisible[fields.PUBLISHED] ? ( - - ) : null} - + const changes = Object.entries(payload).reduce( + (acc, [field, value]) => { + const key = field as LessonPlanItemUpdateField; + if (REQUIRED_FIELDS.includes(key) && !value) return acc; + + const latest = + key in latestValuesRef.current + ? latestValuesRef.current[key] + : (savedValues[key] as ItemValue); + if (sameValue(latest, value)) return acc; + return { ...acc, [key]: value }; + }, + {}, ); - } -} -ItemRow.propTypes = { - id: PropTypes.number.isRequired, - type: PropTypes.string.isRequired, - title: PropTypes.string.isRequired, - startAt: datePropType.isRequired, - endAt: datePropType, - bonusEndAt: datePropType, - published: PropTypes.bool.isRequired, - visibility: PropTypes.shape({}).isRequired, - columnsVisible: PropTypes.shape({}).isRequired, - itemPath: PropTypes.string, - - dispatch: PropTypes.func.isRequired, + if (Object.keys(changes).length === 0) return; + + latestValuesRef.current = { ...latestValuesRef.current, ...changes }; + pendingRef.current = { ...pendingRef.current, ...changes }; + setSaving(true); + debouncedFlush({ id, title, dispatch, t }); + }; + + const updatePublished = (_, isToggled: boolean): void => + queueUpdate({ published: isToggled }); + + if (!visibility[type]) return null; + + const dateProps = { + startAt, + bonusEndAt, + endAt, + updateItem: queueUpdate, + }; + + return ( + + {columnsVisible['ITEM_TYPE'] ? ( + + + + ) : null} + +
+ {title} + {saving ? : null} +
+ + {columnsVisible['START_AT'] ? ( + + ) : null} + {columnsVisible['BONUS_END_AT'] ? ( + + ) : null} + {columnsVisible['END_AT'] ? ( + + ) : null} + {columnsVisible['PUBLISHED'] ? ( + + ) : null} + + ); }; -export default connect(({ lessonPlan }) => ({ - visibility: lessonPlan.lessonPlan.visibilityByType, - columnsVisible: lessonPlan.flags.editPageColumnsVisible, -}))(ItemRow); +export default ItemRow; diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.tsx index 4985cf87286..9a20fb97825 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/MilestoneRow.tsx @@ -1,14 +1,18 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; +import { useEffect, useRef, useState } from 'react'; +import { defineMessages } from 'react-intl'; import { Element } from 'react-scroll'; -import PropTypes from 'prop-types'; +import { setNotification } from 'lib/actions'; import DateTimePicker from 'lib/components/core/fields/DateTimePicker'; +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; +import { FIELD_LONG_DEBOUNCE_DELAY_MS } from 'lib/constants/sharedConstants'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import { useDebounce } from 'lib/hooks/useDebounce'; +import useTranslation from 'lib/hooks/useTranslation'; import moment from 'lib/moment'; -import { fields } from '../../constants'; import { updateMilestone } from '../../operations'; +import { LessonPlanDate } from '../../types'; const translations = defineMessages({ updateSuccess: { @@ -21,69 +25,131 @@ const translations = defineMessages({ }, }); -const sameDate = (a, b) => - (!a && !b) || (a && b && moment(a).isSame(b, 'minute')); +const sameDate = (a?: LessonPlanDate, b?: LessonPlanDate): boolean => + Boolean((!a && !b) || (a && b && moment(a).isSame(b, 'minute'))); -class MilestoneRow extends Component { - updateMilestoneStartAt = (_, newDate, setError) => { - const { id, title, startAt, dispatch } = this.props; - if (sameDate(startAt, newDate)) { +type SetError = (...args: never[]) => void; + +interface PendingUpdate { + startAt: Date; + setError?: SetError; +} + +interface MilestoneRowProps { + id: number; + groupId: string; + title: string; + startAt: LessonPlanDate; +} + +const MilestoneRow = (props: MilestoneRowProps): JSX.Element => { + const { id, groupId, title, startAt } = props; + + const dispatch = useAppDispatch(); + const { t } = useTranslation(); + const columnsVisible = useAppSelector( + (state) => state.lessonPlan.flags.editPageColumnsVisible, + ); + + // See ItemRow: the edit is diffed against the saved value when it is flushed, + // and only one request per row may be in flight at a time. + const pendingRef = useRef(null); + const inFlightRef = useRef(false); + const [saving, setSaving] = useState(false); + const [revision, setRevision] = useState(0); + + // Mirrored so the debounced call always runs the newest closure: it is + // created once, and needs the props as they are when it fires rather than + // when it was made. + const flushRef = useRef<() => void>(() => {}); + + const flush = (): void => { + if (inFlightRef.current) return; + + const pending = pendingRef.current; + pendingRef.current = null; + + if (!pending || sameDate(startAt, pending.startAt)) { + setSaving(false); return; } - const successMessage = ( - - ); - const failureMessage = ; + inFlightRef.current = true; dispatch( - updateMilestone( - id, - { start_at: newDate }, - successMessage, - failureMessage, - setError, - ), - ); + updateMilestone(id, { start_at: pending.startAt }, pending.setError), + ).then((succeeded) => { + inFlightRef.current = false; + + // A newer edit is already queued, so let it give the verdict instead. + const superseded = Boolean(pendingRef.current); + + if (succeeded) { + dispatch(setNotification(t(translations.updateSuccess, { title }))); + } else if (!superseded) { + setRevision((current) => current + 1); + dispatch(setNotification(t(translations.updateFailed))); + } + + if (superseded) { + flushRef.current(); + } else { + setSaving(false); + } + }); }; - render() { - const { title, startAt, groupId, columnsVisible } = this.props; + useEffect(() => { + flushRef.current = flush; + }); - return ( - - -

- {title} -

- - {columnsVisible[fields.START_AT] ? ( - - - - ) : null} - {columnsVisible[fields.BONUS_END_AT] ? : null} - {columnsVisible[fields.END_AT] ? : null} - {columnsVisible[fields.PUBLISHED] ? : null} - - ); - } -} + const debouncedFlush = useDebounce( + () => flushRef.current(), + FIELD_LONG_DEBOUNCE_DELAY_MS, + [], + ); -MilestoneRow.propTypes = { - id: PropTypes.number.isRequired, - groupId: PropTypes.string.isRequired, - title: PropTypes.string.isRequired, - startAt: PropTypes.oneOfType([PropTypes.string, PropTypes.instanceOf(Date)]) - .isRequired, - columnsVisible: PropTypes.shape({}).isRequired, + const updateMilestoneStartAt = ( + _, + newDate: Date | null, + setError?: SetError, + ): void => { + // `start_at` is required server-side, so an empty field is a transient state + // while the date is retyped rather than an edit worth sending. + if (!newDate) return; - dispatch: PropTypes.func.isRequired, + const settled = !inFlightRef.current && !pendingRef.current; + if (settled && sameDate(startAt, newDate)) return; + + pendingRef.current = { startAt: newDate, setError }; + setSaving(true); + debouncedFlush(); + }; + + return ( + + +

+
+ {title} + {saving ? : null} +
+

+ + {columnsVisible.START_AT ? ( + + + + ) : null} + {columnsVisible.BONUS_END_AT ? : null} + {columnsVisible.END_AT ? : null} + {columnsVisible.PUBLISHED ? : null} + + ); }; -export default connect(({ lessonPlan }) => ({ - columnsVisible: lessonPlan.flags.editPageColumnsVisible, -}))(MilestoneRow); +export default MilestoneRow; diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.tsx index c111410256f..007fdca49ae 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/ItemRow.test.tsx @@ -1,7 +1,9 @@ import { createMockAdapter } from 'mocks/axiosMock'; -import { fireEvent, render, waitFor } from 'test-utils'; +import { AppState } from 'store'; +import { fireEvent, render, RenderResult, waitFor } from 'test-utils'; import CourseAPI from 'api/course'; +import { FIELD_LONG_DEBOUNCE_DELAY_MS } from 'lib/constants/sharedConstants'; import ItemRow from '../ItemRow'; @@ -10,6 +12,15 @@ const mock = createMockAdapter(CourseAPI.lessonPlan.client); const startAt = '01-01-2017'; const endAt = '02-02-2017'; +// Saves are debounced by FIELD_LONG_DEBOUNCE_DELAY_MS, so assertions have to +// outlast it. +const AFTER_DEBOUNCE = { timeout: 5000 }; + +const settleDebounce = (): Promise => + new Promise((resolve) => { + setTimeout(resolve, FIELD_LONG_DEBOUNCE_DELAY_MS + 500); + }); + const itemData = { id: 9, published: false, @@ -20,6 +31,8 @@ const itemData = { end_at: new Date(endAt), }; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. const state = { lessonPlan: { lessonPlan: { @@ -27,7 +40,25 @@ const state = { items: [itemData], }, }, -}; +} as unknown as Partial; + +const renderItemRow = (): RenderResult => + render( + , + { state }, + ); + +beforeEach(() => { + jest.clearAllMocks(); +}); describe('', () => { it('shifts end dates when start date is shifted', async () => { @@ -38,58 +69,149 @@ describe('', () => { const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateItem'); - const page = render( - , - { state }, - ); + const page = renderItemRow(); const input = await page.findByDisplayValue(startAt); fireEvent.change(input, { target: { value: newStartAt } }); fireEvent.blur(input); - await waitFor(() => - expect(spy).toHaveBeenCalledWith(itemData.id, { - item: { - start_at: '2017-02-01T16:00:00.000Z', - bonus_end_at: '2017-02-06T18:03:00.000Z', - end_at: '2017-03-05T16:00:00.000Z', - }, - }), + await waitFor( + () => + expect(spy).toHaveBeenCalledWith(itemData.id, { + item: { + start_at: '2017-02-01T16:00:00.000Z', + bonus_end_at: '2017-02-06T18:03:00.000Z', + end_at: '2017-03-05T16:00:00.000Z', + }, + }), + AFTER_DEBOUNCE, ); }); it('clears end date', async () => { const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateItem'); - const page = render( - , - { state }, - ); + const page = renderItemRow(); const input = await page.findByDisplayValue(endAt); fireEvent.change(input, { target: { value: '' } }); fireEvent.blur(input); - expect(spy).toHaveBeenCalledWith(itemData.id, { - item: { end_at: null }, - }); + await waitFor( + () => + expect(spy).toHaveBeenCalledWith(itemData.id, { + item: { end_at: null }, + }), + AFTER_DEBOUNCE, + ); + }); + + it('sends one request when a date is edited several times in quick succession', async () => { + const url = `/courses/${global.courseId}/lesson_plan/items/${itemData.id}`; + mock.onPatch(url).reply(200); + + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateItem'); + + const page = renderItemRow(); + + const input = await page.findByDisplayValue(endAt); + + // Typing a date passes through valid intermediate values, each of which + // fires `onChange`. Without debouncing these become concurrent PATCHes, + // which enqueue racing CoursewidePersonalizedTimelineUpdateJob runs. + fireEvent.change(input, { target: { value: '03-03-2017' } }); + fireEvent.change(input, { target: { value: '04-04-2017' } }); + fireEvent.blur(input); + + await waitFor( + () => + expect(spy).toHaveBeenCalledWith(itemData.id, { + item: { end_at: '2017-04-03T16:00:00.000Z' }, + }), + AFTER_DEBOUNCE, + ); + + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('sends nothing while start date is empty', async () => { + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateItem'); + + const page = renderItemRow(); + + const input = await page.findByDisplayValue(startAt); + fireEvent.change(input, { target: { value: '' } }); + + // start_at is required, so an empty field is a transient state on the way to + // a new date, not an update the server would accept. + await settleDebounce(); + + expect(spy).not.toHaveBeenCalled(); + }); + + it('sends the start date once it is valid again', async () => { + const url = `/courses/${global.courseId}/lesson_plan/items/${itemData.id}`; + mock.onPatch(url).reply(200); + + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateItem'); + + const page = renderItemRow(); + + const input = await page.findByDisplayValue(startAt); + fireEvent.change(input, { target: { value: '' } }); + fireEvent.change(input, { target: { value: '05-05-2017' } }); + + await waitFor( + () => + expect(spy).toHaveBeenCalledWith(itemData.id, { + item: expect.objectContaining({ + start_at: '2017-05-04T16:00:00.000Z', + }), + }), + AFTER_DEBOUNCE, + ); + + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('retries the same value after a failed save', async () => { + const url = `/courses/${global.courseId}/lesson_plan/items/${itemData.id}`; + mock.onPatch(url).reply(500); + + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateItem'); + + const page = renderItemRow(); + + const input = await page.findByDisplayValue(endAt); + fireEvent.change(input, { target: { value: '06-06-2017' } }); + + await waitFor(() => expect(spy).toHaveBeenCalledTimes(1), AFTER_DEBOUNCE); + + // The store never took the rejected value, so re-entering it is a real + // change and has to go out again — otherwise a transient failure can only be + // retried by first picking some other date. + const retryInput = await page.findByDisplayValue(endAt); + fireEvent.change(retryInput, { target: { value: '06-06-2017' } }); + + await waitFor(() => expect(spy).toHaveBeenCalledTimes(2), AFTER_DEBOUNCE); + }); + + it('shows a saving indicator until the update resolves', async () => { + const url = `/courses/${global.courseId}/lesson_plan/items/${itemData.id}`; + mock.onPatch(url).reply(200); + + const page = renderItemRow(); + + const input = await page.findByDisplayValue(endAt); + fireEvent.change(input, { target: { value: '05-05-2017' } }); + + expect(await page.findByTestId('CircularProgress')).toBeVisible(); + + await waitFor( + () => expect(page.queryByTestId('CircularProgress')).toBeNull(), + AFTER_DEBOUNCE, + ); }); }); diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.tsx index 70c386317cf..b20f736a564 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/MilestoneRow.test.tsx @@ -1,7 +1,9 @@ import { createMockAdapter } from 'mocks/axiosMock'; -import { fireEvent, render, waitFor } from 'test-utils'; +import { AppState } from 'store'; +import { fireEvent, render, RenderResult, waitFor } from 'test-utils'; import CourseAPI from 'api/course'; +import { FIELD_LONG_DEBOUNCE_DELAY_MS } from 'lib/constants/sharedConstants'; import MilestoneRow from '../MilestoneRow'; @@ -9,17 +11,44 @@ const mock = createMockAdapter(CourseAPI.lessonPlan.client); beforeEach(() => { mock.reset(); + jest.clearAllMocks(); }); const startAt = '03-03-2017'; const newStartAt = '03-03-2018'; +// Saves are debounced by FIELD_LONG_DEBOUNCE_DELAY_MS, so assertions have to +// outlast it. +const AFTER_DEBOUNCE = { timeout: 5000 }; + +const settleDebounce = (): Promise => + new Promise((resolve) => { + setTimeout(resolve, FIELD_LONG_DEBOUNCE_DELAY_MS + 500); + }); + const milestoneData = { id: 6, title: 'Week 1', start_at: new Date(startAt), }; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. +const state = { + lessonPlan: { milestones: [milestoneData] }, +} as unknown as Partial; + +const renderMilestoneRow = (): RenderResult => + render( + , + { state }, + ); + describe('', () => { it('allows milestone start_at to be updated', async () => { const url = `/courses/${global.courseId}/lesson_plan/milestones/${milestoneData.id}`; @@ -27,25 +56,79 @@ describe('', () => { const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateMilestone'); - const page = render( - , - { state: { lessonPlan: { milestones: [milestoneData] } } }, + const page = renderMilestoneRow(); + + const input = await page.findByDisplayValue(startAt); + + fireEvent.change(input, { target: { value: newStartAt } }); + fireEvent.blur(input); + + await waitFor( + () => + expect(spy).toHaveBeenCalledWith(milestoneData.id, { + lesson_plan_milestone: { start_at: new Date(newStartAt) }, + }), + AFTER_DEBOUNCE, ); + }); + + it('sends one request when the date is edited several times in quick succession', async () => { + const url = `/courses/${global.courseId}/lesson_plan/milestones/${milestoneData.id}`; + mock.onPatch(url).reply(200); + + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateMilestone'); + + const page = renderMilestoneRow(); const input = await page.findByDisplayValue(startAt); + fireEvent.change(input, { target: { value: '04-04-2018' } }); fireEvent.change(input, { target: { value: newStartAt } }); fireEvent.blur(input); - await waitFor(() => - expect(spy).toHaveBeenCalledWith(milestoneData.id, { - lesson_plan_milestone: { start_at: new Date(newStartAt) }, - }), + await waitFor( + () => + expect(spy).toHaveBeenCalledWith(milestoneData.id, { + lesson_plan_milestone: { start_at: new Date(newStartAt) }, + }), + AFTER_DEBOUNCE, ); + + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('retries the same date after a failed save', async () => { + const url = `/courses/${global.courseId}/lesson_plan/milestones/${milestoneData.id}`; + mock.onPatch(url).reply(500); + + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateMilestone'); + + const page = renderMilestoneRow(); + + const input = await page.findByDisplayValue(startAt); + fireEvent.change(input, { target: { value: newStartAt } }); + + await waitFor(() => expect(spy).toHaveBeenCalledTimes(1), AFTER_DEBOUNCE); + + // See ItemRow: the rejected date never reached the store, so re-entering it + // is a real change. + const retryInput = await page.findByDisplayValue(startAt); + fireEvent.change(retryInput, { target: { value: newStartAt } }); + + await waitFor(() => expect(spy).toHaveBeenCalledTimes(2), AFTER_DEBOUNCE); + }); + + it('sends nothing while the date is empty', async () => { + const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateMilestone'); + + const page = renderMilestoneRow(); + + const input = await page.findByDisplayValue(startAt); + fireEvent.change(input, { target: { value: '' } }); + + // A milestone is a lesson plan item, so its start_at is required too. + await settleDebounce(); + + expect(spy).not.toHaveBeenCalled(); }); }); diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.tsx index 2da192fb97c..149586abd86 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/__test__/index.test.tsx @@ -1,26 +1,34 @@ +import { AppState } from 'store'; import { render, waitFor } from 'test-utils'; +import { + LessonPlanGroup, + LessonPlanItem, + LessonPlanMilestone, +} from '../../../types'; import { LessonPlanEdit } from '../index'; -const groups = [ +const milestone: LessonPlanMilestone = { + id: 6, + title: 'Week 1', + start_at: '2017-01-01T02:03:00.000+08:00', +}; + +const item: LessonPlanItem = { + id: 9, + published: false, + title: 'Other Event', + start_at: '2017-01-04T02:03:00.000+08:00', + bonus_end_at: '2017-01-06T02:03:00.000+08:00', + end_at: '2017-01-08T02:03:00.000+08:00', + itemTypeKey: 'Event', +}; + +const groups: LessonPlanGroup[] = [ { id: 'milestone-group-6', - milestone: { - id: 6, - title: 'Week 1', - start_at: '2017-01-01T02:03:00.000+08:00', - }, - items: [ - { - id: 9, - published: false, - title: 'Other Event', - start_at: '2017-01-04T02:03:00.000+08:00', - bonus_end_at: '2017-01-06T02:03:00.000+08:00', - end_at: '2017-01-08T02:03:00.000+08:00', - itemTypeKey: 'Event', - }, - ], + milestone, + items: [item], }, ]; @@ -32,6 +40,8 @@ const columnsVisible = { PUBLISHED: true, }; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. const state = { lessonPlan: { lessonPlan: { @@ -39,7 +49,7 @@ const state = { columnsVisible, }, }, -}; +} as unknown as Partial; describe('', () => { it('renders item and milestone rows', async () => { @@ -53,8 +63,8 @@ describe('', () => { ); await waitFor(() => { - expect(page.getByText(groups[0].items[0].title)).toBeVisible(); - expect(page.getByText(groups[0].milestone.title)).toBeVisible(); + expect(page.getByText(item.title)).toBeVisible(); + expect(page.getByText(milestone.title)).toBeVisible(); }); }); }); diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.tsx index b5bec7bb962..8867a3fb97a 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/index.tsx @@ -1,26 +1,30 @@ -import { Component } from 'react'; -import { FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; - import Page from 'lib/components/core/layouts/Page'; import { getCourseId } from 'lib/helpers/url-helpers'; -import { lessonPlanTypesGroups } from 'lib/types'; +import { useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; -import { fields } from '../../constants'; import ColumnVisibilityDropdown from '../../containers/ColumnVisibilityDropdown'; import NewEventButton from '../../containers/LessonPlanLayout/NewEventButton'; import NewMilestoneButton from '../../containers/LessonPlanLayout/NewMilestoneButton'; import translations from '../../translations'; +import { LessonPlanEditColumn, LessonPlanGroup } from '../../types'; import ItemRow from './ItemRow'; import MilestoneRow from './MilestoneRow'; -const { ITEM_TYPE, TITLE, START_AT, BONUS_END_AT, END_AT, PUBLISHED } = fields; +interface LessonPlanEditProps { + groups: LessonPlanGroup[]; + columnsVisible: Record; + canManageLessonPlan: boolean; +} + +export const LessonPlanEdit = (props: LessonPlanEditProps): JSX.Element => { + const { groups, columnsVisible, canManageLessonPlan } = props; + + const { t } = useTranslation(); + const courseId = getCourseId(); -export class LessonPlanEdit extends Component { - // eslint-disable-next-line class-methods-use-this - renderGroup = (group) => { + const renderGroup = (group: LessonPlanGroup): JSX.Element[] => { const { id, milestone, items } = group; const rows = items @@ -34,7 +38,7 @@ export class LessonPlanEdit extends Component { published={item.published} startAt={item.start_at} title={item.title} - type={item.itemTypeKey} + type={item.itemTypeKey ?? ''} /> )) : []; @@ -45,7 +49,7 @@ export class LessonPlanEdit extends Component { key={`milestone-${id}`} groupId={id} id={milestone.id} - startAt={milestone.start_at} + startAt={milestone.start_at ?? null} title={milestone.title} />, ); @@ -54,65 +58,54 @@ export class LessonPlanEdit extends Component { return rows; }; - renderTableHeader() { - const { columnsVisible } = this.props; + const headerFor = (field: LessonPlanEditColumn): JSX.Element => ( + {t(translations[field])} + ); - const headerFor = (field) => ( - - - - ); - return ( - - - {columnsVisible[ITEM_TYPE] ? headerFor(ITEM_TYPE) : null} - {headerFor(TITLE)} - {columnsVisible[START_AT] ? headerFor(START_AT) : null} - {columnsVisible[BONUS_END_AT] ? headerFor(BONUS_END_AT) : null} - {columnsVisible[END_AT] ? headerFor(END_AT) : null} - {columnsVisible[PUBLISHED] ? headerFor(PUBLISHED) : null} - - - ); - } - - render() { - const { groups } = this.props; - const courseId = getCourseId(); + return ( + + + + + + ) + } + backTo={`/courses/${courseId}/lesson_plan`} + title={t(translations.editLessonPlan)} + > +
+ + + + {columnsVisible.ITEM_TYPE ? headerFor('ITEM_TYPE') : null} + + {columnsVisible.START_AT ? headerFor('START_AT') : null} + {columnsVisible.BONUS_END_AT ? headerFor('BONUS_END_AT') : null} + {columnsVisible.END_AT ? headerFor('END_AT') : null} + {columnsVisible.PUBLISHED ? headerFor('PUBLISHED') : null} + + + {groups.map(renderGroup)} +
{t(translations.title)}
+
+ + ); +}; - return ( - - - - - - ) - } - backTo={`/courses/${courseId}/lesson_plan`} - title={} - > -
- - {this.renderTableHeader()} - {groups.map(this.renderGroup)} -
-
- - ); - } -} +const ConnectedLessonPlanEdit = (): JSX.Element => { + const groups = useAppSelector((state) => state.lessonPlan.lessonPlan.groups); + const flags = useAppSelector((state) => state.lessonPlan.flags); -LessonPlanEdit.propTypes = { - groups: lessonPlanTypesGroups.isRequired, - columnsVisible: PropTypes.shape({}).isRequired, - canManageLessonPlan: PropTypes.bool.isRequired, + return ( + + ); }; -export default connect(({ lessonPlan }) => ({ - groups: lessonPlan.lessonPlan.groups, - columnsVisible: lessonPlan.flags.editPageColumnsVisible, - canManageLessonPlan: lessonPlan.flags.canManageLessonPlan, -}))(LessonPlanEdit); +export default ConnectedLessonPlanEdit; diff --git a/client/app/bundles/course/lesson-plan/types.ts b/client/app/bundles/course/lesson-plan/types.ts new file mode 100644 index 00000000000..35b2d61847b --- /dev/null +++ b/client/app/bundles/course/lesson-plan/types.ts @@ -0,0 +1,146 @@ +import { ReactNode } from 'react'; +import { UseFormSetError } from 'react-hook-form'; +import { AppDispatch } from 'store'; + +import { MessageTranslator } from 'lib/hooks/useTranslation'; + +/** A datetime as it arrives from the server, or as constructed by the pickers. */ +export type LessonPlanDate = string | Date | null; + +/** + * A partial update to a lesson plan item. Dates are ISO strings, or `null` when + * the field is being cleared. + */ +export interface LessonPlanItemUpdate { + start_at?: string | null; + bonus_end_at?: string | null; + end_at?: string | null; + published?: boolean; +} + +export type LessonPlanItemUpdateField = keyof LessonPlanItemUpdate; + +/** + * A lesson plan item as the server serialises it: the fields common to every + * item (see `_item.json.jbuilder`), plus the actable-specific ones these pages + * read. + * + * There is no discriminant on the payload — `lesson_plan_item_type` is an array + * of instructor-defined tab titles, not a type tag — so the actable-specific + * fields are optional rather than a discriminated union. + */ +export interface LessonPlanItem { + id: number; + title: string; + published: boolean; + start_at: LessonPlanDate; + bonus_end_at?: LessonPlanDate; + end_at?: LessonPlanDate; + lesson_plan_item_type?: string[]; + /** Derived client-side by `generateTypeKey`, not sent by the server. */ + itemTypeKey?: string; + item_path?: string; + eventId?: number; + location?: string; + description?: string; +} + +/** + * A lesson plan item that is a course event. Every field is optional, mirroring + * the shape `AdminTools` accepted before the conversion: `id` is the lesson plan + * item, `eventId` the event resource behind it. + */ +export interface LessonPlanEventItem { + id?: number; + eventId?: number; + title?: string; + published?: boolean; + location?: string; + description?: string; + start_at?: LessonPlanDate; + end_at?: LessonPlanDate; + lesson_plan_item_type?: string[]; +} + +/** A milestone as the server sends it. */ +export interface LessonPlanMilestone { + id: number; + title: string; + description?: string | null; + start_at?: LessonPlanDate; +} + +/** + * What the show page's admin tools receive. `LessonPlanGroup` synthesises an + * "Ungrouped Items" placeholder with a null id and a translated element for a + * title, and the tools hide themselves for it. + */ +export interface MilestoneOrPlaceholder { + id?: number | null; + title?: ReactNode; + description?: string | null; + start_at?: LessonPlanDate; +} + +/** Values submitted by the milestone form. */ +export interface MilestoneFormValues { + title?: string; + description?: string; + start_at?: LessonPlanDate; +} + +/** Values submitted by the event form. */ +export interface EventFormValues { + title?: string; + event_type?: string; + location?: string; + description?: string; + start_at?: LessonPlanDate; + end_at?: LessonPlanDate; + published?: boolean; +} + +/** + * A dialog's submit handler, supplied by whoever opens the dialog. Resolves to + * whether the request succeeded, which is what tells the dialog to close itself. + */ +export type FormSubmitHandler = ( + values: Values, + setError: UseFormSetError>, +) => Promise; + +/** + * What a queued request needs. Passed to the debounced flush as an argument so + * that it never closes over props that may have moved on since the edit was + * queued. + */ +export interface SaveContext { + id: number; + title: string; + dispatch: AppDispatch; + t: MessageTranslator; +} + +/** + * The parts of the lesson plan slice these pages read. The reducers are still + * JavaScript, so `combineReducers` cannot infer their shape and selectors have + * to assert it. Drop the assertions once the reducers are converted. + */ +export interface LessonPlanSliceState { + visibilityByType: Record; + items: LessonPlanItem[]; + isLoading: boolean; + groups: LessonPlanGroup[]; +} + +/** Items grouped under the milestone they fall after; see `groupItemsUnderMilestones`. */ +export interface LessonPlanGroup { + id: string; + milestone: LessonPlanMilestone | null; + items: LessonPlanItem[]; +} + +export interface LessonPlanFlagsState { + canManageLessonPlan: boolean; + editPageColumnsVisible: Record; +} From 5f40c0fccf836a8d03db93bf81a189a244ab745c Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Wed, 26 Aug 2026 10:41:24 +0800 Subject: [PATCH 3/4] refactor(lesson-plan): convert LessonPlanLayout, event, milestone forms to typescript - migrate onSubmit() hook from redux store to parent components - remove obsolete javascript reducers (information moved to component state) --- .../bundles/course/lesson-plan/constants.ts | 4 - .../__test__/index.test.tsx | 25 +++ .../ColumnVisibilityDropdown/index.tsx | 135 ++++++------- .../containers/EventFormDialog/EventForm.jsx | 25 +-- .../containers/EventFormDialog/index.tsx | 110 +++++----- .../LessonPlanFilter/__test__/index.test.tsx | 34 ++++ .../containers/LessonPlanFilter/index.tsx | 138 ++++++------- .../LessonPlanLayout/EnterEditModeButton.tsx | 11 +- .../LessonPlanLayout/NewEventButton.tsx | 97 ++++----- .../LessonPlanLayout/NewMilestoneButton.tsx | 87 ++++---- .../__test__/NewEventButton.test.tsx | 15 +- .../__test__/NewMilestoneButton.test.tsx | 15 +- .../containers/LessonPlanLayout/index.tsx | 73 +++---- .../MilestoneFormDialog/MilestoneForm.jsx | 9 +- .../containers/MilestoneFormDialog/index.tsx | 78 ++++---- .../bundles/course/lesson-plan/operations.ts | 17 +- .../pages/LessonPlanEdit/ItemRow/index.tsx | 138 ++++++++----- .../LessonPlanItem/AdminTools.tsx | 188 ++++++++---------- .../__test__/AdminTools.test.tsx | 37 ++-- .../LessonPlanShow/MilestoneAdminTools.tsx | 168 ++++++++-------- .../__test__/MilestoneAdminTools.test.tsx | 44 ++-- .../course/lesson-plan/reducers/eventForm.js | 33 --- .../lesson-plan/reducers/milestoneForm.js | 33 --- .../app/bundles/course/lesson-plan/store.ts | 14 -- .../course/lesson-plan/translations.ts | 32 +-- client/app/routers/course/lessonPlan.tsx | 1 - 26 files changed, 748 insertions(+), 813 deletions(-) create mode 100644 client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/__test__/index.test.tsx create mode 100644 client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/__test__/index.test.tsx delete mode 100644 client/app/bundles/course/lesson-plan/reducers/eventForm.js delete mode 100644 client/app/bundles/course/lesson-plan/reducers/milestoneForm.js diff --git a/client/app/bundles/course/lesson-plan/constants.ts b/client/app/bundles/course/lesson-plan/constants.ts index 126cc9addb1..08eaf3290d1 100644 --- a/client/app/bundles/course/lesson-plan/constants.ts +++ b/client/app/bundles/course/lesson-plan/constants.ts @@ -23,8 +23,6 @@ const actionTypes = mirrorCreator([ 'ITEM_UPDATE_REQUEST', 'ITEM_UPDATE_SUCCESS', 'ITEM_UPDATE_FAILURE', - 'EVENT_FORM_SHOW', - 'EVENT_FORM_HIDE', 'EVENT_UPDATE_REQUEST', 'EVENT_UPDATE_SUCCESS', 'EVENT_UPDATE_FAILURE', @@ -34,8 +32,6 @@ const actionTypes = mirrorCreator([ 'EVENT_DELETE_REQUEST', 'EVENT_DELETE_SUCCESS', 'EVENT_DELETE_FAILURE', - 'MILESTONE_FORM_SHOW', - 'MILESTONE_FORM_HIDE', 'MILESTONE_UPDATE_REQUEST', 'MILESTONE_UPDATE_SUCCESS', 'MILESTONE_UPDATE_FAILURE', diff --git a/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/__test__/index.test.tsx b/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/__test__/index.test.tsx new file mode 100644 index 00000000000..54471655bb8 --- /dev/null +++ b/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/__test__/index.test.tsx @@ -0,0 +1,25 @@ +import { fireEvent, render, waitFor, within } from 'test-utils'; + +import ColumnVisibilityDropdown from '..'; + +const DONE_ICON = 'DoneIcon'; + +describe('', () => { + it('toggles a column through the store', async () => { + const page = render(); + + fireEvent.click(await page.findByRole('button', { name: 'Columns' })); + + // The tick is rendered from the value the store gives back, so clicking and + // watching it exercises the whole round trip — including the shape of the + // payload the action creator is handed. + const endAt = await page.findByRole('menuitem', { name: 'End At' }); + expect(within(endAt).queryByTestId(DONE_ICON)).toBeInTheDocument(); + + fireEvent.click(endAt); + + await waitFor(() => + expect(within(endAt).queryByTestId(DONE_ICON)).not.toBeInTheDocument(), + ); + }); +}); diff --git a/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.tsx b/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.tsx index d347680f3e0..64f1552d9d1 100644 --- a/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.tsx +++ b/client/app/bundles/course/lesson-plan/containers/ColumnVisibilityDropdown/index.tsx @@ -1,17 +1,16 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; +import { MouseEvent, useState } from 'react'; +import { defineMessages } from 'react-intl'; import Done from '@mui/icons-material/Done'; import KeyboardArrowDown from '@mui/icons-material/KeyboardArrowDown'; import { Button, MenuItem, MenuList, Popover } from '@mui/material'; -import PropTypes from 'prop-types'; -import { fields } from '../../constants'; +import { LESSON_PLAN_EDIT_COLUMNS } from 'course/lesson-plan/types'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; + import { actions } from '../../store'; import fieldTranslations from '../../translations'; -const { ITEM_TYPE, START_AT, BONUS_END_AT, END_AT, PUBLISHED } = fields; - const translations = defineMessages({ label: { id: 'course.lessonPlan.ColumnVisibilityDropdown.label', @@ -19,82 +18,64 @@ const translations = defineMessages({ }, }); -class ColumnVisibilityDropdown extends Component { - constructor(props) { - super(props); +const ColumnVisibilityDropdown = (): JSX.Element => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const columnsVisible = useAppSelector( + (state) => state.lessonPlan.flags.editPageColumnsVisible, + ); - this.state = { - open: false, - }; - } + const [anchorEl, setAnchorEl] = useState(null); - handleClick = (event) => { + const handleClick = (event: MouseEvent): void => { // This prevents ghost click. event.preventDefault(); - - this.setState({ - open: true, - anchorEl: event.currentTarget, - }); + setAnchorEl(event.currentTarget); }; - handleRequestClose = () => { - this.setState({ - open: false, - }); - }; - - render() { - const { dispatch, columnsVisible } = this.props; - - return ( - <> - - - - - {[ITEM_TYPE, START_AT, BONUS_END_AT, END_AT, PUBLISHED].map( - (field) => { - const isVisible = columnsVisible[field]; - return ( - - dispatch(actions.setColumnVisibility(field, !isVisible)) - } - style={{ display: 'flex', justifyContent: 'space-between' }} - > - - {isVisible && } - - ); - }, - )} - - - - ); - } -} + return ( + <> + -ColumnVisibilityDropdown.propTypes = { - columnsVisible: PropTypes.shape({}).isRequired, - dispatch: PropTypes.func.isRequired, + setAnchorEl(null)} + open={Boolean(anchorEl)} + transformOrigin={{ horizontal: 'left', vertical: 'top' }} + > + + {LESSON_PLAN_EDIT_COLUMNS.map((field) => { + const isVisible = columnsVisible[field]; + return ( + { + dispatch( + actions.setColumnVisibility({ + field, + isVisible: !isVisible, + }), + ); + }} + style={{ display: 'flex', justifyContent: 'space-between' }} + > + {t(fieldTranslations[field])} + {isVisible && } + + ); + })} + + + + ); }; -export default connect(({ lessonPlan }) => ({ - columnsVisible: lessonPlan.flags.editPageColumnsVisible, -}))(ColumnVisibilityDropdown); +export default ColumnVisibilityDropdown; diff --git a/client/app/bundles/course/lesson-plan/containers/EventFormDialog/EventForm.jsx b/client/app/bundles/course/lesson-plan/containers/EventFormDialog/EventForm.jsx index 616e242e890..d58e588e0ff 100644 --- a/client/app/bundles/course/lesson-plan/containers/EventFormDialog/EventForm.jsx +++ b/client/app/bundles/course/lesson-plan/containers/EventFormDialog/EventForm.jsx @@ -13,19 +13,8 @@ import FormTextField from 'lib/components/form/fields/TextField'; import FormToggleField from 'lib/components/form/fields/ToggleField'; import formTranslations from 'lib/translations/form'; -import { fields } from '../../constants'; import translations from '../../translations'; -const { - TITLE, - EVENT_TYPE, - LOCATION, - DESCRIPTION, - START_AT, - END_AT, - PUBLISHED, -} = fields; - const styles = { columns: { display: 'flex', @@ -103,7 +92,7 @@ const EventForm = (props) => { InputLabelProps={{ shrink: true, }} - label={} + label={} required variant="standard" /> @@ -119,7 +108,7 @@ const EventForm = (props) => { field={field} fieldState={fieldState} fullWidth - label={} + label={} options={eventTypes} selectOnFocus style={styles.eventType} @@ -135,7 +124,7 @@ const EventForm = (props) => { field={field} fieldState={fieldState} fullWidth - label={} + label={} options={eventLocations} selectOnFocus style={styles.eventType} @@ -155,7 +144,7 @@ const EventForm = (props) => { InputLabelProps={{ shrink: true, }} - label={} + label={} multiline rows={2} variant="standard" @@ -171,7 +160,7 @@ const EventForm = (props) => { disabled={disabled} field={field} fieldState={fieldState} - label={} + label={} style={styles.oneColumn} /> )} @@ -184,7 +173,7 @@ const EventForm = (props) => { disabled={disabled} field={field} fieldState={fieldState} - label={} + label={} style={styles.oneColumn} /> )} @@ -198,7 +187,7 @@ const EventForm = (props) => { disabled={disabled} field={field} fieldState={fieldState} - label={} + label={} style={styles.toggle} /> )} diff --git a/client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.tsx b/client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.tsx index e0b89c0a2dd..be0d3d23fdf 100644 --- a/client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.tsx +++ b/client/app/bundles/course/lesson-plan/containers/EventFormDialog/index.tsx @@ -1,25 +1,42 @@ import { useState } from 'react'; -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; import FormDialogue from 'lib/components/form/FormDialogue'; +import { useAppSelector } from 'lib/hooks/store'; -import { actions } from '../../store'; +import { EventFormValues, FormSubmitHandler } from '../../types'; import EventForm from './EventForm'; -const EventFormDialog = ({ - visible, - disabled, - formTitle, - initialValues, - onSubmit, - dispatch, - items, -}) => { +interface EventFormDialogProps { + open: boolean; + onClose: () => void; + formTitle?: string; + initialValues: EventFormValues; + onSubmit: FormSubmitHandler; +} + +interface EventSuggestions { + eventTypes: string[]; + eventLocations: string[]; +} + +/** + * Controlled by whoever opens it: the owner supplies the handler and the initial + * values, and the dialog closes itself once `onSubmit` reports success. The + * handler used to be stashed in the Redux store, which is not serialisable. + * + * The existing event types and locations are still read from the store, since + * they are derived from the lesson plan itself rather than from the opener. + */ +const EventFormDialog = (props: EventFormDialogProps): JSX.Element => { + const { open, onClose, formTitle, initialValues, onSubmit } = props; + const [isDirty, setIsDirty] = useState(false); + const [submitting, setSubmitting] = useState(false); - const { eventTypes, eventLocations } = items.reduce( + const items = useAppSelector((state) => state.lessonPlan.lessonPlan.items); + + const { eventTypes, eventLocations } = items.reduce( (values, item) => { if (!item.eventId) { return values; @@ -27,66 +44,47 @@ const EventFormDialog = ({ if (item.location) { values.eventLocations.push(item.location); } - values.eventTypes.push(item.lesson_plan_item_type[0]); + if (item.lesson_plan_item_type?.[0]) { + values.eventTypes.push(item.lesson_plan_item_type[0]); + } return values; }, { eventTypes: [], eventLocations: [] }, ); + const handleSubmit: FormSubmitHandler = async ( + data, + setError, + ) => { + setSubmitting(true); + try { + const succeeded = await onSubmit(data, setError); + if (succeeded) onClose(); + return succeeded; + } finally { + setSubmitting(false); + } + }; + return ( dispatch(actions.hideEventForm())} - open={visible} + hideForm={onClose} + open={open} skipConfirmation={!isDirty} title={formTitle} > ); }; -EventFormDialog.defaultProps = { - visible: false, - disabled: false, -}; - -EventFormDialog.propTypes = { - visible: PropTypes.bool, - disabled: PropTypes.bool, - formTitle: PropTypes.string, - initialValues: PropTypes.shape({ - id: PropTypes.number, - eventId: PropTypes.number, - title: PropTypes.string, - event_type: PropTypes.string, - location: PropTypes.string, - description: PropTypes.string, - start_at: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.instanceOf(Date), - ]), - end_at: PropTypes.oneOfType([PropTypes.string, PropTypes.instanceOf(Date)]), - published: PropTypes.bool, - }), - items: PropTypes.arrayOf( - PropTypes.shape({ - eventId: PropTypes.number, - location: PropTypes.string, - lesson_plan_item_type: PropTypes.arrayOf(PropTypes.string), - }), - ), - onSubmit: PropTypes.func.isRequired, - dispatch: PropTypes.func.isRequired, -}; - -export default connect(({ lessonPlan }) => ({ - ...lessonPlan.eventForm, - items: lessonPlan.lessonPlan.items, -}))(EventFormDialog); +export default EventFormDialog; diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/__test__/index.test.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/__test__/index.test.tsx new file mode 100644 index 00000000000..b13a110a55d --- /dev/null +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/__test__/index.test.tsx @@ -0,0 +1,34 @@ +import { AppState } from 'store'; +import { fireEvent, render, waitFor, within } from 'test-utils'; + +import LessonPlanFilter from '..'; + +const DONE_ICON = 'DoneIcon'; + +// `Partial` only allows omitting whole slices, and this test seeds +// just the field the component reads, so the shape is asserted. +const state = { + lessonPlan: { lessonPlan: { visibilityByType: { Event: true } } }, +} as unknown as Partial; + +describe('', () => { + it('toggles an item type through the store', async () => { + const page = render(, { state }); + + fireEvent.click(await page.findByRole('button', { name: 'Filter' })); + + // The tick is rendered from the value the store gives back, so clicking and + // watching it exercises the whole round trip — including the shape of the + // payload the action creator is handed. + const eventType = await page.findByRole('menuitem', { name: /Event/ }); + expect(within(eventType).queryByTestId(DONE_ICON)).toBeInTheDocument(); + + fireEvent.click(eventType); + + await waitFor(() => + expect( + within(eventType).queryByTestId(DONE_ICON), + ).not.toBeInTheDocument(), + ); + }); +}); diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.tsx index 61431219d76..b127f981bde 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanFilter/index.tsx @@ -1,10 +1,11 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; +import { MouseEvent, useState } from 'react'; +import { defineMessages } from 'react-intl'; import Done from '@mui/icons-material/Done'; import KeyboardArrowUp from '@mui/icons-material/KeyboardArrowUp'; import { Button, MenuItem, MenuList, Popover } from '@mui/material'; -import PropTypes from 'prop-types'; + +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; import { actions } from '../../store'; import TranslatedItemType from '../TranslatedItemType'; @@ -16,86 +17,67 @@ const translations = defineMessages({ }, }); -class LessonPlanFilter extends Component { - constructor(props) { - super(props); +const LessonPlanFilter = (): JSX.Element | null => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const visibility = useAppSelector( + (state) => state.lessonPlan.lessonPlan.visibilityByType, + ); + + const [anchorEl, setAnchorEl] = useState(null); - this.state = { - open: false, - }; - } + const itemTypes = Object.keys(visibility); + if (itemTypes.length < 1) return null; - handleClick = (event) => { + const handleClick = (event: MouseEvent): void => { // This prevents ghost click. event.preventDefault(); - - this.setState({ - open: true, - anchorEl: event.currentTarget, - }); + setAnchorEl(event.currentTarget); }; - handleRequestClose = () => { - this.setState({ - open: false, - }); - }; - - render() { - const { dispatch, visibility } = this.props; - const itemTypes = Object.keys(visibility); - - if (itemTypes.length < 1) { - return null; - } - - return ( - <> - - - - {itemTypes.map((itemType) => { - const isVisible = visibility[itemType]; - return ( - - dispatch( - actions.setItemTypeVisibility(itemType, !isVisible), - ) - } - style={{ display: 'flex', justifyContent: 'space-between' }} - > - - {isVisible && } - - ); - })} - - - - ); - } -} + return ( + <> + -LessonPlanFilter.propTypes = { - visibility: PropTypes.shape({}).isRequired, - dispatch: PropTypes.func.isRequired, + setAnchorEl(null)} + open={Boolean(anchorEl)} + transformOrigin={{ horizontal: 'right', vertical: 'bottom' }} + > + + {itemTypes.map((itemType) => { + const isVisible = visibility[itemType]; + return ( + { + dispatch( + actions.setItemTypeVisibility({ + itemType, + isVisible: !isVisible, + }), + ); + }} + style={{ display: 'flex', justifyContent: 'space-between' }} + > + + {isVisible && } + + ); + })} + + + + ); }; -export default connect(({ lessonPlan }) => ({ - visibility: lessonPlan.lessonPlan.visibilityByType, -}))(LessonPlanFilter); +export default LessonPlanFilter; diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.tsx index 0441c9bcbdc..91024b24e28 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/EnterEditModeButton.tsx @@ -1,8 +1,9 @@ -import { defineMessages, FormattedMessage } from 'react-intl'; +import { defineMessages } from 'react-intl'; import { useNavigate } from 'react-router-dom'; import { Button } from '@mui/material'; import { getCourseId } from 'lib/helpers/url-helpers'; +import useTranslation from 'lib/hooks/useTranslation'; const translations = defineMessages({ enterEditMode: { @@ -11,15 +12,17 @@ const translations = defineMessages({ }, }); -const EnterEditModeButton = () => { +const EnterEditModeButton = (): JSX.Element => { + const { t } = useTranslation(); const navigate = useNavigate(); const courseId = getCourseId(); + return ( ); }; diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.tsx index b1b05ce01d4..2b5e9487baa 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewEventButton.tsx @@ -1,12 +1,13 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage, injectIntl } from 'react-intl'; -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; +import { useState } from 'react'; +import { defineMessages } from 'react-intl'; import AddButton from 'lib/components/core/buttons/AddButton'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; import { createEvent } from '../../operations'; -import { actions } from '../../store'; +import { EventFormValues, FormSubmitHandler } from '../../types'; +import EventFormDialog from '../EventFormDialog'; const translations = defineMessages({ newEvent: { @@ -23,53 +24,55 @@ const translations = defineMessages({ }, }); -class NewEventButton extends Component { - createEventHandler = (data) => { - const { dispatch } = this.props; - const successMessage = ; - const failureMessage = ; - return dispatch(createEvent(data, successMessage, failureMessage)); - }; - - showForm = () => { - const { dispatch, intl } = this.props; - return dispatch( - actions.showEventForm({ - onSubmit: this.createEventHandler, - formTitle: intl.formatMessage(translations.newEvent), - initialValues: { - title: '', - event_type: '', - location: '', - description: '', - start_at: null, - end_at: null, - published: false, - }, - }), - ); - }; +const initialValues: EventFormValues = { + title: '', + event_type: '', + location: '', + description: '', + start_at: null, + end_at: null, + published: false, +}; - render() { - if (!this.props.canManageLessonPlan) return null; +const NewEventButton = (): JSX.Element | null => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const canManageLessonPlan = useAppSelector( + (state) => state.lessonPlan.flags.canManageLessonPlan, + ); - const { intl } = this.props; + const [formVisible, setFormVisible] = useState(false); - return ( - - {intl.formatMessage(translations.newEvent)} - + const createEventHandler: FormSubmitHandler = ( + data, + setError, + ) => + dispatch( + createEvent( + data, + t(translations.success), + t(translations.failure), + setError, + ), ); - } -} -NewEventButton.propTypes = { - canManageLessonPlan: PropTypes.bool.isRequired, + if (!canManageLessonPlan) return null; + + return ( + <> + setFormVisible(true)}> + {t(translations.newEvent)} + - dispatch: PropTypes.func.isRequired, - intl: PropTypes.object.isRequired, + setFormVisible(false)} + onSubmit={createEventHandler} + open={formVisible} + /> + + ); }; -export default connect(({ lessonPlan }) => ({ - canManageLessonPlan: lessonPlan.flags.canManageLessonPlan, -}))(injectIntl(NewEventButton)); +export default NewEventButton; diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.tsx index 8bdf7b29d1b..328212cdd77 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/NewMilestoneButton.tsx @@ -1,12 +1,13 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage, injectIntl } from 'react-intl'; -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; +import { useState } from 'react'; +import { defineMessages } from 'react-intl'; import AddButton from 'lib/components/core/buttons/AddButton'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; import { createMilestone } from '../../operations'; -import { actions } from '../../store'; +import { FormSubmitHandler, MilestoneFormValues } from '../../types'; +import MilestoneFormDialog from '../MilestoneFormDialog'; const translations = defineMessages({ newMilestone: { @@ -23,47 +24,51 @@ const translations = defineMessages({ }, }); -class NewMilestoneButton extends Component { - createMilestoneHandler = (data, setError) => { - const { dispatch } = this.props; - const successMessage = ; - const failureMessage = ; - return dispatch( - createMilestone(data, successMessage, failureMessage, setError), - ); - }; - - showForm = () => { - const { dispatch, intl } = this.props; - return dispatch( - actions.showMilestoneForm({ - onSubmit: this.createMilestoneHandler, - formTitle: intl.formatMessage(translations.newMilestone), - initialValues: { title: '', description: '', start_at: null }, - }), - ); - }; +const initialValues: MilestoneFormValues = { + title: '', + description: '', + start_at: null, +}; - render() { - if (!this.props.canManageLessonPlan) return null; +const NewMilestoneButton = (): JSX.Element | null => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const canManageLessonPlan = useAppSelector( + (state) => state.lessonPlan.flags.canManageLessonPlan, + ); - const { intl } = this.props; + const [formVisible, setFormVisible] = useState(false); - return ( - - {intl.formatMessage(translations.newMilestone)} - + const createMilestoneHandler: FormSubmitHandler = ( + data, + setError, + ) => + dispatch( + createMilestone( + data, + t(translations.success), + t(translations.failure), + setError, + ), ); - } -} -NewMilestoneButton.propTypes = { - canManageLessonPlan: PropTypes.bool.isRequired, + if (!canManageLessonPlan) return null; + + return ( + <> + setFormVisible(true)}> + {t(translations.newMilestone)} + - dispatch: PropTypes.func.isRequired, - intl: PropTypes.object.isRequired, + setFormVisible(false)} + onSubmit={createMilestoneHandler} + open={formVisible} + /> + + ); }; -export default connect(({ lessonPlan }) => ({ - canManageLessonPlan: lessonPlan.flags.canManageLessonPlan, -}))(injectIntl(NewMilestoneButton)); +export default NewMilestoneButton; diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.tsx index 861387c7ffc..ed28deac43f 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewEventButton.test.tsx @@ -1,13 +1,16 @@ +import { AppState } from 'store'; import { fireEvent, render, waitFor } from 'test-utils'; import CourseAPI from 'api/course'; -import EventFormDialog from 'course/lesson-plan/containers/EventFormDialog'; import NewEventButton from '../NewEventButton'; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. + const state = { lessonPlan: { flags: { canManageLessonPlan: true } }, -}; +} as unknown as Partial; const startAt = '01-01-2017 12:12'; @@ -25,13 +28,7 @@ describe('', () => { it('allows event to be created via EventFormDialog', async () => { const spyCreate = jest.spyOn(CourseAPI.lessonPlan, 'createEvent'); - const page = render( - <> - - - , - { state }, - ); + const page = render(, { state }); fireEvent.click(await page.findByRole('button', { name: 'New Event' })); diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.tsx index 44cb7217f35..416cb77c8da 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/__test__/NewMilestoneButton.test.tsx @@ -1,7 +1,7 @@ +import { AppState } from 'store'; import { fireEvent, render, waitFor } from 'test-utils'; import CourseAPI from 'api/course'; -import MilestoneFormDialog from 'course/lesson-plan/containers/MilestoneFormDialog'; import NewMilestoneButton from '../NewMilestoneButton'; @@ -13,21 +13,18 @@ const milestoneData = { start_at: new Date(startAt), }; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. + const state = { lessonPlan: { flags: { canManageLessonPlan: true } }, -}; +} as unknown as Partial; describe('', () => { it('allows milestone to be created via MilestoneFormDialog', async () => { const spyCreate = jest.spyOn(CourseAPI.lessonPlan, 'createMilestone'); - const page = render( - <> - - - , - { state }, - ); + const page = render(, { state }); fireEvent.click(await page.findByRole('button', { name: 'New Milestone' })); diff --git a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.tsx b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.tsx index 390168beab7..207a6089fad 100644 --- a/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.tsx +++ b/client/app/bundles/course/lesson-plan/containers/LessonPlanLayout/index.tsx @@ -1,24 +1,20 @@ -import { Component } from 'react'; -import { FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; +import { useEffect } from 'react'; import { Outlet } from 'react-router-dom'; import { ListSubheader } from '@mui/material'; -import PropTypes from 'prop-types'; import LoadingIndicator from 'lib/components/core/LoadingIndicator'; import DeleteConfirmation from 'lib/containers/DeleteConfirmation'; -import { lessonPlanTypesGroups } from 'lib/types'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; import { fetchLessonPlan } from '../../operations'; import translations from '../../translations'; -import EventFormDialog from '../EventFormDialog'; import LessonPlanFilter from '../LessonPlanFilter'; import LessonPlanNav from '../LessonPlanNav'; -import MilestoneFormDialog from '../MilestoneFormDialog'; const styles = { tools: { - position: 'fixed', + position: 'fixed' as const, bottom: 12, right: 24, display: 'flex', @@ -31,54 +27,37 @@ const styles = { }, }; -class LessonPlanLayout extends Component { - componentDidMount() { - const { dispatch } = this.props; - dispatch(fetchLessonPlan()); - } +const LessonPlanLayout = (): JSX.Element => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); - render() { - const { isLoading, groups } = this.props; + const { isLoading, groups } = useAppSelector( + (state) => state.lessonPlan.lessonPlan, + ); - if (isLoading) return ; + useEffect(() => { + dispatch(fetchLessonPlan()); + }, []); - if (!groups || groups.length < 1) - return ( - - - - ); + if (isLoading) return ; - return ( -
- + if (!groups || groups.length < 1) + return {t(translations.empty)}; -
- - -
+ return ( +
+ - - - +
+ +
- ); - } -} -LessonPlanLayout.propTypes = { - isLoading: PropTypes.bool.isRequired, - groups: lessonPlanTypesGroups.isRequired, - dispatch: PropTypes.func.isRequired, - children: PropTypes.node.isRequired, + +
+ ); }; const handle = translations.lessonPlan; -export default Object.assign( - connect(({ lessonPlan }) => ({ - isLoading: lessonPlan.lessonPlan.isLoading, - groups: lessonPlan.lessonPlan.groups, - }))(LessonPlanLayout), - { handle }, -); +export default Object.assign(LessonPlanLayout, { handle }); diff --git a/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/MilestoneForm.jsx b/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/MilestoneForm.jsx index 7a48a2e3ac3..1ea723d72ba 100644 --- a/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/MilestoneForm.jsx +++ b/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/MilestoneForm.jsx @@ -11,11 +11,8 @@ import FormRichTextField from 'lib/components/form/fields/RichTextField'; import FormTextField from 'lib/components/form/fields/TextField'; import formTranslations from 'lib/translations/form'; -import { fields } from '../../constants'; import translations from '../../translations'; -const { TITLE, DESCRIPTION, START_AT } = fields; - const validationSchema = yup.object({ title: yup.string().required(formTranslations.required), description: yup.string().nullable(), @@ -62,7 +59,7 @@ const MilestoneForm = (props) => { InputLabelProps={{ shrink: true, }} - label={} + label={} required variant="standard" /> @@ -80,7 +77,7 @@ const MilestoneForm = (props) => { InputLabelProps={{ shrink: true, }} - label={} + label={} multiline rows={2} variant="standard" @@ -95,7 +92,7 @@ const MilestoneForm = (props) => { disabled={disabled} field={field} fieldState={fieldState} - label={} + label={} /> )} /> diff --git a/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.tsx b/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.tsx index 6c55e855290..5f814702f0b 100644 --- a/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.tsx +++ b/client/app/bundles/course/lesson-plan/containers/MilestoneFormDialog/index.tsx @@ -1,61 +1,61 @@ import { useState } from 'react'; -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; import FormDialogue from 'lib/components/form/FormDialogue'; -import { actions } from '../../store'; +import { FormSubmitHandler, MilestoneFormValues } from '../../types'; import MilestoneForm from './MilestoneForm'; -const MilestoneFormDialog = ({ - visible, - disabled, - formTitle, - initialValues, - onSubmit, - dispatch, -}) => { +interface MilestoneFormDialogProps { + open: boolean; + onClose: () => void; + formTitle?: string; + initialValues: MilestoneFormValues; + onSubmit: FormSubmitHandler; +} + +/** + * Controlled by whoever opens it: the owner supplies the handler and the initial + * values, and the dialog closes itself once `onSubmit` reports success. The + * handler used to be stashed in the Redux store, which is not serialisable. + */ +const MilestoneFormDialog = (props: MilestoneFormDialogProps): JSX.Element => { + const { open, onClose, formTitle, initialValues, onSubmit } = props; + const [isDirty, setIsDirty] = useState(false); + const [submitting, setSubmitting] = useState(false); + + const handleSubmit: FormSubmitHandler = async ( + data, + setError, + ) => { + setSubmitting(true); + try { + const succeeded = await onSubmit(data, setError); + if (succeeded) onClose(); + return succeeded; + } finally { + setSubmitting(false); + } + }; return ( dispatch(actions.hideMilestoneForm())} - open={visible} + hideForm={onClose} + open={open} skipConfirmation={!isDirty} title={formTitle} > ); }; -MilestoneFormDialog.defaultProps = { - visible: false, - disabled: false, -}; - -MilestoneFormDialog.propTypes = { - visible: PropTypes.bool, - disabled: PropTypes.bool, - formTitle: PropTypes.string, - initialValues: PropTypes.shape({ - title: PropTypes.string, - description: PropTypes.string, - start_at: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.instanceOf(Date), - ]), - }), - onSubmit: PropTypes.func.isRequired, - dispatch: PropTypes.func.isRequired, -}; - -export default connect(({ lessonPlan }) => ({ - ...lessonPlan.milestoneForm, -}))(MilestoneFormDialog); +export default MilestoneFormDialog; diff --git a/client/app/bundles/course/lesson-plan/operations.ts b/client/app/bundles/course/lesson-plan/operations.ts index dfa68ff4587..7cd9dbfc501 100644 --- a/client/app/bundles/course/lesson-plan/operations.ts +++ b/client/app/bundles/course/lesson-plan/operations.ts @@ -5,7 +5,6 @@ import { setNotification } from 'lib/actions'; import { setReactHookFormError } from 'lib/helpers/react-hook-form-helper'; import actionTypes from './constants'; -import { actions } from './store'; export function fetchLessonPlan(): Operation { return async (dispatch) => { @@ -32,7 +31,7 @@ export function createMilestone( successMessage, failureMessage, setError, -): Operation { +): Operation { return async (dispatch) => { dispatch({ type: actionTypes.MILESTONE_CREATE_REQUEST }); return CourseAPI.lessonPlan @@ -42,8 +41,8 @@ export function createMilestone( type: actionTypes.MILESTONE_CREATE_SUCCESS, milestone: response.data, }); - dispatch(actions.hideMilestoneForm()); setNotification(successMessage)(dispatch); + return true; }) .catch((error) => { dispatch({ type: actionTypes.MILESTONE_CREATE_FAILURE }); @@ -51,6 +50,7 @@ export function createMilestone( if (error?.response?.data?.errors) { setReactHookFormError(setError, error.response.data.errors); } + return false; }); }; } @@ -73,6 +73,7 @@ export function updateMilestone(id, values, setError): Operation { if (error?.response?.data?.errors && setError) { setReactHookFormError(setError, error.response.data.errors); } + return false; }); }; } @@ -115,7 +116,7 @@ export function createEvent( successMessage, failureMessage, setError, -): Operation { +): Operation { return async (dispatch) => { dispatch({ type: actionTypes.EVENT_CREATE_REQUEST }); return CourseAPI.lessonPlan @@ -125,8 +126,8 @@ export function createEvent( type: actionTypes.EVENT_CREATE_SUCCESS, event: response.data, }); - dispatch(actions.hideEventForm()); setNotification(successMessage)(dispatch); + return true; }) .catch((error) => { dispatch({ type: actionTypes.EVENT_CREATE_FAILURE }); @@ -134,6 +135,7 @@ export function createEvent( if (error?.response?.data?.errors) { setReactHookFormError(setError, error.response.data.errors); } + return false; }); }; } @@ -144,7 +146,7 @@ export function updateEvent( successMessage, failureMessage, setError, -): Operation { +): Operation { return async (dispatch) => { dispatch({ type: actionTypes.EVENT_UPDATE_REQUEST }); return CourseAPI.lessonPlan @@ -155,8 +157,8 @@ export function updateEvent( eventId, event: response.data, }); - dispatch(actions.hideEventForm()); setNotification(successMessage)(dispatch); + return true; }) .catch((error) => { dispatch({ type: actionTypes.EVENT_UPDATE_FAILURE }); @@ -164,6 +166,7 @@ export function updateEvent( if (error?.response?.data?.errors) { setReactHookFormError(setError, error.response.data.errors); } + return false; }); }; } diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx index 8a5b77261cc..0a66b0c6c84 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanEdit/ItemRow/index.tsx @@ -1,6 +1,7 @@ -import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { defineMessages } from 'react-intl'; +import { setNotification } from 'lib/actions'; import Link from 'lib/components/core/Link'; import LoadingIndicator from 'lib/components/core/LoadingIndicator'; import { FIELD_LONG_DEBOUNCE_DELAY_MS } from 'lib/constants/sharedConstants'; @@ -70,20 +71,46 @@ const ItemRow = (props: ItemRowProps): JSX.Element | null => { (state) => state.lessonPlan.flags.editPageColumnsVisible, ); - // Changes queued but not yet sent. Merged so that editing several fields of the - // same row results in one request rather than one per field. + // Edits the user has made that have not been sent yet. const pendingRef = useRef({}); - // The latest value we know for each field, whether or not it has been saved yet. - // Guards against re-sending a value the user has already queued. - const latestValuesRef = useRef({}); // At most one request per row may be in flight. const inFlightRef = useRef(false); const [saving, setSaving] = useState(false); + // Bumped to remount the pickers after a failed save, to put the saved values + // back on screen: the store never changed, so nothing else would. + const [revision, setRevision] = useState(0); + + const savedValues: Record = { + start_at: startAt as ItemValue, + bonus_end_at: bonusEndAt as ItemValue, + end_at: endAt as ItemValue, + published, + }; + + /** The subset of `payload` that actually differs from what is saved. */ + const changedFromSaved = ( + payload: LessonPlanItemUpdate, + ): LessonPlanItemUpdate => + Object.entries(payload).reduce( + (acc, [field, value]) => { + const key = field as LessonPlanItemUpdateField; + if (sameValue(savedValues[key], value)) return acc; + return { ...acc, [key]: value }; + }, + {}, + ); + + // Mirrored so the debounced call always runs the newest closure: it is + // created once, and needs the props as they are when it fires rather than + // when it was made. + const flushRef = useRef<() => void>(() => {}); - const flush = (context: SaveContext): void => { + const flush = (): void => { if (inFlightRef.current) return; - const payload = pendingRef.current; + // Diffed here rather than when queued: an in-flight request may have moved + // the saved state on since, and after a failure it will not have. + const payload = changedFromSaved(pendingRef.current); pendingRef.current = {}; if (Object.keys(payload).length === 0) { @@ -91,46 +118,44 @@ const ItemRow = (props: ItemRowProps): JSX.Element | null => { return; } - const successMessage = context.t(translations.updateSuccess, { - title: context.title, - }); - const failureMessage = context.t(translations.updateFailed, { - title: context.title, - }); - inFlightRef.current = true; - context - .dispatch(updateItem(context.id, payload, successMessage, failureMessage)) - .finally(() => { - inFlightRef.current = false; - if (Object.keys(pendingRef.current).length > 0) { - flush(context); - } else { - setSaving(false); - } - }); + dispatch(updateItem(id, payload)).then((succeeded) => { + inFlightRef.current = false; + + // A newer edit is already queued, so let it give the verdict instead — + // otherwise the user gets a failure followed by a success for one action. + const superseded = Object.keys(pendingRef.current).length > 0; + + if (succeeded) { + dispatch(setNotification(t(translations.updateSuccess, { title }))); + } else if (!superseded) { + setRevision((current) => current + 1); + dispatch(setNotification(t(translations.updateFailed, { title }))); + } + + if (superseded) { + flushRef.current(); + } else { + setSaving(false); + } + }); }; - const debouncedFlush = useDebounce(flush, FIELD_LONG_DEBOUNCE_DELAY_MS, []); + useEffect(() => { + flushRef.current = flush; + }); - const queueUpdate = (payload: LessonPlanItemUpdate): void => { - const savedValues: Record = { - start_at: startAt, - bonus_end_at: bonusEndAt, - end_at: endAt, - published, - }; + const debouncedFlush = useDebounce( + () => flushRef.current(), + FIELD_LONG_DEBOUNCE_DELAY_MS, + [], + ); + const queueUpdate = (payload: LessonPlanItemUpdate): void => { const changes = Object.entries(payload).reduce( (acc, [field, value]) => { const key = field as LessonPlanItemUpdateField; if (REQUIRED_FIELDS.includes(key) && !value) return acc; - - const latest = - key in latestValuesRef.current - ? latestValuesRef.current[key] - : (savedValues[key] as ItemValue); - if (sameValue(latest, value)) return acc; return { ...acc, [key]: value }; }, {}, @@ -138,10 +163,16 @@ const ItemRow = (props: ItemRowProps): JSX.Element | null => { if (Object.keys(changes).length === 0) return; - latestValuesRef.current = { ...latestValuesRef.current, ...changes }; + // With nothing queued or in flight the saved values are current, so a no-op + // can be dropped now rather than showing a spinner until the flush. While + // either is outstanding the saved values may still move, so the flush decides. + const settled = + !inFlightRef.current && !Object.keys(pendingRef.current).length; + if (settled && Object.keys(changedFromSaved(changes)).length === 0) return; + pendingRef.current = { ...pendingRef.current, ...changes }; setSaving(true); - debouncedFlush({ id, title, dispatch, t }); + debouncedFlush(); }; const updatePublished = (_, isToggled: boolean): void => @@ -158,7 +189,7 @@ const ItemRow = (props: ItemRowProps): JSX.Element | null => { return ( - {columnsVisible['ITEM_TYPE'] ? ( + {columnsVisible.ITEM_TYPE ? ( @@ -169,20 +200,31 @@ const ItemRow = (props: ItemRowProps): JSX.Element | null => { {saving ? : null}
- {columnsVisible['START_AT'] ? ( - + {columnsVisible.START_AT ? ( + ) : null} - {columnsVisible['BONUS_END_AT'] ? ( + {columnsVisible.BONUS_END_AT ? ( ) : null} - {columnsVisible['END_AT'] ? ( - + {columnsVisible.END_AT ? ( + ) : null} - {columnsVisible['PUBLISHED'] ? ( + {columnsVisible.PUBLISHED ? ( { - const { - dispatch, - intl, - item: { id, eventId }, - } = this.props; - const successMessage = intl.formatMessage(translations.deleteSuccess); - const failureMessage = intl.formatMessage(translations.deleteFailure); - const handleDelete = () => - dispatch(deleteEvent(id, eventId, successMessage, failureMessage)); - return dispatch(showDeleteConfirmation(handleDelete)); - }; +interface AdminToolsProps { + item: LessonPlanEventItem; +} - showEditEventDialog = () => { - const { dispatch, intl, item } = this.props; - const { - title, - lesson_plan_item_type, - location, - description, - start_at, - end_at, - published, - } = item; - - return dispatch( - actions.showEventForm({ - onSubmit: this.updateEventHandler, - formTitle: intl.formatMessage(translations.editEvent), - initialValues: { - title, - location, - description, - start_at, - end_at, - published, - event_type: lesson_plan_item_type[0], - }, - }), - ); - }; +const AdminTools = (props: AdminToolsProps): JSX.Element | null => { + const { item } = props; + + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const canManageLessonPlan = useAppSelector( + (state) => state.lessonPlan.flags.canManageLessonPlan, + ); + + const [formVisible, setFormVisible] = useState(false); - updateEventHandler = (data) => { - const { - dispatch, - intl, - item: { eventId }, - } = this.props; - const successMessage = intl.formatMessage(translations.updateSuccess); - const failureMessage = intl.formatMessage(translations.updateFailure); - return dispatch(updateEvent(eventId, data, successMessage, failureMessage)); + const deleteEventHandler = (): void => { + const handleDelete = (): Promise => + dispatch( + deleteEvent( + item.id, + item.eventId, + t(translations.deleteSuccess), + t(translations.deleteFailure), + ), + ); + + dispatch(showDeleteConfirmation(handleDelete)); }; - render() { - const { - item: { eventId }, - canManageLessonPlan, - } = this.props; - if (!canManageLessonPlan || eventId === undefined) { - return null; - } - - return ( - - - - - - - - - + const updateEventHandler: FormSubmitHandler = ( + data, + setError, + ) => + dispatch( + updateEvent( + item.eventId, + data, + t(translations.updateSuccess), + t(translations.updateFailure), + setError, + ), ); - } -} -AdminTools.propTypes = { - item: PropTypes.shape({ - id: PropTypes.number, - eventId: PropTypes.number, - title: PropTypes.string, - published: PropTypes.bool, - location: PropTypes.string, - description: PropTypes.string, - start_at: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.instanceOf(Date), - ]), - end_at: PropTypes.oneOfType([PropTypes.string, PropTypes.instanceOf(Date)]), - lesson_plan_item_type: PropTypes.arrayOf(PropTypes.string), - }).isRequired, - canManageLessonPlan: PropTypes.bool.isRequired, - - dispatch: PropTypes.func.isRequired, - intl: PropTypes.object.isRequired, + if (!canManageLessonPlan || item.eventId === undefined) return null; + + const { + title, + lesson_plan_item_type: itemType, + location, + description, + start_at: startAt, + end_at: endAt, + published, + } = item; + + return ( + + setFormVisible(true)}> + + + + + + + + setFormVisible(false)} + onSubmit={updateEventHandler} + open={formVisible} + /> + + ); }; -export default connect(({ lessonPlan }) => ({ - canManageLessonPlan: lessonPlan.flags.canManageLessonPlan, -}))(injectIntl(AdminTools)); +export default AdminTools; diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.tsx index 97ac7d9cfe9..1afbf05d86a 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/LessonPlanItem/__test__/AdminTools.test.tsx @@ -1,16 +1,21 @@ -import { fireEvent, render, waitFor } from 'test-utils'; +import { AppState } from 'store'; +import { fireEvent, render, RenderResult, waitFor } from 'test-utils'; import CourseAPI from 'api/course'; -import EventFormDialog from 'course/lesson-plan/containers/EventFormDialog'; import DeleteConfirmation from 'lib/containers/DeleteConfirmation'; +import { LessonPlanEventItem } from '../../../../types'; import AdminTools from '../AdminTools'; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. + const state = { lessonPlan: { flags: { canManageLessonPlan: true } }, -}; +} as unknown as Partial; -const renderElement = (item) => render(, { state }); +const renderElement = (item: LessonPlanEventItem): RenderResult => + render(, { state }); describe('', () => { it('does not show admin menu for lesson plan events', async () => { @@ -57,20 +62,16 @@ describe('', () => { }; const page = render( - <> - - - - , + , { state }, ); diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.tsx index e4fc28a6ab1..c365e042ed1 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/MilestoneAdminTools.tsx @@ -1,16 +1,20 @@ -/* eslint-disable camelcase */ -import { PureComponent } from 'react'; -import { defineMessages, injectIntl } from 'react-intl'; -import { connect } from 'react-redux'; +import { useState } from 'react'; +import { defineMessages } from 'react-intl'; import Delete from '@mui/icons-material/Delete'; import Edit from '@mui/icons-material/Edit'; import { IconButton } from '@mui/material'; -import PropTypes from 'prop-types'; -import { showDeleteConfirmation } from 'lib/actions'; +import { setNotification, showDeleteConfirmation } from 'lib/actions'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import useTranslation from 'lib/hooks/useTranslation'; +import MilestoneFormDialog from '../../containers/MilestoneFormDialog'; import { deleteMilestone, updateMilestone } from '../../operations'; -import { actions } from '../../store'; +import { + FormSubmitHandler, + MilestoneFormValues, + MilestoneOrPlaceholder, +} from '../../types'; const translations = defineMessages({ editMilestone: { @@ -35,89 +39,87 @@ const translations = defineMessages({ }, }); -class MilestoneAdminTools extends PureComponent { - deleteMilestoneHandler = () => { - const { - dispatch, - intl, - milestone: { id }, - } = this.props; - const successMessage = intl.formatMessage(translations.deleteSuccess); - const failureMessage = intl.formatMessage(translations.deleteFailure); - const handleDelete = () => - dispatch(deleteMilestone(id, successMessage, failureMessage)); - return dispatch(showDeleteConfirmation(handleDelete)); +interface MilestoneAdminToolsProps { + milestone: MilestoneOrPlaceholder; +} + +const MilestoneAdminTools = ( + props: MilestoneAdminToolsProps, +): JSX.Element | null => { + const { milestone } = props; + + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const canManageLessonPlan = useAppSelector( + (state) => state.lessonPlan.flags.canManageLessonPlan, + ); + + const [formVisible, setFormVisible] = useState(false); + + const deleteMilestoneHandler = (): void => { + const handleDelete = (): Promise => + dispatch( + deleteMilestone( + milestone.id, + t(translations.deleteSuccess), + t(translations.deleteFailure), + ), + ); + + dispatch(showDeleteConfirmation(handleDelete)); }; - showEditMilestoneDialog = () => { - const { - dispatch, - intl, - milestone: { title, description, start_at }, - } = this.props; - - return dispatch( - actions.showMilestoneForm({ - onSubmit: this.updateMilestoneHandler, - formTitle: intl.formatMessage(translations.editMilestone), - initialValues: { title, description, start_at }, - }), + const updateMilestoneHandler: FormSubmitHandler = async ( + data, + setError, + ) => { + // `updateMilestone` reports the outcome and leaves the message to us; see + // the operation for why. + const succeeded = await dispatch( + updateMilestone(milestone.id, data, setError), ); - }; - updateMilestoneHandler = (data, setError) => { - const { - dispatch, - intl, - milestone: { id }, - } = this.props; - - const successMessage = intl.formatMessage(translations.updateSuccess); - const failureMessage = intl.formatMessage(translations.updateFailure); - return dispatch( - updateMilestone(id, data, successMessage, failureMessage, setError), + dispatch( + setNotification( + succeeded + ? t(translations.updateSuccess) + : t(translations.updateFailure), + ), ); + + return succeeded; }; - render() { - const { milestone, canManageLessonPlan } = this.props; - if (!milestone.id || !canManageLessonPlan) { - return null; - } - - return ( - - - - - - - - - - ); - } -} + if (!milestone.id || !canManageLessonPlan) return null; + + const { title, description, start_at: startAt } = milestone; + // Only the synthesised milestone carries a node title, and it is filtered out + // by the guard above, so anything reaching the form is a plain string. + const editableTitle = typeof title === 'string' ? title : undefined; + + return ( + + setFormVisible(true)}> + + + + + + -MilestoneAdminTools.propTypes = { - milestone: PropTypes.shape({ - id: PropTypes.number, - description: PropTypes.string, - start_at: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.instanceOf(Date), - ]), - title: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.node, // Allow node containing translation - ]), - }), - canManageLessonPlan: PropTypes.bool, - - dispatch: PropTypes.func.isRequired, - intl: PropTypes.object.isRequired, + setFormVisible(false)} + onSubmit={updateMilestoneHandler} + open={formVisible} + /> + + ); }; -export default connect(({ lessonPlan }) => ({ - canManageLessonPlan: lessonPlan.flags.canManageLessonPlan, -}))(injectIntl(MilestoneAdminTools)); +export default MilestoneAdminTools; diff --git a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.tsx b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.tsx index 3a070983da1..2cb065b2608 100644 --- a/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.tsx +++ b/client/app/bundles/course/lesson-plan/pages/LessonPlanShow/__test__/MilestoneAdminTools.test.tsx @@ -1,15 +1,26 @@ -import { fireEvent, render, waitFor } from 'test-utils'; +import { AppState } from 'store'; +import { fireEvent, render, RenderResult, waitFor } from 'test-utils'; import CourseAPI from 'api/course'; -import MilestoneFormDialog from 'course/lesson-plan/containers/MilestoneFormDialog'; import DeleteConfirmation from 'lib/containers/DeleteConfirmation'; +import { MilestoneOrPlaceholder } from '../../../types'; import MilestoneAdminTools from '../MilestoneAdminTools'; -const renderElement = (canManageLessonPlan, milestone) => { - const state = { lessonPlan: { flags: { canManageLessonPlan } } }; - return render(, { state }); -}; +// `Partial` only allows omitting whole slices, and these tests seed +// just the few fields the component reads, so the shape is asserted. +const stateWith = (canManageLessonPlan: boolean): Partial => + ({ + lessonPlan: { flags: { canManageLessonPlan } }, + }) as unknown as Partial; + +const renderElement = ( + canManageLessonPlan: boolean, + milestone: MilestoneOrPlaceholder, +): RenderResult => + render(, { + state: stateWith(canManageLessonPlan), + }); describe('', () => { it('hides admin tools for dummy milestone', async () => { @@ -59,7 +70,7 @@ describe('', () => { }} /> , - { state: { lessonPlan: { flags: { canManageLessonPlan: true } } } }, + { state: stateWith(true) }, ); fireEvent.click((await page.findAllByRole('button'))[1]); @@ -84,17 +95,14 @@ describe('', () => { const spy = jest.spyOn(CourseAPI.lessonPlan, 'updateMilestone'); const page = render( - <> - - - , - { state: { lessonPlan: { flags: { canManageLessonPlan: true } } } }, + , + { state: stateWith(true) }, ); fireEvent.click((await page.findAllByRole('button'))[0]); diff --git a/client/app/bundles/course/lesson-plan/reducers/eventForm.js b/client/app/bundles/course/lesson-plan/reducers/eventForm.js deleted file mode 100644 index f7c2b1dfe47..00000000000 --- a/client/app/bundles/course/lesson-plan/reducers/eventForm.js +++ /dev/null @@ -1,33 +0,0 @@ -import actionTypes from '../constants'; - -export const initialState = { - visible: false, - disabled: false, - onSubmit: () => {}, - formTitle: '', - initialValues: {}, -}; - -export default function (state = initialState, action) { - const { type } = action; - switch (type) { - case actionTypes.EVENT_FORM_SHOW: { - return { ...state, ...action.formParams, visible: true }; - } - case actionTypes.EVENT_FORM_HIDE: { - return { ...state, visible: false }; - } - case actionTypes.EVENT_UPDATE_REQUEST: - case actionTypes.EVENT_CREATE_REQUEST: { - return { ...state, disabled: true }; - } - case actionTypes.EVENT_UPDATE_SUCCESS: - case actionTypes.EVENT_UPDATE_FAILURE: - case actionTypes.EVENT_CREATE_SUCCESS: - case actionTypes.EVENT_CREATE_FAILURE: { - return { ...state, disabled: false }; - } - default: - return state; - } -} diff --git a/client/app/bundles/course/lesson-plan/reducers/milestoneForm.js b/client/app/bundles/course/lesson-plan/reducers/milestoneForm.js deleted file mode 100644 index 0e8caef501c..00000000000 --- a/client/app/bundles/course/lesson-plan/reducers/milestoneForm.js +++ /dev/null @@ -1,33 +0,0 @@ -import actionTypes from '../constants'; - -export const initialState = { - visible: false, - disabled: false, - onSubmit: () => {}, - formTitle: '', - initialValues: {}, -}; - -export default function (state = initialState, action) { - const { type } = action; - switch (type) { - case actionTypes.MILESTONE_FORM_SHOW: { - return { ...state, ...action.formParams, visible: true }; - } - case actionTypes.MILESTONE_FORM_HIDE: { - return { ...state, visible: false }; - } - case actionTypes.MILESTONE_UPDATE_REQUEST: - case actionTypes.MILESTONE_CREATE_REQUEST: { - return { ...state, disabled: true }; - } - case actionTypes.MILESTONE_UPDATE_SUCCESS: - case actionTypes.MILESTONE_UPDATE_FAILURE: - case actionTypes.MILESTONE_CREATE_SUCCESS: - case actionTypes.MILESTONE_CREATE_FAILURE: { - return { ...state, disabled: false }; - } - default: - return state; - } -} diff --git a/client/app/bundles/course/lesson-plan/store.ts b/client/app/bundles/course/lesson-plan/store.ts index 2558d862979..c579dc7c6cc 100644 --- a/client/app/bundles/course/lesson-plan/store.ts +++ b/client/app/bundles/course/lesson-plan/store.ts @@ -1,17 +1,13 @@ /* eslint-disable @typescript-eslint/explicit-function-return-type */ import { combineReducers } from 'redux'; -import eventFormReducer from './reducers/eventForm'; import flagsReducer from './reducers/flags'; import lessonPlanReducer from './reducers/lessonPlan'; -import milestoneFormReducer from './reducers/milestoneForm'; import actionTypes from './constants'; const reducer = combineReducers({ flags: flagsReducer, lessonPlan: lessonPlanReducer, - eventForm: eventFormReducer, - milestoneForm: milestoneFormReducer, }); export const actions = { @@ -25,16 +21,6 @@ export const actions = { field, isVisible, }), - showMilestoneForm: (formParams) => ({ - type: actionTypes.MILESTONE_FORM_SHOW, - formParams, - }), - hideMilestoneForm: () => ({ type: actionTypes.MILESTONE_FORM_HIDE }), - showEventForm: (formParams) => ({ - type: actionTypes.EVENT_FORM_SHOW, - formParams, - }), - hideEventForm: () => ({ type: actionTypes.EVENT_FORM_HIDE }), }; export default reducer; diff --git a/client/app/bundles/course/lesson-plan/translations.ts b/client/app/bundles/course/lesson-plan/translations.ts index 8d557f8fff7..26837b7986f 100644 --- a/client/app/bundles/course/lesson-plan/translations.ts +++ b/client/app/bundles/course/lesson-plan/translations.ts @@ -1,53 +1,39 @@ import { defineMessages } from 'react-intl'; -import { fields } from './constants'; - -const { - ITEM_TYPE, - TITLE, - START_AT, - BONUS_END_AT, - END_AT, - PUBLISHED, - LOCATION, - DESCRIPTION, - EVENT_TYPE, -} = fields; - const translations = defineMessages({ - [ITEM_TYPE]: { + ITEM_TYPE: { id: 'course.lessonPlan.itemType', defaultMessage: 'Type', }, - [EVENT_TYPE]: { + eventType: { id: 'course.lessonPlan.eventType', defaultMessage: 'Event Type', }, - [TITLE]: { + title: { id: 'course.lessonPlan.title', defaultMessage: 'Title', }, - [DESCRIPTION]: { + description: { id: 'course.lessonPlan.description', defaultMessage: 'Description', }, - [LOCATION]: { + location: { id: 'course.lessonPlan.location', defaultMessage: 'Location', }, - [START_AT]: { + START_AT: { id: 'course.lessonPlan.startAt', defaultMessage: 'Start At *', }, - [BONUS_END_AT]: { + BONUS_END_AT: { id: 'course.lessonPlan.bonusEndAt', defaultMessage: 'Bonus End At', }, - [END_AT]: { + END_AT: { id: 'course.lessonPlan.endAt', defaultMessage: 'End At', }, - [PUBLISHED]: { + PUBLISHED: { id: 'course.lessonPlan.published', defaultMessage: 'Published', }, diff --git a/client/app/routers/course/lessonPlan.tsx b/client/app/routers/course/lessonPlan.tsx index 4778f3f96fd..ca61d3f72e5 100644 --- a/client/app/routers/course/lessonPlan.tsx +++ b/client/app/routers/course/lessonPlan.tsx @@ -14,7 +14,6 @@ const lessonPlanRouter: Translated = (_) => ({ ).default; return { - // @ts-ignore `connect` throws error when cannot find `store` as direct parent element: , handle: LessonPlanLayout.handle, }; From fd8b0d056a2f73ea91e19360cddb6e20ce81614c Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Thu, 27 Aug 2026 16:38:00 +0800 Subject: [PATCH 4/4] refactor(lesson-plan): fully convert lesson plan Redux state to TypeScript --- .../bundles/course/lesson-plan/constants.ts | 46 ---- .../bundles/course/lesson-plan/operations.ts | 52 +--- .../course/lesson-plan/reducers/flags.ts | 71 +++-- .../course/lesson-plan/reducers/lessonPlan.ts | 251 +++++++++++------- .../course/lesson-plan/reducers/utils.ts | 89 ++++--- .../app/bundles/course/lesson-plan/store.ts | 19 +- .../app/bundles/course/lesson-plan/types.ts | 55 ++-- 7 files changed, 283 insertions(+), 300 deletions(-) delete mode 100644 client/app/bundles/course/lesson-plan/constants.ts diff --git a/client/app/bundles/course/lesson-plan/constants.ts b/client/app/bundles/course/lesson-plan/constants.ts deleted file mode 100644 index 08eaf3290d1..00000000000 --- a/client/app/bundles/course/lesson-plan/constants.ts +++ /dev/null @@ -1,46 +0,0 @@ -import mirrorCreator from 'mirror-creator'; - -export const formNames = mirrorCreator(['EVENT', 'MILESTONE']); - -export const fields = mirrorCreator([ - 'ITEM_TYPE', - 'TITLE', - 'START_AT', - 'BONUS_END_AT', - 'END_AT', - 'PUBLISHED', - 'LOCATION', - 'DESCRIPTION', - 'EVENT_TYPE', -]); - -const actionTypes = mirrorCreator([ - 'SET_ITEM_TYPE_VISIBILITY', - 'SET_COLUMN_VISIBILITY', - 'LOAD_LESSON_PLAN_REQUEST', - 'LOAD_LESSON_PLAN_SUCCESS', - 'LOAD_LESSON_PLAN_FAILURE', - 'ITEM_UPDATE_REQUEST', - 'ITEM_UPDATE_SUCCESS', - 'ITEM_UPDATE_FAILURE', - 'EVENT_UPDATE_REQUEST', - 'EVENT_UPDATE_SUCCESS', - 'EVENT_UPDATE_FAILURE', - 'EVENT_CREATE_REQUEST', - 'EVENT_CREATE_SUCCESS', - 'EVENT_CREATE_FAILURE', - 'EVENT_DELETE_REQUEST', - 'EVENT_DELETE_SUCCESS', - 'EVENT_DELETE_FAILURE', - 'MILESTONE_UPDATE_REQUEST', - 'MILESTONE_UPDATE_SUCCESS', - 'MILESTONE_UPDATE_FAILURE', - 'MILESTONE_CREATE_REQUEST', - 'MILESTONE_CREATE_SUCCESS', - 'MILESTONE_CREATE_FAILURE', - 'MILESTONE_DELETE_REQUEST', - 'MILESTONE_DELETE_SUCCESS', - 'MILESTONE_DELETE_FAILURE', -]); - -export default actionTypes; diff --git a/client/app/bundles/course/lesson-plan/operations.ts b/client/app/bundles/course/lesson-plan/operations.ts index 7cd9dbfc501..bb1d08564cc 100644 --- a/client/app/bundles/course/lesson-plan/operations.ts +++ b/client/app/bundles/course/lesson-plan/operations.ts @@ -4,24 +4,18 @@ import CourseAPI from 'api/course'; import { setNotification } from 'lib/actions'; import { setReactHookFormError } from 'lib/helpers/react-hook-form-helper'; -import actionTypes from './constants'; +import { actions } from './store'; export function fetchLessonPlan(): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.LOAD_LESSON_PLAN_REQUEST }); + dispatch(actions.loadRequested()); return CourseAPI.lessonPlan .fetch() .then((response) => { - dispatch({ - type: actionTypes.LOAD_LESSON_PLAN_SUCCESS, - items: response.data.items, - milestones: response.data.milestones, - flags: response.data.flags, - visibilitySettings: response.data.visibilitySettings, - }); + dispatch(actions.loadSucceeded(response.data)); }) .catch(() => { - dispatch({ type: actionTypes.LOAD_LESSON_PLAN_FAILURE }); + dispatch(actions.loadFailed()); }); }; } @@ -33,19 +27,14 @@ export function createMilestone( setError, ): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.MILESTONE_CREATE_REQUEST }); return CourseAPI.lessonPlan .createMilestone({ lesson_plan_milestone: values }) .then((response) => { - dispatch({ - type: actionTypes.MILESTONE_CREATE_SUCCESS, - milestone: response.data, - }); + dispatch(actions.milestoneCreated(response.data)); setNotification(successMessage)(dispatch); return true; }) .catch((error) => { - dispatch({ type: actionTypes.MILESTONE_CREATE_FAILURE }); setNotification(failureMessage)(dispatch); if (error?.response?.data?.errors) { setReactHookFormError(setError, error.response.data.errors); @@ -62,7 +51,6 @@ export function createMilestone( */ export function updateMilestone(id, values, setError): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.MILESTONE_UPDATE_REQUEST }); return CourseAPI.lessonPlan .updateMilestone(id, { lesson_plan_milestone: values }) .then((response) => { @@ -80,18 +68,13 @@ export function updateMilestone(id, values, setError): Operation { export function deleteMilestone(id, successMessage, failureMessage): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.MILESTONE_DELETE_REQUEST }); return CourseAPI.lessonPlan .deleteMilestone(id) .then(() => { - dispatch({ - type: actionTypes.MILESTONE_DELETE_SUCCESS, - milestoneId: id, - }); + dispatch(actions.milestoneDeleted(id)); setNotification(successMessage)(dispatch); }) .catch(() => { - dispatch({ type: actionTypes.MILESTONE_DELETE_FAILURE }); setNotification(failureMessage)(dispatch); }); }; @@ -100,7 +83,6 @@ export function deleteMilestone(id, successMessage, failureMessage): Operation { /** See `updateMilestone` for why the notification is the caller's. */ export function updateItem(id, values): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.ITEM_UPDATE_REQUEST }); return CourseAPI.lessonPlan .updateItem(id, { item: values }) .then(() => { @@ -118,19 +100,14 @@ export function createEvent( setError, ): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.EVENT_CREATE_REQUEST }); return CourseAPI.lessonPlan .createEvent({ lesson_plan_event: values }) .then((response) => { - dispatch({ - type: actionTypes.EVENT_CREATE_SUCCESS, - event: response.data, - }); + dispatch(actions.eventCreated(response.data)); setNotification(successMessage)(dispatch); return true; }) .catch((error) => { - dispatch({ type: actionTypes.EVENT_CREATE_FAILURE }); setNotification(failureMessage)(dispatch); if (error?.response?.data?.errors) { setReactHookFormError(setError, error.response.data.errors); @@ -148,20 +125,14 @@ export function updateEvent( setError, ): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.EVENT_UPDATE_REQUEST }); return CourseAPI.lessonPlan .updateEvent(eventId, { lesson_plan_event: values }) .then((response) => { - dispatch({ - type: actionTypes.EVENT_UPDATE_SUCCESS, - eventId, - event: response.data, - }); + dispatch(actions.eventUpdated(response.data)); setNotification(successMessage)(dispatch); return true; }) .catch((error) => { - dispatch({ type: actionTypes.EVENT_UPDATE_FAILURE }); setNotification(failureMessage)(dispatch); if (error?.response?.data?.errors) { setReactHookFormError(setError, error.response.data.errors); @@ -178,18 +149,13 @@ export function deleteEvent( failureMessage, ): Operation { return async (dispatch) => { - dispatch({ type: actionTypes.EVENT_DELETE_REQUEST }); return CourseAPI.lessonPlan .deleteEvent(eventId) .then(() => { - dispatch({ - type: actionTypes.EVENT_DELETE_SUCCESS, - itemId, - }); + dispatch(actions.eventDeleted(itemId)); setNotification(successMessage)(dispatch); }) .catch(() => { - dispatch({ type: actionTypes.EVENT_DELETE_FAILURE }); setNotification(failureMessage)(dispatch); }); }; diff --git a/client/app/bundles/course/lesson-plan/reducers/flags.ts b/client/app/bundles/course/lesson-plan/reducers/flags.ts index 37d6f9428b5..7bb9a1ef1b3 100644 --- a/client/app/bundles/course/lesson-plan/reducers/flags.ts +++ b/client/app/bundles/course/lesson-plan/reducers/flags.ts @@ -1,36 +1,51 @@ -import actionTypes, { fields } from '../constants'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -export const initialState = { +import { LessonPlanEditColumn } from '../types'; + +import { lessonPlanActions } from './lessonPlan'; + +export interface LessonPlanFlagsState { + canManageLessonPlan: boolean; + milestonesExpanded: string; + editPageColumnsVisible: Record; +} + +export const initialState: LessonPlanFlagsState = { canManageLessonPlan: false, milestonesExpanded: 'current', editPageColumnsVisible: { - [fields.ITEM_TYPE]: true, - [fields.START_AT]: true, - [fields.BONUS_END_AT]: false, - [fields.END_AT]: true, - [fields.PUBLISHED]: true, + ITEM_TYPE: true, + START_AT: true, + BONUS_END_AT: false, + END_AT: true, + PUBLISHED: true, }, }; -export default function (state = initialState, action) { - const { type } = action; +export const flagsSlice = createSlice({ + name: 'lessonPlanFlags', + initialState, + reducers: { + setColumnVisibility( + state, + action: PayloadAction<{ field: string; isVisible: boolean }>, + ) { + const { field, isVisible } = action.payload; + state.editPageColumnsVisible[field] = isVisible; + }, + }, + // The flags arrive with the lesson plan itself, so this slice listens to the + // load rather than owning a fetch of its own. + extraReducers: (builder) => { + builder.addCase(lessonPlanActions.loadSucceeded, (state, action) => { + const { flags } = action.payload; + state.canManageLessonPlan = flags.canManageLessonPlan; + state.milestonesExpanded = + flags.milestonesExpanded || initialState.milestonesExpanded; + }); + }, +}); - switch (type) { - case actionTypes.SET_COLUMN_VISIBILITY: { - const editPageColumnsVisible = { - ...state.editPageColumnsVisible, - [action.field]: action.isVisible, - }; - return { ...state, editPageColumnsVisible }; - } - case actionTypes.LOAD_LESSON_PLAN_SUCCESS: { - const nextState = { ...state, ...action.flags }; - if (!nextState.milestonesExpanded) { - nextState.milestonesExpanded = initialState.milestonesExpanded; - } - return nextState; - } - default: - return state; - } -} +export const flagsActions = flagsSlice.actions; + +export default flagsSlice.reducer; diff --git a/client/app/bundles/course/lesson-plan/reducers/lessonPlan.ts b/client/app/bundles/course/lesson-plan/reducers/lessonPlan.ts index 16f164f8013..d50ebef29d9 100644 --- a/client/app/bundles/course/lesson-plan/reducers/lessonPlan.ts +++ b/client/app/bundles/course/lesson-plan/reducers/lessonPlan.ts @@ -1,6 +1,13 @@ -import { deleteIfFound, updateOrAppend } from 'lib/helpers/reducer-helpers'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import actionTypes from '../constants'; +import { + LessonPlanEventItem, + LessonPlanGroup, + LessonPlanItem, + LessonPlanItemUpdate, + LessonPlanMilestone, + LessonPlanPayload, +} from '../types'; import { generateTypeKey, @@ -9,7 +16,15 @@ import { initializeVisibility, } from './utils'; -const initialState = { +export interface LessonPlanState { + items: LessonPlanItem[]; + milestones: LessonPlanMilestone[]; + groups: LessonPlanGroup[]; + visibilityByType: Record; + isLoading: boolean; +} + +const initialState: LessonPlanState = { items: [], milestones: [], groups: [], @@ -17,101 +32,141 @@ const initialState = { isLoading: false, }; -export default function (state = initialState, action) { - switch (action.type) { - case actionTypes.SET_ITEM_TYPE_VISIBILITY: { - const visibilityByType = { - ...state.visibilityByType, - [action.itemType]: action.isVisible, - }; - return { ...state, visibilityByType }; - } - case actionTypes.LOAD_LESSON_PLAN_REQUEST: { - return { ...state, isLoading: true }; - } - case actionTypes.LOAD_LESSON_PLAN_FAILURE: { - return { ...state, isLoading: false }; - } - case actionTypes.LOAD_LESSON_PLAN_SUCCESS: { - const items = action.items.map(generateTypeKey); +/** + * Ids arrive as both numbers and strings depending on the payload, so they are + * compared as strings, matching the `reducer-helpers` this slice replaced. + */ +const findById = ( + array: T[], + id?: number | string | null, +): number => array.findIndex((element) => String(element.id) === String(id)); + +/** + * Merges `element` into the matching entry, or appends it. The merge matters: + * an item update carries only the fields that changed, and the rest of the row + * must survive. + */ +const updateOrAppend = ( + array: T[], + element: T, +): T[] => { + const index = findById(array, element.id); + if (index === -1) return [...array, element]; + + const updated = [...array]; + updated[index] = { ...updated[index], ...element }; + return updated; +}; + +const deleteIfFound = ( + array: T[], + id?: number | string | null, +): T[] => { + const index = findById(array, id); + if (index === -1) return array; + + return array.filter((_, position) => position !== index); +}; + +export const lessonPlanSlice = createSlice({ + name: 'lessonPlan', + initialState, + reducers: { + setItemTypeVisibility( + state, + action: PayloadAction<{ itemType: string; isVisible: boolean }>, + ) { + const { itemType, isVisible } = action.payload; + state.visibilityByType[itemType] = isVisible; + }, + + loadRequested(state) { + state.isLoading = true; + }, + + loadFailed(state) { + state.isLoading = false; + }, + + loadSucceeded(state, action: PayloadAction) { + const items = action.payload.items.map(generateTypeKey); const visibilitySettings = generateVisibilitySettings( - action.visibilitySettings, + action.payload.visibilitySettings, ); - return { - ...state, - items, - milestones: action.milestones, - groups: groupItemsUnderMilestones(items, action.milestones), - visibilityByType: initializeVisibility(items, visibilitySettings), - isLoading: false, - }; - } - case actionTypes.ITEM_UPDATE_SUCCESS: { - const item = action.item.lesson_plan_item_type - ? generateTypeKey(action.item) - : action.item; - const items = updateOrAppend(state.items, item); - return { - ...state, - items, - groups: groupItemsUnderMilestones(items, state.milestones), - }; - } - case actionTypes.MILESTONE_CREATE_SUCCESS: { - const milestones = [...state.milestones, action.milestone]; - return { - ...state, - milestones, - groups: groupItemsUnderMilestones(state.items, milestones), - }; - } - case actionTypes.MILESTONE_UPDATE_SUCCESS: { - const milestones = updateOrAppend(state.milestones, action.milestone); - return { - ...state, - milestones, - groups: groupItemsUnderMilestones(state.items, milestones), - }; - } - case actionTypes.MILESTONE_DELETE_SUCCESS: { - const milestones = deleteIfFound(state.milestones, action.milestoneId); - return { - ...state, - milestones, - groups: groupItemsUnderMilestones(state.items, milestones), - }; - } - case actionTypes.EVENT_CREATE_SUCCESS: { - const items = [...state.items, generateTypeKey(action.event)]; - const { visibilityByType } = state; - return { - ...state, - items, - groups: groupItemsUnderMilestones(items, state.milestones), - visibilityByType: initializeVisibility(items, visibilityByType), - }; - } - case actionTypes.EVENT_UPDATE_SUCCESS: { - const items = updateOrAppend(state.items, generateTypeKey(action.event)); - const { visibilityByType } = state; - return { - ...state, - items, - groups: groupItemsUnderMilestones(items, state.milestones), - visibilityByType: initializeVisibility(items, visibilityByType), - }; - } - case actionTypes.EVENT_DELETE_SUCCESS: { - const items = deleteIfFound(state.items, action.itemId); - const { visibilityByType } = state; - return { - ...state, + + state.items = items; + state.milestones = action.payload.milestones; + state.groups = groupItemsUnderMilestones( items, - groups: groupItemsUnderMilestones(items, state.milestones), - visibilityByType: initializeVisibility(items, visibilityByType), - }; - } - default: - return state; - } -} + action.payload.milestones, + ); + state.visibilityByType = initializeVisibility(items, visibilitySettings); + state.isLoading = false; + }, + + itemUpdated( + state, + action: PayloadAction, + ) { + const payload = action.payload as LessonPlanItem; + const item = payload.lesson_plan_item_type + ? generateTypeKey(payload) + : payload; + + state.items = updateOrAppend(state.items, item); + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + }, + + milestoneCreated(state, action: PayloadAction) { + state.milestones = [...state.milestones, action.payload]; + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + }, + + milestoneUpdated(state, action: PayloadAction) { + state.milestones = updateOrAppend(state.milestones, action.payload); + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + }, + + milestoneDeleted(state, action: PayloadAction) { + state.milestones = deleteIfFound(state.milestones, action.payload); + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + }, + + eventCreated(state, action: PayloadAction) { + state.items = [ + ...state.items, + generateTypeKey(action.payload as LessonPlanItem), + ]; + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + state.visibilityByType = initializeVisibility( + state.items, + state.visibilityByType, + ); + }, + + eventUpdated(state, action: PayloadAction) { + state.items = updateOrAppend( + state.items, + generateTypeKey(action.payload as LessonPlanItem), + ); + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + state.visibilityByType = initializeVisibility( + state.items, + state.visibilityByType, + ); + }, + + eventDeleted(state, action: PayloadAction) { + state.items = deleteIfFound(state.items, action.payload); + state.groups = groupItemsUnderMilestones(state.items, state.milestones); + state.visibilityByType = initializeVisibility( + state.items, + state.visibilityByType, + ); + }, + }, +}); + +export const lessonPlanActions = lessonPlanSlice.actions; + +export default lessonPlanSlice.reducer; diff --git a/client/app/bundles/course/lesson-plan/reducers/utils.ts b/client/app/bundles/course/lesson-plan/reducers/utils.ts index fec42ff4fb5..ce3c02bcd23 100644 --- a/client/app/bundles/course/lesson-plan/reducers/utils.ts +++ b/client/app/bundles/course/lesson-plan/reducers/utils.ts @@ -1,15 +1,22 @@ import moment from 'lib/moment'; +import { + LessonPlanGroup, + LessonPlanItem, + LessonPlanMilestone, + VisibilitySetting, +} from '../types'; + /** * Adds a new attribute itemTypeKey to the lesson plan item. * itemTypeKey has two functions: * 1. It serves as key for the visibilityByType hash * 2. It is used as the display string for the 'type' of the item. */ -export function generateTypeKey(item) { +export function generateTypeKey(item: LessonPlanItem): LessonPlanItem { return { ...item, - itemTypeKey: item.lesson_plan_item_type.join(': '), + itemTypeKey: (item.lesson_plan_item_type ?? []).join(': '), }; } @@ -24,15 +31,20 @@ export function generateTypeKey(item) { * This becomes the visibilitySetting hash { 'Standard Assessment: Tab 2': false } where the key * is in the same format as itemTypeKey. */ -export function generateVisibilitySettings(visibilitySettings) { - const newVisibilitySettings = {}; +export function generateVisibilitySettings( + visibilitySettings: VisibilitySetting[], +): Record { + const newVisibilitySettings: Record = {}; visibilitySettings.forEach((setting) => { newVisibilitySettings[setting.setting_key.join(': ')] = setting.visible; }); return newVisibilitySettings; } -function sortByStartAt(a, b) { +function sortByStartAt( + a: { start_at?: LessonPlanItem['start_at'] }, + b: { start_at?: LessonPlanItem['start_at'] }, +): number { const aStartAt = moment(a.start_at); if (aStartAt.isAfter(b.start_at)) { return 1; @@ -44,60 +56,60 @@ function sortByStartAt(a, b) { } /** - * Groups lesson plan items under their respective milestones. - * An item falls under a milestone if the milestone is the latest milestone - * to have an earlier start_at date-time than the item. - * Items that precedes all milestones are grouped with an empty milestone. - * Items are sorted by startAt, then itemTypeKey, then title. - * - * @param {Array} items - * @param {Array} milestones - * @return {Array.<{ milestone: Object, items: Array }>} + * Groups lesson plan items under the milestone they fall after. Items before the + * first milestone are grouped on their own. */ -export function groupItemsUnderMilestones(items, milestones) { +export function groupItemsUnderMilestones( + items: LessonPlanItem[], + milestones: LessonPlanMilestone[], +): LessonPlanGroup[] { const sortedMilestones = [...milestones].sort(sortByStartAt); const sortedItems = [...items].sort((a, b) => { const startAtSortResult = sortByStartAt(a, b); if (startAtSortResult !== 0) { return startAtSortResult; } - const itemTypeSortResult = a.itemTypeKey.localeCompare(b.itemTypeKey); + const itemTypeSortResult = (a.itemTypeKey ?? '').localeCompare( + b.itemTypeKey ?? '', + ); if (itemTypeSortResult !== 0) { return itemTypeSortResult; } return a.title.localeCompare(b.title); }); - const groups = []; - const group = { id: null, milestone: null, items: [] }; + const groups: LessonPlanGroup[] = []; + let milestone: LessonPlanMilestone | null = null; + let groupItems: LessonPlanItem[] = []; - // Adds current group to groups and resets group - const addGroup = () => { - if (group.items.length > 0 || group.milestone) { - const milestoneId = group.milestone ? group.milestone.id : 'ungrouped'; - group.id = `milestone-group-${milestoneId}`; - groups.push({ ...group }); + // Adds the current group to groups and resets it + const addGroup = (): void => { + if (groupItems.length > 0 || milestone) { + groups.push({ + id: `milestone-group-${milestone ? milestone.id : 'ungrouped'}`, + milestone, + items: groupItems, + }); - group.id = null; - group.milestone = null; - group.items = []; + milestone = null; + groupItems = []; } }; - sortedMilestones.forEach((milestone) => { + sortedMilestones.forEach((nextMilestone) => { // Group items that come before the current milestone under the previous milestone while ( sortedItems.length > 0 && - moment(sortedItems[0].start_at).isBefore(milestone.start_at) + moment(sortedItems[0].start_at).isBefore(nextMilestone.start_at) ) { - group.items.push(sortedItems.shift()); + groupItems.push(sortedItems.shift() as LessonPlanItem); } // Finalize the group, then start a new group with the current milestone addGroup(); - group.milestone = milestone; + milestone = nextMilestone; }); // The remaining items belong with the last milestone - group.items = group.items.concat(sortedItems); + groupItems = groupItems.concat(sortedItems); addGroup(); return groups; @@ -109,14 +121,13 @@ export function groupItemsUnderMilestones(items, milestones) { * as read from the given visibilitySettings. * * All other items are visible by default. - * - * @param {Array} items - * @param {{itemTypeKey: Boolean}} visibilitySettings keyed by itemTypeKey - * @return {Object} */ -export function initializeVisibility(items, visibilitySettings) { - const itemTypes = new Set(items.map((item) => item.itemTypeKey)); - const visibility = {}; +export function initializeVisibility( + items: LessonPlanItem[], + visibilitySettings: Record, +): Record { + const itemTypes = new Set(items.map((item) => item.itemTypeKey ?? '')); + const visibility: Record = {}; itemTypes.forEach((itemType) => { const hasVisibilitySetting = Object.prototype.hasOwnProperty.call( visibilitySettings, diff --git a/client/app/bundles/course/lesson-plan/store.ts b/client/app/bundles/course/lesson-plan/store.ts index c579dc7c6cc..7bb14ab070b 100644 --- a/client/app/bundles/course/lesson-plan/store.ts +++ b/client/app/bundles/course/lesson-plan/store.ts @@ -1,26 +1,13 @@ -/* eslint-disable @typescript-eslint/explicit-function-return-type */ import { combineReducers } from 'redux'; -import flagsReducer from './reducers/flags'; -import lessonPlanReducer from './reducers/lessonPlan'; -import actionTypes from './constants'; +import flagsReducer, { flagsActions } from './reducers/flags'; +import lessonPlanReducer, { lessonPlanActions } from './reducers/lessonPlan'; const reducer = combineReducers({ flags: flagsReducer, lessonPlan: lessonPlanReducer, }); -export const actions = { - setItemTypeVisibility: (itemType, isVisible) => ({ - type: actionTypes.SET_ITEM_TYPE_VISIBILITY, - itemType, - isVisible, - }), - setColumnVisibility: (field, isVisible) => ({ - type: actionTypes.SET_COLUMN_VISIBILITY, - field, - isVisible, - }), -}; +export const actions = { ...lessonPlanActions, ...flagsActions }; export default reducer; diff --git a/client/app/bundles/course/lesson-plan/types.ts b/client/app/bundles/course/lesson-plan/types.ts index 35b2d61847b..e812a338dcc 100644 --- a/client/app/bundles/course/lesson-plan/types.ts +++ b/client/app/bundles/course/lesson-plan/types.ts @@ -1,12 +1,19 @@ import { ReactNode } from 'react'; import { UseFormSetError } from 'react-hook-form'; -import { AppDispatch } from 'store'; - -import { MessageTranslator } from 'lib/hooks/useTranslation'; /** A datetime as it arrives from the server, or as constructed by the pickers. */ export type LessonPlanDate = string | Date | null; +/** A column rendered on the lesson plan edit page, used in toggling visibility. */ +export const LESSON_PLAN_EDIT_COLUMNS = [ + 'ITEM_TYPE', + 'START_AT', + 'BONUS_END_AT', + 'END_AT', + 'PUBLISHED', +] as const; +export type LessonPlanEditColumn = (typeof LESSON_PLAN_EDIT_COLUMNS)[number]; + /** * A partial update to a lesson plan item. Dates are ISO strings, or `null` when * the field is being cleared. @@ -109,30 +116,6 @@ export type FormSubmitHandler = ( setError: UseFormSetError>, ) => Promise; -/** - * What a queued request needs. Passed to the debounced flush as an argument so - * that it never closes over props that may have moved on since the edit was - * queued. - */ -export interface SaveContext { - id: number; - title: string; - dispatch: AppDispatch; - t: MessageTranslator; -} - -/** - * The parts of the lesson plan slice these pages read. The reducers are still - * JavaScript, so `combineReducers` cannot infer their shape and selectors have - * to assert it. Drop the assertions once the reducers are converted. - */ -export interface LessonPlanSliceState { - visibilityByType: Record; - items: LessonPlanItem[]; - isLoading: boolean; - groups: LessonPlanGroup[]; -} - /** Items grouped under the milestone they fall after; see `groupItemsUnderMilestones`. */ export interface LessonPlanGroup { id: string; @@ -140,7 +123,19 @@ export interface LessonPlanGroup { items: LessonPlanItem[]; } -export interface LessonPlanFlagsState { - canManageLessonPlan: boolean; - editPageColumnsVisible: Record; +/** An item type's visibility as the server sends it, keyed by its type path. */ +export interface VisibilitySetting { + setting_key: string[]; + visible: boolean; +} + +/** The payload of a lesson plan fetch. */ +export interface LessonPlanPayload { + items: LessonPlanItem[]; + milestones: LessonPlanMilestone[]; + visibilitySettings: VisibilitySetting[]; + flags: { + canManageLessonPlan: boolean; + milestonesExpanded: string; + }; }