chore: address review feedback

open-craft · Jan 17, 2024 · eee98a3 · eee98a3
1 parent 7d4f536
commit eee98a3
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 79 deletions.
diff --git a/src/course-outline/CourseOutline.jsx b/src/course-outline/CourseOutline.jsx
@@ -135,9 +135,9 @@ const CourseOutline = ({ courseId }) => {
     subsection.childInfo = { ...subsection.childInfo };
     subsection.childInfo.children = updatedUnits();
 
-    const updatedSubsection = [...subsection.childInfo.children];
-    updatedSubsection[subsectionIndex] = subsection;
-    section.childInfo.children = updatedSubsection;
+    const updatedSubsections = [...section.childInfo.children];
+    updatedSubsections[subsectionIndex] = subsection;
+    section.childInfo.children = updatedSubsections;
     setSections([...sections.slice(0, sectionIndex), section, ...sections.slice(sectionIndex + 1)]);
   };
 
@@ -255,28 +255,26 @@ const CourseOutline = ({ courseId }) => {
                                       onDuplicateSubmit={handleDuplicateSubsectionSubmit}
                                       onNewUnitSubmit={handleNewUnitSubmit}
                                     >
-                                      {subsection.childInfo && (
-                                        <DraggableList
-                                          itemList={subsection.childInfo.children}
-                                          setState={setUnit(sectionIndex, subsectionIndex)}
-                                          updateOrder={finalizeUnitOrder(section, subsection)}
-                                        >
-                                          {subsection.childInfo.children.map((unit) => (
-                                            <UnitCard
-                                              key={unit.id}
-                                              unit={unit}
-                                              subsection={subsection}
-                                              section={section}
-                                              savingStatus={savingStatus}
-                                              onOpenPublishModal={openPublishModal}
-                                              onOpenDeleteModal={openDeleteModal}
-                                              onEditSubmit={handleEditSubmit}
-                                              onDuplicateSubmit={handleDuplicateUnitSubmit}
-                                              getTitleLink={getUnitUrl}
-                                            />
-                                          ))}
-                                        </DraggableList>
-                                      )}
+                                      <DraggableList
+                                        itemList={subsection.childInfo.children}
+                                        setState={setUnit(sectionIndex, subsectionIndex)}
+                                        updateOrder={finalizeUnitOrder(section, subsection)}
+                                      >
+                                        {subsection.childInfo.children.map((unit) => (
+                                          <UnitCard
+                                            key={unit.id}
+                                            unit={unit}
+                                            subsection={subsection}
+                                            section={section}
+                                            savingStatus={savingStatus}
+                                            onOpenPublishModal={openPublishModal}
+                                            onOpenDeleteModal={openDeleteModal}
+                                            onEditSubmit={handleEditSubmit}
+                                            onDuplicateSubmit={handleDuplicateUnitSubmit}
+                                            getTitleLink={getUnitUrl}
+                                          />
+                                        ))}
+                                      </DraggableList>
                                     </SubsectionCard>
                                   ))}
                                 </DraggableList>

