diff --git a/src/CoursewareRedirect.jsx b/src/CoursewareRedirect.jsx new file mode 100644 index 00000000..f1647be3 --- /dev/null +++ b/src/CoursewareRedirect.jsx @@ -0,0 +1,30 @@ +import React from 'react'; +import { Switch, Route, useRouteMatch } from 'react-router'; +import { getConfig } from '@edx/frontend-platform'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import PageLoading from './PageLoading'; + +export default () => { + const { path } = useRouteMatch(); + return ( +
+ + )} + /> + + + { + global.location.assign(`${getConfig().LMS_BASE_URL}/courses/${match.params.courseId}/course/`); + }} + /> + +
+ ); +}; diff --git a/src/PageLoading.jsx b/src/PageLoading.jsx index 1b1135dc..d7a81295 100644 --- a/src/PageLoading.jsx +++ b/src/PageLoading.jsx @@ -33,5 +33,5 @@ export default class PageLoading extends Component { } PageLoading.propTypes = { - srMessage: PropTypes.string.isRequired, + srMessage: PropTypes.node.isRequired, }; diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 84855596..19b86367 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -1,9 +1,9 @@ import React, { useEffect, useCallback } from 'react'; import PropTypes from 'prop-types'; import { useSelector, useDispatch } from 'react-redux'; -import { history, getConfig } from '@edx/frontend-platform'; +import { history } from '@edx/frontend-platform'; -import { useRouteMatch } from 'react-router'; +import { useRouteMatch, Redirect } from 'react-router'; import { fetchCourse, fetchSequence, @@ -120,21 +120,6 @@ function useSavedSequencePosition(courseId, sequenceId, unitId) { }, [unitId]); } -/** - * Redirects the user away from the app if they don't have access to view this course. - * - * @param {*} courseStatus - * @param {*} course - */ -function useAccessDeniedRedirect(courseStatus, courseId) { - const course = useModel('courses', courseId); - useEffect(() => { - if (courseStatus === 'loaded' && !course.userHasAccess && !course.isStaff) { - global.location.assign(`${getConfig().LMS_BASE_URL}/courses/${course.id}/course/`); - } - }, [courseStatus, course]); -} - export default function CoursewareContainer() { const { params } = useRouteMatch(); const { @@ -173,11 +158,14 @@ export default function CoursewareContainer() { const previousSequenceHandler = usePreviousSequenceHandler(courseId, sequenceId); const unitNavigationHandler = useUnitNavigationHandler(courseId, sequenceId, routeUnitId); - useAccessDeniedRedirect(courseStatus, courseId); useContentRedirect(courseStatus, sequenceStatus); useExamRedirect(sequenceId); useSavedSequencePosition(courseId, sequenceId, routeUnitId); + if (courseStatus === 'denied') { + return ; + } + return (
{ if (courseStatus === 'loaded' && sequenceStatus === 'loaded') { - return [section, sequence].map((node) => ({ + return [section, sequence].filter(node => !!node).map((node) => ({ id: node.id, label: node.title, url: `${getConfig().LMS_BASE_URL}/courses/${course.id}/course/#${node.id}`, diff --git a/src/courseware/data/selectors.js b/src/courseware/data/selectors.js index 50eebc66..73514b13 100644 --- a/src/courseware/data/selectors.js +++ b/src/courseware/data/selectors.js @@ -3,11 +3,11 @@ export function sequenceIdsSelector(state) { if (state.courseware.courseStatus !== 'loaded') { return []; } - const { sectionIds } = state.models.courses[state.courseware.courseId]; - let sequenceIds = []; - sectionIds.forEach(sectionId => { - sequenceIds = [...sequenceIds, ...state.models.sections[sectionId].sequenceIds]; - }); + const { sectionIds = [] } = state.models.courses[state.courseware.courseId]; + + const sequenceIds = sectionIds + .flatMap(sectionId => state.models.sections[sectionId].sequenceIds); + return sequenceIds; } @@ -15,6 +15,11 @@ export function firstSequenceIdSelector(state) { if (state.courseware.courseStatus !== 'loaded') { return null; } - const sectionId = state.models.courses[state.courseware.courseId].sectionIds[0]; - return state.models.sections[sectionId].sequenceIds[0]; + const { sectionIds = [] } = state.models.courses[state.courseware.courseId]; + + if (sectionIds.length === 0) { + return null; + } + + return state.models.sections[sectionIds[0]].sequenceIds[0]; } diff --git a/src/data/slice.js b/src/data/slice.js index 8166b1bb..088e9519 100644 --- a/src/data/slice.js +++ b/src/data/slice.js @@ -4,6 +4,7 @@ import { createSlice } from '@reduxjs/toolkit'; export const LOADING = 'loading'; export const LOADED = 'loaded'; export const FAILED = 'failed'; +export const DENIED = 'denied'; const slice = createSlice({ name: 'courseware', @@ -26,6 +27,10 @@ const slice = createSlice({ state.courseId = payload.courseId; state.courseStatus = FAILED; }, + fetchCourseDenied: (state, { payload }) => { + state.courseId = payload.courseId; + state.courseStatus = DENIED; + }, fetchSequenceRequest: (state, { payload }) => { state.sequenceId = payload.sequenceId; state.sequenceStatus = LOADING; @@ -45,6 +50,7 @@ export const { fetchCourseRequest, fetchCourseSuccess, fetchCourseFailure, + fetchCourseDenied, fetchSequenceRequest, fetchSequenceSuccess, fetchSequenceFailure, diff --git a/src/data/thunks.js b/src/data/thunks.js index 6a82ca68..ded167b6 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -5,12 +5,13 @@ import { getSequenceMetadata, } from './api'; import { - addModelsMap, updateModel, updateModels, updateModelsMap, + addModelsMap, updateModel, updateModels, updateModelsMap, addModel, } from '../model-store'; import { fetchCourseRequest, fetchCourseSuccess, fetchCourseFailure, + fetchCourseDenied, fetchSequenceRequest, fetchSequenceSuccess, fetchSequenceFailure, @@ -19,39 +20,66 @@ import { export function fetchCourse(courseId) { return async (dispatch) => { dispatch(fetchCourseRequest({ courseId })); - Promise.all([ - getCourseBlocks(courseId), + Promise.allSettled([ getCourseMetadata(courseId), - ]).then(([ - { - courses, sections, sequences, units, - }, - course, - ]) => { - dispatch(addModelsMap({ - modelType: 'courses', - modelsMap: courses, - })); - dispatch(updateModel({ - modelType: 'courses', - model: course, - })); - dispatch(addModelsMap({ - modelType: 'sections', - modelsMap: sections, - })); - // We update for sequences and units because the sequence metadata may have come back first. - dispatch(updateModelsMap({ - modelType: 'sequences', - modelsMap: sequences, - })); - dispatch(updateModelsMap({ - modelType: 'units', - modelsMap: units, - })); - dispatch(fetchCourseSuccess({ courseId })); - }).catch((error) => { - logError(error); + getCourseBlocks(courseId), + ]).then(([courseMetadataResult, courseBlocksResult]) => { + if (courseMetadataResult.status === 'fulfilled') { + dispatch(addModel({ + modelType: 'courses', + model: courseMetadataResult.value, + })); + } + + if (courseBlocksResult.status === 'fulfilled') { + const { + courses, sections, sequences, units, + } = courseBlocksResult.value; + + dispatch(updateModelsMap({ + modelType: 'courses', + modelsMap: courses, + })); + dispatch(addModelsMap({ + modelType: 'sections', + modelsMap: sections, + })); + // We update for sequences and units because the sequence metadata may have come back first. + dispatch(updateModelsMap({ + modelType: 'sequences', + modelsMap: sequences, + })); + dispatch(updateModelsMap({ + modelType: 'units', + modelsMap: units, + })); + } + + const fetchedMetadata = courseMetadataResult.status === 'fulfilled'; + const fetchedBlocks = courseBlocksResult.status === 'fulfilled'; + + // Log errors for each request if needed. Course block failures may occur + // even if the course metadata request is successful + if (!fetchedBlocks) { + logError(courseBlocksResult.reason); + } + if (!fetchedMetadata) { + logError(courseMetadataResult.reason); + } + + if (fetchedMetadata) { + if (courseMetadataResult.value.userHasAccess && fetchedBlocks) { + // User has access + dispatch(fetchCourseSuccess({ courseId })); + return; + } + // User either doesn't have access or only has partial access + // (can't access course blocks) + dispatch(fetchCourseDenied({ courseId })); + return; + } + + // Definitely an error happening dispatch(fetchCourseFailure({ courseId })); }); }; diff --git a/src/index.jsx b/src/index.jsx index cffa86a6..78e0c77d 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -19,6 +19,7 @@ import './index.scss'; import './assets/favicon.ico'; import CoursewareContainer from './courseware'; import CourseHomeContainer from './course-home'; +import CoursewareRedirect from './CoursewareRedirect'; import store from './store'; @@ -27,6 +28,7 @@ subscribe(APP_READY, () => { + { }); initialize({ + // TODO: Remove this once the course blocks api supports unauthenticated + // access and we are prepared to support public courses in this app. + requireAuthenticatedUser: true, messages: [ appMessages, headerMessages,