diff --git a/src/course-home/data/api.js b/src/course-home/data/api.js index e6fd22d5..9791171a 100644 --- a/src/course-home/data/api.js +++ b/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; } diff --git a/src/course-home/data/thunks.js b/src/course-home/data/thunks.js index 1002a7b0..99fdc6fd 100644 --- a/src/course-home/data/thunks.js +++ b/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 })); diff --git a/src/course-home/dates-tab/DatesTab.test.jsx b/src/course-home/dates-tab/DatesTab.test.jsx index d0937772..62c9e195 100644 --- a/src/course-home/dates-tab/DatesTab.test.jsx +++ b/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}`); + }); }); }); diff --git a/src/generic/model-store/hooks.js b/src/generic/model-store/hooks.js index 8ee8b41e..15294697 100644 --- a/src/generic/model-store/hooks.js +++ b/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, ); diff --git a/src/shared/access-denied-redirect/index.js b/src/shared/access-denied-redirect/index.js deleted file mode 100644 index cc0d1e00..00000000 --- a/src/shared/access-denied-redirect/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import AccessDeniedRedirect from './AccessDeniedRedirect'; - -export default AccessDeniedRedirect; diff --git a/src/shared/access-denied-redirect/AccessDeniedRedirect.jsx b/src/shared/access.js similarity index 62% rename from src/shared/access-denied-redirect/AccessDeniedRedirect.jsx rename to src/shared/access.js index 02e05d08..759475d0 100644 --- a/src/shared/access-denied-redirect/AccessDeniedRedirect.jsx +++ b/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 ( - - ); + return url; } - -AccessDeniedRedirect.defaultProps = { - unitId: null, -}; - -AccessDeniedRedirect.propTypes = { - courseId: PropTypes.string.isRequired, - metadataModel: PropTypes.string.isRequired, - unitId: PropTypes.string, -}; - -export default AccessDeniedRedirect; diff --git a/src/tab-page/TabPage.jsx b/src/tab-page/TabPage.jsx index 87438dd1..a84ad3ac 100644 --- a/src/tab-page/TabPage.jsx +++ b/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 (); + } + } + + // 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 ( <> {toastHeader} - + ); } - if (courseStatus === 'denied') { - return ( - - ); - } - // 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,