diff --git a/src/course-outline/CourseOutline.test.jsx b/src/course-outline/CourseOutline.test.jsx
@@ -17,8 +17,6 @@ import {
   getCourseBlockApiUrl,
   getCourseItemApiUrl,
   getXBlockBaseApiUrl,
-  getChapterBlockApiUrl,
-  getSequentialBlockApiUrl,
 } from './data/api';
 import { RequestStatus } from '../data/constants';
 import {
@@ -739,14 +737,13 @@ describe('<CourseOutline />', () => {
   it('check that new subsection list is saved when dragged', async () => {
     const { findAllByTestId } = render(<RootWrapper />);
 
-    const courseBlockId = courseOutlineIndexMock.courseStructure.id;
     const [sectionElement] = await findAllByTestId('section-card');
-    const section = store.getState().courseOutline.sectionsList[0];
+    const [section] = store.getState().courseOutline.sectionsList;
     const subsectionsDraggers = within(sectionElement).getAllByRole('button', { name: 'Drag to reorder' });
     const draggableButton = subsectionsDraggers[1];
 
     axiosMock
-      .onPut(getChapterBlockApiUrl(courseBlockId, section.id))
+      .onPut(getCourseItemApiUrl(section.id))
       .reply(200, { dummy: 'value' });
 
     const subsection1 = section.childInfo.children[0].id;
@@ -766,14 +763,13 @@ describe('<CourseOutline />', () => {
   it('check that new subsection list is restored to original order when API call fails', async () => {
     const { findAllByTestId } = render(<RootWrapper />);
 
-    const courseBlockId = courseOutlineIndexMock.courseStructure.id;
     const [sectionElement] = await findAllByTestId('section-card');
-    const section = store.getState().courseOutline.sectionsList[0];
+    const [section] = store.getState().courseOutline.sectionsList;
     const subsectionsDraggers = within(sectionElement).getAllByRole('button', { name: 'Drag to reorder' });
     const draggableButton = subsectionsDraggers[1];
 
     axiosMock
-      .onPut(getChapterBlockApiUrl(courseBlockId, section.id))
+      .onPut(getCourseItemApiUrl(section.id))
       .reply(500);
 
     const subsection1 = section.childInfo.children[0].id;
@@ -792,16 +788,15 @@ describe('<CourseOutline />', () => {
 
   it('check that new unit list is saved when dragged', async () => {
     const { findAllByTestId } = render(<RootWrapper />);
-    const courseBlockId = courseOutlineIndexMock.courseStructure.id;
     const subsectionElement = (await findAllByTestId('subsection-card'))[3];
-    const subsection = store.getState().courseOutline.sectionsList[1].childInfo.children[0];
+    const [subsection] = store.getState().courseOutline.sectionsList[1].childInfo.children;
     const expandBtn = within(subsectionElement).getByTestId('subsection-card-header__expanded-btn');
     fireEvent.click(expandBtn);
     const unitDraggers = await within(subsectionElement).findAllByRole('button', { name: 'Drag to reorder' });
     const draggableButton = unitDraggers[1];
 
     axiosMock
-      .onPut(getSequentialBlockApiUrl(courseBlockId, subsection.id))
+      .onPut(getCourseItemApiUrl(subsection.id))
       .reply(200, { dummy: 'value' });
 
     const unit1 = subsection.childInfo.children[0].id;
@@ -820,16 +815,15 @@ describe('<CourseOutline />', () => {
 
   it('check that new unit list is restored to original order when API call fails', async () => {
     const { findAllByTestId } = render(<RootWrapper />);
-    const courseBlockId = courseOutlineIndexMock.courseStructure.id;
     const subsectionElement = (await findAllByTestId('subsection-card'))[3];
-    const subsection = store.getState().courseOutline.sectionsList[1].childInfo.children[0];
+    const [subsection] = store.getState().courseOutline.sectionsList[1].childInfo.children;
     const expandBtn = within(subsectionElement).getByTestId('subsection-card-header__expanded-btn');
     fireEvent.click(expandBtn);
     const unitDraggers = await within(subsectionElement).findAllByRole('button', { name: 'Drag to reorder' });
     const draggableButton = unitDraggers[1];
 
     axiosMock
-      .onPut(getSequentialBlockApiUrl(courseBlockId, subsection.id))
+      .onPut(getCourseItemApiUrl(subsection.id))
       .reply(500);
 
     const unit1 = subsection.childInfo.children[0].id;

diff --git a/src/course-outline/data/api.js b/src/course-outline/data/api.js
@@ -24,18 +24,6 @@ export const getCourseBlockApiUrl = (courseId) => {
   return `${getApiBaseUrl()}/xblock/block-v1:${formattedCourseId}+type@course+block@course`;
 };
 
-export const getChapterBlockApiUrl = (courseId, chapterId) => {
-  const formattedCourseId = courseId.split('course-v1:')[1];
-  const formattedChapterId = chapterId.split('@').slice(-1)[0];
-  return `${getApiBaseUrl()}/xblock/block-v1:${formattedCourseId}+type@chapter+block@${formattedChapterId}`;
-};
-
-export const getSequentialBlockApiUrl = (courseId, unitId) => {
-  const formattedCourseId = courseId.split('course-v1:')[1];
-  const formattedUnitId = unitId.split('@').slice(-1)[0];
-  return `${getApiBaseUrl()}/xblock/block-v1:${formattedCourseId}+type@sequential+block@${formattedUnitId}`;
-};
-
 export const getCourseReindexApiUrl = (reindexLink) => `${getApiBaseUrl()}${reindexLink}`;
 export const getXBlockBaseApiUrl = () => `${getApiBaseUrl()}/xblock/`;
 export const getCourseItemApiUrl = (itemId) => `${getXBlockBaseApiUrl()}${itemId}`;
@@ -326,14 +314,13 @@ export async function setSectionOrderList(courseId, children) {
 
 /**
  * Set order for the list of the subsections
- * @param {string} courseId
- * @param {string} sectionId
+ * @param {string} itemId Subsection or unit ID
  * @param {Array<string>} children list of sections id's
  * @returns {Promise<Object>}
 */
-export async function setSubsectionOrderList(courseId, sectionId, children) {
+export async function setCourseItemOrderList(itemId, children) {
   const { data } = await getAuthenticatedHttpClient()
-    .put(getChapterBlockApiUrl(courseId, sectionId), {
+    .put(getCourseItemApiUrl(itemId), {
       children,
     });
 
@@ -356,19 +343,3 @@ export async function setVideoSharingOption(courseId, videoSharingOption) {
 
   return data;
 }
-
-/**
- * Set order for the list of the units
- * @param {string} courseId
- * @param {string} subsectionId
- * @param {Array<string>} children list of unit id's
- * @returns {Promise<Object>}
-*/
-export async function setUnitOrderList(courseId, subsectionId, children) {
-  const { data } = await getAuthenticatedHttpClient()
-    .put(getSequentialBlockApiUrl(courseId, subsectionId), {
-      children,
-    });
-
-  return data;
-}
diff --git a/src/course-outline/data/thunk.js b/src/course-outline/data/thunk.js
@@ -25,8 +25,7 @@ import {
   updateCourseSectionHighlights,
   setSectionOrderList,
   setVideoSharingOption,
-  setSubsectionOrderList,
-  setUnitOrderList,
+  setCourseItemOrderList,
 } from './api';
 import {
   addSection,
@@ -470,13 +469,13 @@ export function setSectionOrderListQuery(courseId, sectionListIds, restoreCallba
   };
 }
 
-export function setSubsectionOrderListQuery(courseId, sectionId, subsectionListIds, restoreCallback) {
+export function setSubsectionOrderListQuery(sectionId, subsectionListIds, restoreCallback) {
   return async (dispatch) => {
     dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
     dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.saving));
 
     try {
-      await setSubsectionOrderList(courseId, sectionId, subsectionListIds).then(async (result) => {
+      await setCourseItemOrderList(sectionId, subsectionListIds).then(async (result) => {
         if (result) {
           dispatch(reorderSubsectionList({ sectionId, subsectionListIds }));
           dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL }));
@@ -491,13 +490,13 @@ export function setSubsectionOrderListQuery(courseId, sectionId, subsectionListI
   };
 }
 
-export function setUnitOrderListQuery(courseId, sectionId, subsectionId, unitListIds, restoreCallback) {
+export function setUnitOrderListQuery(sectionId, subsectionId, unitListIds, restoreCallback) {
   return async (dispatch) => {
     dispatch(updateSavingStatus({ status: RequestStatus.PENDING }));
     dispatch(showProcessingNotification(NOTIFICATION_MESSAGES.saving));
 
     try {
-      await setUnitOrderList(courseId, subsectionId, unitListIds).then(async (result) => {
+      await setCourseItemOrderList(subsectionId, unitListIds).then(async (result) => {
         if (result) {
           dispatch(reorderUnitList({ sectionId, subsectionId, unitListIds }));
           dispatch(updateSavingStatus({ status: RequestStatus.SUCCESSFUL }));

diff --git a/src/course-outline/hooks.jsx b/src/course-outline/hooks.jsx
@@ -192,15 +192,15 @@ const useCourseOutline = ({ courseId }) => {
   };
 
   const handleSubsectionDragAndDrop = (sectionId, subsectionListIds, restoreCallback) => {
-    dispatch(setSubsectionOrderListQuery(courseId, sectionId, subsectionListIds, restoreCallback));
+    dispatch(setSubsectionOrderListQuery(sectionId, subsectionListIds, restoreCallback));
   };
 
   const handleVideoSharingOptionChange = (value) => {
     dispatch(setVideoSharingOptionQuery(courseId, value));
   };
 
   const handleUnitDragAndDrop = (sectionId, subsectionId, unitListIds, restoreCallback) => {
-    dispatch(setUnitOrderListQuery(courseId, sectionId, subsectionId, unitListIds, restoreCallback));
+    dispatch(setUnitOrderListQuery(sectionId, subsectionId, unitListIds, restoreCallback));
   };
 
   useEffect(() => {