fix: re-enable access error redirects for course home (#570)

These redirects are already in place for the courseware, and will
redirect to the outline page, or the dashboard, or wherever, based
on the type of access error (unenrolled, access expired, survey
needed, etc).

This commit stops the course home tabs from paying attention to the
401 error messages coming from the backend - course_access in the
metadata API handles that now.

This commit also changes our useModel hook to more gracefully handle
not being able to find the model - it is a valid case we want to
allow (still will cause problems if you actually try to use the data,
but will at least provide an object you can inspect).

src/course-home/data/api.js

@@ -199,10 +199,12 @@ export async function getDatesTabData(courseId) {
     const { httpErrorStatus } = error && error.customAttributes;
     if (httpErrorStatus === 404) {
       global.location.replace(`${getConfig().LMS_BASE_URL}/courses/${courseId}/dates`);
+      return {};
     }
-    // 401 can be returned for unauthenticated users or users who are not enrolled
     if (httpErrorStatus === 401) {
-      global.location.replace(`${getConfig().BASE_URL}/course/${courseId}/home`);
+      // The backend sends this for unenrolled and unauthenticated learners, but we handle those cases by examining
+      // courseAccess in the metadata call, so just ignore this status for now.
+      return {};
     }
     throw error;
   }
@@ -266,10 +268,12 @@ export async function getProgressTabData(courseId, targetUserId) {
     const { httpErrorStatus } = error && error.customAttributes;
     if (httpErrorStatus === 404) {
       global.location.replace(`${getConfig().LMS_BASE_URL}/courses/${courseId}/progress`);
+      return {};
     }
-    // 401 can be returned for unauthenticated users or users who are not enrolled
     if (httpErrorStatus === 401) {
-      global.location.replace(`${getConfig().BASE_URL}/course/${courseId}/home`);
+      // The backend sends this for unenrolled and unauthenticated learners, but we handle those cases by examining
+      // courseAccess in the metadata call, so just ignore this status for now.
+      return {};
     }
     throw error;
   }

src/course-home/data/thunks.js

@@ -17,7 +17,7 @@ import {
 } from '../../generic/model-store';
 
 import {
-  // fetchTabDenied,
+  fetchTabDenied,
   fetchTabFailure,
   fetchTabRequest,
   fetchTabSuccess,
@@ -63,10 +63,9 @@ export function fetchTab(courseId, tab, getTabData, targetUserId) {
       }
 
       // Disable the access-denied path for now - it caused a regression
-      /* if (fetchedCourseHomeCourseMetadata && !courseHomeCourseMetadataResult.value.courseAccess.hasAccess) {
+      if (fetchedCourseHomeCourseMetadata && !courseHomeCourseMetadataResult.value.courseAccess.hasAccess) {
         dispatch(fetchTabDenied({ courseId }));
-      } else */
-      if (fetchedCourseHomeCourseMetadata && fetchedTabData) {
+      } else if (fetchedCourseHomeCourseMetadata && fetchedTabData) {
         dispatch(fetchTabSuccess({ courseId, targetUserId }));
       } else {
         dispatch(fetchTabFailure({ courseId }));

src/course-home/dates-tab/DatesTab.test.jsx

@@ -299,7 +299,7 @@ describe('DatesTab', () => {
     });
   });
 
-  describe.skip('when receiving an access denied error', () => {
+  describe('when receiving an access denied error', () => {
     // These tests could go into any particular tab, as they all go through the same flow. But dates tab works.
 
     async function renderDenied(errorCode) {
@@ -338,5 +338,15 @@ describe('DatesTab', () => {
       await renderDenied('course_not_started');
       expect(global.location.href).toEqual('http://localhost/redirect/dashboard?notlive=2/5/2013'); // date from factory
     });
+
+    it('redirects to the home page when unauthenticated', async () => {
+      await renderDenied('authentication_required');
+      expect(global.location.href).toEqual(`http://localhost/redirect/course-home/${courseMetadata.id}`);
+    });
+
+    it('redirects to the home page when unenrolled', async () => {
+      await renderDenied('enrollment_required');
+      expect(global.location.href).toEqual(`http://localhost/redirect/course-home/${courseMetadata.id}`);
+    });
   });
 });

src/generic/model-store/hooks.js

@@ -2,7 +2,7 @@ import { useSelector, shallowEqual } from 'react-redux';
 
 export function useModel(type, id) {
   return useSelector(
-    state => (state.models[type] !== undefined ? state.models[type][id] : undefined),
+    state => (state.models[type] !== undefined ? state.models[type][id] : {}),
     shallowEqual,
   );
 }
@@ -10,7 +10,7 @@ export function useModel(type, id) {
 export function useModels(type, ids) {
   return useSelector(
     state => ids.map(
-      id => (state.models[type] !== undefined ? state.models[type][id] : undefined),
+      id => (state.models[type] !== undefined ? state.models[type][id] : {}),
     ),
     shallowEqual,
   );

src/shared/access-denied-redirect/index.js

@@ -1,3 +0,0 @@
-import AccessDeniedRedirect from './AccessDeniedRedirect';
-
-export default AccessDeniedRedirect;

src/shared/access.js

@@ -1,26 +1,11 @@
-import React from 'react';
-import PropTypes from 'prop-types';
+/* eslint-disable import/prefer-default-export */
 import { getLocale } from '@edx/frontend-platform/i18n';
-import { Redirect } from 'react-router';
-import { useModel } from '../../generic/model-store';
 
-// This component inspects an access denied error and redirects to a /redirect/... path, which then renders a nice
-// little message while the browser loads the next page.
+// This function inspects an access denied error and provides a redirect url (looks like a /redirect/... path),
+// which then renders a nice little message while the browser loads the next page.
 // This is basically a frontend version of check_course_access_with_redirect in the backend.
-
-function AccessDeniedRedirect(props) {
-  const {
-    courseId,
-    metadataModel,
-    unitId,
-  } = props;
-
-  const {
-    courseAccess,
-    start,
-  } = useModel(metadataModel, courseId);
-
-  let url = `/redirect/course-home/${courseId}`;
+export function getAccessDeniedRedirectUrl(courseId, activeTabSlug, courseAccess, start, unitId) {
+  let url = null;
   switch (courseAccess.errorCode) {
     case 'audit_expired':
       url = `/redirect/dashboard?access_response_error=${courseAccess.additionalContextUserMessage}`;
@@ -48,20 +33,9 @@ function AccessDeniedRedirect(props) {
     case 'authentication_required':
     case 'enrollment_required':
     default:
+      if (activeTabSlug !== 'outline') {
+        url = `/redirect/course-home/${courseId}`;
+      }
   }
-  return (
-    <Redirect to={url} />
-  );
+  return url;
 }
-
-AccessDeniedRedirect.defaultProps = {
-  unitId: null,
-};
-
-AccessDeniedRedirect.propTypes = {
-  courseId: PropTypes.string.isRequired,
-  metadataModel: PropTypes.string.isRequired,
-  unitId: PropTypes.string,
-};
-
-export default AccessDeniedRedirect;

src/tab-page/TabPage.jsx

@@ -2,31 +2,37 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 import { useDispatch, useSelector } from 'react-redux';
+import { Redirect } from 'react-router';
 
 import { Toast } from '@edx/paragon';
 import { Header } from '../course-header';
-import AccessDeniedRedirect from '../shared/access-denied-redirect';
+import { getAccessDeniedRedirectUrl } from '../shared/access';
 import PageLoading from '../generic/PageLoading';
+import { useModel } from '../generic/model-store';
 
 import genericMessages from '../generic/messages';
 import messages from './messages';
 import LoadedTabPage from './LoadedTabPage';
 import { setCallToActionToast } from '../course-home/data/slice';
 
-function TabPage({
-  intl,
-  courseId,
-  courseStatus,
-  metadataModel,
-  unitId,
-  ...passthroughProps
-}) {
+function TabPage({ intl, ...props }) {
+  const {
+    activeTabSlug,
+    courseId,
+    courseStatus,
+    metadataModel,
+    unitId,
+  } = props;
   const {
     toastBodyLink,
     toastBodyText,
     toastHeader,
   } = useSelector(state => state.courseHome);
   const dispatch = useDispatch();
+  const {
+    courseAccess,
+    start,
+  } = useModel(metadataModel, courseId);
 
   if (courseStatus === 'loading') {
     return (
@@ -39,7 +45,16 @@ function TabPage({
     );
   }
 
-  if (courseStatus === 'loaded') {
+  if (courseStatus === 'denied') {
+    const redirectUrl = getAccessDeniedRedirectUrl(courseId, activeTabSlug, courseAccess, start, unitId);
+    if (redirectUrl) {
+      return (<Redirect to={redirectUrl} />);
+    }
+  }
+
+  // Either a success state or a denied state that wasn't redirected above (some tabs handle denied states themselves,
+  // like the outline tab handling unenrolled learners)
+  if (courseStatus === 'loaded' || courseStatus === 'denied') {
     return (
       <>
         <Toast
@@ -53,21 +68,11 @@ function TabPage({
         >
           {toastHeader}
         </Toast>
-        <LoadedTabPage courseId={courseId} metadataModel={metadataModel} unitId={unitId} {...passthroughProps} />
+        <LoadedTabPage {...props} />
       </>
     );
   }
 
-  if (courseStatus === 'denied') {
-    return (
-      <AccessDeniedRedirect
-        courseId={courseId}
-        metadataModel={metadataModel}
-        unitId={unitId}
-      />
-    );
-  }
-
   // courseStatus 'failed' and any other unexpected course status.
   return (
     <>
@@ -80,12 +85,14 @@ function TabPage({
 }
 
 TabPage.defaultProps = {
+  courseId: null,
   unitId: null,
 };
 
 TabPage.propTypes = {
+  activeTabSlug: PropTypes.string.isRequired,
   intl: intlShape.isRequired,
-  courseId: PropTypes.string.isRequired,
+  courseId: PropTypes.string,
   courseStatus: PropTypes.string.isRequired,
   metadataModel: PropTypes.string.isRequired,
   unitId: PropTypes.string,