feat: remove waffle flags for managing course outline sidebar

src/courseware/CoursewareContainer.test.jsx

@@ -1,7 +1,7 @@
 import { getConfig, history } from '@edx/frontend-platform';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 import { AppProvider } from '@edx/frontend-platform/react';
-import { waitForElementToBeRemoved, fireEvent } from '@testing-library/dom';
+import { waitForElementToBeRemoved } from '@testing-library/dom';
 import '@testing-library/jest-dom';
 import { render, screen } from '@testing-library/react';
 import React from 'react';
@@ -193,15 +193,13 @@ describe('CoursewareContainer', () => {
       expect(courseHeader.querySelector('.course-title')).toHaveTextContent(courseHomeMetadata.title);
     }
 
-    function assertSequenceNavigation(container, expectedUnitCount = 3) {
-      // Ensure we had appropriate sequence navigation buttons.  We should only have one unit.
+    function assertNoSequenceNavigation(container) {
       const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button');
-      expect(sequenceNavButtons).toHaveLength(expectedUnitCount + 2);
+      expect(sequenceNavButtons).toHaveLength(0);
 
-      expect(sequenceNavButtons[0]).toHaveTextContent('Previous');
-      // Prove this button is rendering an SVG tasks icon, meaning it's a unit/vertical.
-      expect(sequenceNavButtons[1].querySelector('svg')).toHaveClass('fa-tasks');
-      expect(sequenceNavButtons[sequenceNavButtons.length - 1]).toHaveTextContent('Next');
+      expect(container.querySelector('button, a')).not.toHaveTextContent('Previous');
+      expect(container.querySelector('svg.fa-tasks')).toBeNull();
+      expect(container.querySelector('button, a')).not.toHaveTextContent('Next');
     }
 
     beforeEach(async () => {
@@ -224,7 +222,7 @@ describe('CoursewareContainer', () => {
         const container = await loadContainer();
 
         assertLoadedHeader(container);
-        assertSequenceNavigation(container);
+        assertNoSequenceNavigation(container);
 
         expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents');
         expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId);
@@ -247,7 +245,7 @@ describe('CoursewareContainer', () => {
         const container = await loadContainer();
 
         assertLoadedHeader(container);
-        assertSequenceNavigation(container);
+        assertNoSequenceNavigation(container);
 
         expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents');
         expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId);
@@ -274,29 +272,12 @@ describe('CoursewareContainer', () => {
         setUpMockRequests({ courseBlocks });
       });
 
-      // describe('when the URL contains a unit ID', () => {
-      //   it('should ignore the section ID and redirect based on the unit ID', async () => {
-      //     const urlUnit = unitTree[1][1][1];
-      //     setUrl(sectionTree[1].id, urlUnit.id);
-      //     const container = await loadContainer();
-      //     assertLoadedHeader(container);
-      //     assertSequenceNavigation(container, 2);
-      //     assertLocation(container, sequenceTree[1][1].id, urlUnit.id);
-      //   });
-
-      //   it('should ignore invalid unit IDs and redirect to the course root', async () => {
-      //     setUrl(sectionTree[1].id, 'foobar');
-      //     await loadContainer();
-      //     expect(global.location.href).toEqual(`http://localhost/course/${courseId}`);
-      //   });
-      // });
-
       describe('when the URL does not contain a unit ID', () => {
         it('should choose a unit within the section\'s first sequence', async () => {
           setUrl(sectionTree[1].id);
           const container = await loadContainer();
           assertLoadedHeader(container);
-          assertSequenceNavigation(container, 2);
+          assertNoSequenceNavigation(container);
           assertLocation(container, sequenceTree[1][0].id, unitTree[1][0][0].id);
         });
       });
@@ -342,27 +323,6 @@ describe('CoursewareContainer', () => {
       });
     });
 
-    // describe('when the URL only contains a unit ID', () => {
-    //   const { courseBlocks, unitTree, sequenceTree } = buildBinaryCourseBlocks(courseId, courseMetadata.name);
-
-    //   beforeEach(async () => {
-    //     setUpMockRequests({ courseBlocks });
-    //   });
-
-    //   it('should insert the sequence ID into the URL', async () => {
-    //     const unit = unitTree[1][0][1];
-    //     history.push(`/course/${courseId}/${unit.id}`);
-    //     const container = await loadContainer();
-
-    //     assertLoadedHeader(container);
-    //     assertSequenceNavigation(container, 2);
-    //     const expectedSequenceId = sequenceTree[1][0].id;
-    //     const expectedUrl = `http://localhost/course/${courseId}/${expectedSequenceId}/${unit.id}`;
-    //     expect(global.location.href).toEqual(expectedUrl);
-    //     expect(container.querySelector('.fake-unit')).toHaveTextContent(unit.id);
-    //   });
-    // });
-
     describe('when the URL contains a course ID and sequence ID', () => {
       const sequenceBlock = defaultSequenceBlock;
       const unitBlocks = defaultUnitBlocks;
@@ -372,7 +332,7 @@ describe('CoursewareContainer', () => {
         const container = await loadContainer();
 
         assertLoadedHeader(container);
-        assertSequenceNavigation(container);
+        assertNoSequenceNavigation(container);
 
         expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents');
         expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId);
@@ -391,7 +351,7 @@ describe('CoursewareContainer', () => {
         const container = await loadContainer();
 
         assertLoadedHeader(container);
-        assertSequenceNavigation(container);
+        assertNoSequenceNavigation(container);
 
         expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents');
         expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId);
@@ -408,44 +368,24 @@ describe('CoursewareContainer', () => {
         const container = await loadContainer();
 
         assertLoadedHeader(container);
-        assertSequenceNavigation(container);
+        assertNoSequenceNavigation(container);
 
         expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents');
         expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId);
         expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].id);
       });
 
-      it('should navigate between units and check block completion', async () => {
-        axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/get_completion`).reply(200, {
-          complete: true,
-        });
+      it('should render the sequence_navigation plugin slot correctly', async () => {
+        axiosMock
+          .onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/get_completion`)
+          .reply(200, { complete: true });
 
         history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`);
-        const container = await loadContainer();
-
-        const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation a, nav.sequence-navigation button');
-        const sequenceNextButton = sequenceNavButtons[4];
-        expect(sequenceNextButton).toHaveTextContent('Next');
-        fireEvent.click(sequenceNextButton);
+        await loadContainer();
 
-        expect(global.location.href).toEqual(`http://localhost/course/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`);
+        expect(screen.getByTestId('org.openedx.frontend.learning.sequence_navigation.v1')).toBeInTheDocument();
       });
     });
-
-    // describe('when the current sequence is an exam', () => {
-    //   const { location } = window;
-
-    //   beforeEach(() => {
-    //     delete window.location;
-    //     window.location = {
-    //       assign: jest.fn(),
-    //     };
-    //   });
-
-    //   afterEach(() => {
-    //     window.location = location;
-    //   });
-    // });
   });
 
   describe('when receiving a course_access error_code', () => {

src/courseware/course/Course.jsx

@@ -1,14 +1,13 @@
 import { useEffect, useState } from 'react';
 import PropTypes from 'prop-types';
 import { Helmet } from 'react-helmet';
-import { useDispatch, useSelector } from 'react-redux';
+import { useDispatch } from 'react-redux';
 import { getConfig } from '@edx/frontend-platform';
 import { useLocation, useNavigate } from 'react-router-dom';
 import { breakpoints, useWindowSize } from '@openedx/paragon';
 
 import { AlertList } from '@src/generic/user-messages';
 import { useModel } from '@src/generic/model-store';
-import { getCoursewareOutlineSidebarSettings } from '../data/selectors';
 import Chat from './chat/Chat';
 import SidebarProvider from './sidebar/SidebarContextProvider';
 import NewSidebarProvider from './new-sidebar/SidebarContextProvider';
@@ -37,8 +36,6 @@ const Course = ({
   } = useModel('courseHomeMeta', courseId);
   const sequence = useModel('sequences', sequenceId);
   const section = useModel('sections', sequence ? sequence.sectionId : null);
-  const { enableNavigationSidebar } = useSelector(getCoursewareOutlineSidebarSettings);
-  const navigationDisabled = enableNavigationSidebar || (sequence?.navigationDisabled ?? false);
   const navigate = useNavigate();
   const { pathname } = useLocation();
 
@@ -84,17 +81,13 @@ const Course = ({
         <title>{`${pageTitleBreadCrumbs.join(' | ')} | ${getConfig().SITE_NAME}`}</title>
       </Helmet>
       <div className="position-relative d-flex align-items-xl-center mb-4 mt-1 flex-column flex-xl-row">
-        {navigationDisabled || (
-        <>
-          <CourseBreadcrumbsSlot
-            courseId={courseId}
-            sectionId={section ? section.id : null}
-            sequenceId={sequenceId}
-            isStaff={isStaff}
-            unitId={unitId}
-          />
-        </>
-        )}
+        <CourseBreadcrumbsSlot
+          courseId={courseId}
+          sectionId={section ? section.id : null}
+          sequenceId={sequenceId}
+          isStaff={isStaff}
+          unitId={unitId}
+        />
         {shouldDisplayChat && (
           <>
             <Chat

src/courseware/course/Course.test.jsx

@@ -202,15 +202,15 @@ describe('Course', () => {
     });
   });
 
-  it('renders course breadcrumbs as expected', async () => {
+  it('doesn\'t renders course breadcrumbs by default', async () => {
     const courseMetadata = Factory.build('courseMetadata');
     const unitBlocks = Array.from({ length: 3 }).map(() => Factory.build(
       'block',
       { type: 'vertical' },
       { courseId: courseMetadata.id },
     ));
     const testStore = await initializeTestStore({
-      courseMetadata, unitBlocks, enableNavigationSidebar: { enable_navigation_sidebar: false },
+      courseMetadata, unitBlocks,
     }, false);
     const { courseware, models } = testStore.getState();
     const { courseId, sequenceId } = courseware;
@@ -226,10 +226,10 @@ describe('Course', () => {
     await waitFor(() => {
       expect(screen.queryByText('Loading learning sequence...')).not.toBeInTheDocument();
     });
-    // expect the section and sequence "titles" to be loaded in as breadcrumb labels.
-    waitFor(() => {
-      expect(screen.findByText(Object.values(models.sections)[0].title)).toBeInTheDocument();
-      expect(screen.findByText(Object.values(models.sequences)[0].title)).toBeInTheDocument();
+    // expect the section and sequence "titles" not to be loaded in as breadcrumb labels.
+    await waitFor(() => {
+      expect(screen.queryByText(Object.values(models.sections)[0].title)).not.toBeInTheDocument();
+      expect(screen.queryByText(Object.values(models.sequences)[0].title)).not.toBeInTheDocument();
     });
   });
 

src/courseware/course/sequence/Sequence.jsx

@@ -19,7 +19,6 @@ import { CourseOutlineSidebarTriggerSlot } from '@src/plugin-slots/CourseOutline
 import { NotificationsDiscussionsSidebarSlot } from '@src/plugin-slots/NotificationsDiscussionsSidebarSlot';
 import SequenceNavigationSlot from '@src/plugin-slots/SequenceNavigationSlot';
 
-import { getCoursewareOutlineSidebarSettings } from '../../data/selectors';
 import CourseLicense from '../course-license';
 import messages from './messages';
 import HiddenAfterDue from './hidden-after-due';
@@ -48,7 +47,7 @@ const Sequence = ({
   const unit = useModel('units', unitId);
   const sequenceStatus = useSelector(state => state.courseware.sequenceStatus);
   const sequenceMightBeUnit = useSelector(state => state.courseware.sequenceMightBeUnit);
-  const { enableNavigationSidebar: isEnabledOutlineSidebar } = useSelector(getCoursewareOutlineSidebarSettings);
+
   const handleNext = () => {
     const nextIndex = sequence.unitIds.indexOf(unitId) + 1;
     const newUnitId = sequence.unitIds[nextIndex];
@@ -91,6 +90,30 @@ const Sequence = ({
     sendTrackingLogEvent(eventName, payload);
   };
 
+  /* istanbul ignore next */
+  const nextHandler = () => {
+    logEvent('edx.ui.lms.sequence.next_selected', 'top');
+    handleNext();
+  };
+
+  /* istanbul ignore next */
+  const previousHandler = () => {
+    logEvent('edx.ui.lms.sequence.previous_selected', 'top');
+    handlePrevious();
+  };
+
+  /* istanbul ignore next */
+  const onNavigate = (destinationUnitId) => {
+    logEvent('edx.ui.lms.sequence.tab_selected', 'top', destinationUnitId);
+    handleNavigate(destinationUnitId);
+  };
+
+  const sequenceNavProps = {
+    nextHandler,
+    previousHandler,
+    onNavigate,
+  };
+
   useSequenceBannerTextAlert(sequenceId);
   useSequenceEntranceExamAlert(courseId, sequenceId, intl);
 
@@ -171,30 +194,25 @@ const Sequence = ({
         />
         <CourseOutlineSidebarSlot />
         <div className="sequence w-100">
-          {!isEnabledOutlineSidebar && (
-            <div className="sequence-navigation-container">
-              <SequenceNavigationSlot
-                sequenceId={sequenceId}
-                unitId={unitId}
-                nextHandler={() => {
-                  logEvent('edx.ui.lms.sequence.next_selected', 'top');
-                  handleNext();
-                }}
-                onNavigate={(destinationUnitId) => {
-                  logEvent('edx.ui.lms.sequence.tab_selected', 'top', destinationUnitId);
-                  handleNavigate(destinationUnitId);
-                }}
-                previousHandler={() => {
-                  logEvent('edx.ui.lms.sequence.previous_selected', 'top');
-                  handlePrevious();
-                }}
-                {...{
-                  nextSequenceHandler,
-                  handleNavigate,
-                }}
-              />
-            </div>
-          )}
+          <div className="sequence-navigation-container">
+            {/**
+             SequenceNavigationSlot renders nothing by default.
+             However, we still pass nextHandler, previousHandler, and onNavigate,
+             because, as per the slot's contract, if this slot is replaced
+             with the default SequenceNavigation component, these props are required.
+             These handlers are excluded from test coverage via istanbul ignore,
+             since they are not used unless the slot is overridden.
+             */}
+            <SequenceNavigationSlot
+              sequenceId={sequenceId}
+              unitId={unitId}
+              {...{
+                ...sequenceNavProps,
+                nextSequenceHandler,
+                handleNavigate,
+              }}
+            />
+          </div>
 
           <div className="unit-container flex-grow-1 pt-4">
             <SequenceContent
@@ -204,7 +222,6 @@ const Sequence = ({
               unitId={unitId}
               unitLoadedHandler={handleUnitLoaded}
               isOriginalUserStaff={originalUserIsStaff}
-              isEnabledOutlineSidebar={isEnabledOutlineSidebar}
               renderUnitNavigation={renderUnitNavigation}
             />
             {unitHasLoaded && renderUnitNavigation(false)}