From 7fcc501d2edec3805e13e7899feb36ba52fcd84e Mon Sep 17 00:00:00 2001 From: Peter Kulko <93188219+PKulkoRaccoonGang@users.noreply.github.com> Date: Mon, 15 Jan 2024 14:34:42 +0200 Subject: [PATCH] feat: Unit creation button logic and refactoring --- src/CourseAuthoringRoutes.jsx | 1 + src/course-unit/CourseUnit.jsx | 21 +++-- src/course-unit/CourseUnit.test.jsx | 90 +++++++++++++++++++ .../__mocks__/courseSectionVertical.js | 9 ++ src/course-unit/__mocks__/courseUnit.js | 84 +++++++++++++++++ src/course-unit/__mocks__/index.js | 1 + src/course-unit/course-sequence/Sequence.jsx | 3 + src/course-unit/course-sequence/hooks.js | 29 ++---- .../SequenceNavigation.jsx | 3 + .../SequenceNavigationDropdown.jsx | 16 +++- .../SequenceNavigationTabs.jsx | 31 +++++-- .../sequence-navigation/UnitButton.jsx | 6 +- src/course-unit/data/api.js | 22 ++--- src/course-unit/data/selectors.js | 10 +-- src/course-unit/data/slice.js | 16 ++++ src/course-unit/data/thunk.js | 80 +++++++---------- src/course-unit/data/utils.js | 22 +++++ src/course-unit/header-title/HeaderTitle.jsx | 10 ++- .../header-title/HeaderTitle.test.jsx | 43 ++++++--- src/course-unit/hooks.jsx | 34 ++++--- 20 files changed, 393 insertions(+), 138 deletions(-) create mode 100644 src/course-unit/__mocks__/courseUnit.js diff --git a/src/CourseAuthoringRoutes.jsx b/src/CourseAuthoringRoutes.jsx index bdeffa110..0f1a470eb 100644 --- a/src/CourseAuthoringRoutes.jsx +++ b/src/CourseAuthoringRoutes.jsx @@ -73,6 +73,7 @@ const CourseAuthoringRoutes = () => { /> {DECODED_ROUTES.COURSE_UNIT.map((path) => ( } /> diff --git a/src/course-unit/CourseUnit.jsx b/src/course-unit/CourseUnit.jsx index 402017204..1e550adf7 100644 --- a/src/course-unit/CourseUnit.jsx +++ b/src/course-unit/CourseUnit.jsx @@ -27,8 +27,10 @@ const CourseUnit = ({ courseId }) => { isLoading, sequenceId, unitTitle, + isQueryPending, savingStatus, - isTitleEditFormOpen, + isEditTitleFormOpen, + isErrorAlert, isInternetConnectionAlertFailed, handleTitleEditSubmit, headerNavigationsActions, @@ -52,7 +54,7 @@ const CourseUnit = ({ courseId }) => { <>
- + {intl.formatMessage(messages.alertFailedGeneric, { actionName: 'save', type: 'changes' })} { title={( @@ -78,6 +80,7 @@ const CourseUnit = ({ courseId }) => { courseId={courseId} sequenceId={sequenceId} unitId={blockId} + handleCreateNewCourseXblock={handleCreateNewCourseXblock} /> { isShow={isShowProcessingNotification} title={processingNotificationTitle} /> - + {isQueryPending && ( + + )} ); diff --git a/src/course-unit/CourseUnit.test.jsx b/src/course-unit/CourseUnit.test.jsx index 01a3199c9..15b94b438 100644 --- a/src/course-unit/CourseUnit.test.jsx +++ b/src/course-unit/CourseUnit.test.jsx @@ -7,6 +7,7 @@ import { IntlProvider } from '@edx/frontend-platform/i18n'; import { AppProvider } from '@edx/frontend-platform/react'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { cloneDeep, set } from 'lodash'; import { getCourseSectionVerticalApiUrl, @@ -23,11 +24,13 @@ import { courseCreateXblockMock, courseSectionVerticalMock, courseUnitIndexMock, + courseUnitMock, } from './__mocks__'; import { executeThunk } from '../utils'; import CourseUnit from './CourseUnit'; import headerNavigationsMessages from './header-navigations/messages'; import headerTitleMessages from './header-title/messages'; +import courseSequenceMessages from './course-sequence/messages'; import messages from './add-component/messages'; let axiosMock; @@ -192,6 +195,93 @@ describe('', () => { }); }); + it('correct addition of a new course unit after click on the "Add new unit" button', async () => { + const { getByRole, getAllByTestId } = render(); + let units = null; + const updatedCourseSectionVerticalData = cloneDeep(courseSectionVerticalMock); + const updatedAncestorsChild = updatedCourseSectionVerticalData.xblock_info.ancestor_info.ancestors[0]; + set(updatedCourseSectionVerticalData, 'xblock_info.ancestor_info.ancestors[0].child_info.children', [ + ...updatedAncestorsChild.child_info.children, + courseUnitMock, + ]); + + await waitFor(async () => { + units = getAllByTestId('course-unit-btn'); + const courseUnits = courseSectionVerticalMock.xblock_info.ancestor_info.ancestors[0].child_info.children; + expect(units.length).toEqual(courseUnits.length); + }); + + axiosMock + .onPost(postXBlockBaseApiUrl(), { parent_locator: blockId, category: 'vertical', display_name: 'Unit' }) + .reply(200, { dummy: 'value' }); + axiosMock.reset(); + axiosMock + .onGet(getCourseSectionVerticalApiUrl(blockId)) + .reply(200, { + ...updatedCourseSectionVerticalData, + }); + + await executeThunk(fetchCourseSectionVerticalData(blockId), store.dispatch); + + const addNewUnitBtn = getByRole('button', { name: courseSequenceMessages.newUnitBtnText.defaultMessage }); + units = getAllByTestId('course-unit-btn'); + const updatedCourseUnits = updatedCourseSectionVerticalData + .xblock_info.ancestor_info.ancestors[0].child_info.children; + + userEvent.click(addNewUnitBtn); + expect(units.length).toEqual(updatedCourseUnits.length); + expect(mockedUsedNavigate).toHaveBeenCalled(); + expect(mockedUsedNavigate) + .toHaveBeenCalledWith(`/course/${courseId}/container/${blockId}/${updatedAncestorsChild.id}`, { replace: true }); + }); + + it('the sequence unit is updated after changing the unit header', async () => { + const { getAllByTestId, getByRole } = render(); + const updatedCourseSectionVerticalData = cloneDeep(courseSectionVerticalMock); + const updatedAncestorsChild = updatedCourseSectionVerticalData.xblock_info.ancestor_info.ancestors[0]; + set(updatedCourseSectionVerticalData, 'xblock_info.ancestor_info.ancestors[0].child_info.children', [ + ...updatedAncestorsChild.child_info.children, + courseUnitMock, + ]); + + const newDisplayName = `${unitDisplayName} new`; + + axiosMock + .onPost(getXBlockBaseApiUrl(blockId, { + metadata: { + display_name: newDisplayName, + }, + })) + .reply(200, { dummy: 'value' }) + .onGet(getCourseUnitApiUrl(blockId)) + .reply(200, { + ...courseUnitIndexMock, + metadata: { + ...courseUnitIndexMock.metadata, + display_name: newDisplayName, + }, + }) + .onGet(getCourseSectionVerticalApiUrl(blockId)) + .reply(200, { + ...updatedCourseSectionVerticalData, + }); + + await executeThunk(fetchCourseSectionVerticalData(blockId), store.dispatch); + + const editTitleButton = getByRole('button', { name: headerTitleMessages.altButtonEdit.defaultMessage }); + fireEvent.click(editTitleButton); + + const titleEditField = getByRole('textbox', { name: headerTitleMessages.ariaLabelButtonEdit.defaultMessage }); + fireEvent.change(titleEditField, { target: { value: newDisplayName } }); + + await act(async () => fireEvent.blur(titleEditField)); + + await waitFor(async () => { + const units = getAllByTestId('course-unit-btn'); + expect(units.some(unit => unit.title === newDisplayName)).toBe(true); + }); + }); + it('handles creating Video xblock and navigates to editor page', async () => { const { courseKey, locator } = courseCreateXblockMock; axiosMock diff --git a/src/course-unit/__mocks__/courseSectionVertical.js b/src/course-unit/__mocks__/courseSectionVertical.js index fdae7cdd5..ceea70fd5 100644 --- a/src/course-unit/__mocks__/courseSectionVertical.js +++ b/src/course-unit/__mocks__/courseSectionVertical.js @@ -498,6 +498,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'other', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@4f6c1b4e316a419ab5b6bf30e6c708e9', @@ -581,6 +582,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'video', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@3dc16db8d14842e38324e95d4030b8a0', @@ -664,6 +666,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'other', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@4a1bba2a403f40bca5ec245e945b0d76', @@ -747,6 +750,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'video', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@256f17a44983429fb1a60802203ee4e0', @@ -830,6 +834,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'other', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@e3601c0abee6427d8c17e6d6f8fdddd1', @@ -913,6 +918,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'video', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@a79d59cd72034188a71d388f4954a606', @@ -996,6 +1002,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'other', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@134df56c516a4a0dbb24dd5facef746e', @@ -1079,6 +1086,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'other', }, { id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@d91b9e5d8bc64d57a1332d06bf2f2193', @@ -1162,6 +1170,7 @@ module.exports = { selected_groups_label: '', }, enable_copy_paste_units: false, + xblock_type: 'video', }, ], }, diff --git a/src/course-unit/__mocks__/courseUnit.js b/src/course-unit/__mocks__/courseUnit.js new file mode 100644 index 000000000..07c2bf03b --- /dev/null +++ b/src/course-unit/__mocks__/courseUnit.js @@ -0,0 +1,84 @@ +module.exports = { + id: 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@d91b9e5d8bc64d57a1332d06bf2f2144', + display_name: 'Getting Started new', + category: 'vertical', + has_children: true, + edited_on: 'Dec 28, 2023 at 10:00 UTC', + published: true, + published_on: 'Dec 28, 2023 at 10:00 UTC', + studio_url: '/container/block-v1:edX+DemoX+Demo_Course+type@vertical+block@d91b9e5d8bc64d57a1332d06bf2f2193', + released_to_students: true, + release_date: 'Feb 05, 2013 at 05:00 UTC', + visibility_state: 'live', + has_explicit_staff_lock: false, + start: '2013-02-05T05:00:00Z', + graded: false, + due_date: '', + due: null, + relative_weeks_due: null, + format: null, + course_graders: [ + 'Homework', + 'Exam', + ], + has_changes: false, + actions: { + deletable: true, + draggable: true, + childAddable: true, + duplicable: true, + }, + explanatory_message: null, + group_access: {}, + user_partitions: [ + { + id: 50, + name: 'Enrollment Track Groups', + scheme: 'enrollment_track', + groups: [ + { + id: 2, + name: 'Verified Certificate', + selected: false, + deleted: false, + }, + { + id: 1, + name: 'Audit', + selected: false, + deleted: false, + }, + ], + }, + ], + show_correctness: 'always', + discussion_enabled: true, + ancestor_has_staff_lock: false, + user_partition_info: { + selectable_partitions: [ + { + id: 50, + name: 'Enrollment Track Groups', + scheme: 'enrollment_track', + groups: [ + { + id: 2, + name: 'Verified Certificate', + selected: false, + deleted: false, + }, + { + id: 1, + name: 'Audit', + selected: false, + deleted: false, + }, + ], + }, + ], + selected_partition_index: -1, + selected_groups_label: '', + }, + enable_copy_paste_units: false, + xblock_type: 'other', +}; diff --git a/src/course-unit/__mocks__/index.js b/src/course-unit/__mocks__/index.js index a6f44a81d..cd5c4ffdf 100644 --- a/src/course-unit/__mocks__/index.js +++ b/src/course-unit/__mocks__/index.js @@ -1,3 +1,4 @@ export { default as courseUnitIndexMock } from './courseUnitIndex'; export { default as courseSectionVerticalMock } from './courseSectionVertical'; +export { default as courseUnitMock } from './courseUnit'; export { default as courseCreateXblockMock } from './courseCreateXblock'; diff --git a/src/course-unit/course-sequence/Sequence.jsx b/src/course-unit/course-sequence/Sequence.jsx index 09477dbc6..2e8909857 100644 --- a/src/course-unit/course-sequence/Sequence.jsx +++ b/src/course-unit/course-sequence/Sequence.jsx @@ -13,6 +13,7 @@ const Sequence = ({ courseId, sequenceId, unitId, + handleCreateNewCourseXblock, }) => { const intl = useIntl(); const { IN_PROGRESS, FAILED, SUCCESSFUL } = RequestStatus; @@ -26,6 +27,7 @@ const Sequence = ({ sequenceId={sequenceId} unitId={unitId} courseId={courseId} + handleCreateNewCourseXblock={handleCreateNewCourseXblock} /> @@ -58,6 +60,7 @@ Sequence.propTypes = { unitId: PropTypes.string, courseId: PropTypes.string.isRequired, sequenceId: PropTypes.string, + handleCreateNewCourseXblock: PropTypes.func.isRequired, }; Sequence.defaultProps = { diff --git a/src/course-unit/course-sequence/hooks.js b/src/course-unit/course-sequence/hooks.js index a4ff2f9a9..043a693ff 100644 --- a/src/course-unit/course-sequence/hooks.js +++ b/src/course-unit/course-sequence/hooks.js @@ -3,36 +3,23 @@ import { useLayoutEffect, useRef, useState } from 'react'; import { useWindowSize } from '@openedx/paragon'; import { useModel } from '../../generic/model-store'; -import { RequestStatus } from '../../data/constants'; -import { getCourseSectionVertical, getSequenceStatus, sequenceIdsSelector } from '../data/selectors'; +import { + getCourseSectionVertical, + getCourseUnit, + sequenceIdsSelector, +} from '../data/selectors'; export function useSequenceNavigationMetadata(currentSequenceId, currentUnitId) { - const { SUCCESSFUL } = RequestStatus; const sequenceIds = useSelector(sequenceIdsSelector); - const sequenceStatus = useSelector(getSequenceStatus); const { nextUrl, prevUrl } = useSelector(getCourseSectionVertical); const sequence = useModel('sequences', currentSequenceId); - const { courseId, status } = useSelector(state => state.courseDetail); - - const isCourseOrSequenceNotSuccessful = status !== SUCCESSFUL || sequenceStatus !== SUCCESSFUL; - const areIdsNotValid = !currentSequenceId || !currentUnitId || !sequence.unitIds; - const isNotSuccessfulCompletion = isCourseOrSequenceNotSuccessful || areIdsNotValid; - - // If we don't know the sequence and unit yet, then assume no. - if (isNotSuccessfulCompletion) { - return { isFirstUnit: false, isLastUnit: false }; - } + const { courseId } = useSelector(getCourseUnit); + const isFirstUnit = !prevUrl; + const isLastUnit = !nextUrl; const sequenceIndex = sequenceIds.indexOf(currentSequenceId); const unitIndex = sequence.unitIds.indexOf(currentUnitId); - const isFirstSequence = sequenceIndex === 0; - const isFirstUnitInSequence = unitIndex === 0; - const isFirstUnit = isFirstSequence && isFirstUnitInSequence; - const isLastSequence = sequenceIndex === sequenceIds.length - 1; - const isLastUnitInSequence = unitIndex === sequence.unitIds.length - 1; - const isLastUnit = isLastSequence && isLastUnitInSequence; - const nextSequenceId = sequenceIndex < sequenceIds.length - 1 ? sequenceIds[sequenceIndex + 1] : null; const previousSequenceId = sequenceIndex > 0 ? sequenceIds[sequenceIndex - 1] : null; diff --git a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigation.jsx b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigation.jsx index 79d7d3d2c..43da94894 100644 --- a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigation.jsx +++ b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigation.jsx @@ -23,6 +23,7 @@ const SequenceNavigation = ({ unitId, sequenceId, className, + handleCreateNewCourseXblock, }) => { const sequenceStatus = useSelector(getSequenceStatus); const { @@ -42,6 +43,7 @@ const SequenceNavigation = ({ ); }; @@ -105,6 +107,7 @@ SequenceNavigation.propTypes = { unitId: PropTypes.string, className: PropTypes.string, sequenceId: PropTypes.string, + handleCreateNewCourseXblock: PropTypes.func.isRequired, }; SequenceNavigation.defaultProps = { diff --git a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationDropdown.jsx b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationDropdown.jsx index e0b977869..e601ce2f3 100644 --- a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationDropdown.jsx +++ b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationDropdown.jsx @@ -1,16 +1,17 @@ import PropTypes from 'prop-types'; -import { Dropdown } from '@openedx/paragon'; +import { Button, Dropdown } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { Plus as PlusIcon } from '@openedx/paragon/icons/'; import messages from '../messages'; import UnitButton from './UnitButton'; -const SequenceNavigationDropdown = ({ unitId, unitIds }) => { +const SequenceNavigationDropdown = ({ unitId, unitIds, handleClick }) => { const intl = useIntl(); return ( - + {intl.formatMessage(messages.sequenceDropdownTitle, { current: unitIds.indexOf(unitId) + 1, total: unitIds.length, @@ -27,6 +28,14 @@ const SequenceNavigationDropdown = ({ unitId, unitIds }) => { unitId={buttonUnitId} /> ))} + ); @@ -35,6 +44,7 @@ const SequenceNavigationDropdown = ({ unitId, unitIds }) => { SequenceNavigationDropdown.propTypes = { unitId: PropTypes.string.isRequired, unitIds: PropTypes.arrayOf(PropTypes.string).isRequired, + handleClick: PropTypes.func.isRequired, }; export default SequenceNavigationDropdown; diff --git a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx index d287305d5..49ed5a00b 100644 --- a/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx +++ b/src/course-unit/course-sequence/sequence-navigation/SequenceNavigationTabs.jsx @@ -1,16 +1,25 @@ +import { useDispatch, useSelector } from 'react-redux'; import PropTypes from 'prop-types'; -import { Link } from 'react-router-dom'; import { Button } from '@openedx/paragon'; import { Plus as PlusIcon } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { useNavigate } from 'react-router-dom'; -import { useIndexOfLastVisibleChild } from '../hooks'; +import { changeEditTitleFormOpen, updateQueryPendingStatus } from '../../data/slice'; +import { getCourseId, getSequenceId } from '../../data/selectors'; +import { createCorrectInternalRoute } from '../../../utils'; import messages from '../messages'; +import { useIndexOfLastVisibleChild } from '../hooks'; import SequenceNavigationDropdown from './SequenceNavigationDropdown'; import UnitButton from './UnitButton'; -const SequenceNavigationTabs = ({ unitIds, unitId }) => { +const SequenceNavigationTabs = ({ unitIds, unitId, handleCreateNewCourseXblock }) => { const intl = useIntl(); + const dispatch = useDispatch(); + const navigate = useNavigate(); + const sequenceId = useSelector(getSequenceId); + const courseId = useSelector(getCourseId); + const [ indexOfLastVisibleChild, containerRef, @@ -18,6 +27,14 @@ const SequenceNavigationTabs = ({ unitIds, unitId }) => { ] = useIndexOfLastVisibleChild(); const shouldDisplayDropdown = indexOfLastVisibleChild === -1; + const handleAddNewSequenceUnit = () => { + dispatch(updateQueryPendingStatus(true)); + handleCreateNewCourseXblock({ parentLocator: sequenceId, category: 'vertical', displayName: 'Unit' }, ({ courseKey, locator }) => { + navigate(createCorrectInternalRoute(`/course/${courseKey}/container/${locator}/${sequenceId}`), courseId); + dispatch(changeEditTitleFormOpen(true)); + }); + }; + return (
@@ -32,13 +49,11 @@ const SequenceNavigationTabs = ({ unitIds, unitId }) => { isActive={unitId === buttonUnitId} /> ))} - {/* TODO: The functionality of the New unit button will be implemented in https://youtrack.raccoongang.com/issue/AXIMST-14 */} @@ -48,6 +63,7 @@ const SequenceNavigationTabs = ({ unitIds, unitId }) => { )}
@@ -57,6 +73,7 @@ const SequenceNavigationTabs = ({ unitIds, unitId }) => { SequenceNavigationTabs.propTypes = { unitId: PropTypes.string.isRequired, unitIds: PropTypes.arrayOf(PropTypes.string).isRequired, + handleCreateNewCourseXblock: PropTypes.func.isRequired, }; export default SequenceNavigationTabs; diff --git a/src/course-unit/course-sequence/sequence-navigation/UnitButton.jsx b/src/course-unit/course-sequence/sequence-navigation/UnitButton.jsx index 71918bc15..bb9801077 100644 --- a/src/course-unit/course-sequence/sequence-navigation/UnitButton.jsx +++ b/src/course-unit/course-sequence/sequence-navigation/UnitButton.jsx @@ -4,12 +4,13 @@ import { Button } from '@openedx/paragon'; import { Link } from 'react-router-dom'; import UnitIcon from './UnitIcon'; +import { getCourseId, getSequenceId } from '../../data/selectors'; const UnitButton = ({ title, contentType, isActive, unitId, className, showTitle, }) => { - const courseId = useSelector(state => state.courseUnit.courseId); - const sequenceId = useSelector(state => state.courseUnit.sequenceId); + const courseId = useSelector(getCourseId); + const sequenceId = useSelector(getSequenceId); return (