Moving courseStatus and courseId to “activeCourse” reducer.

Removing both from courseware and courseHome reducers.  This makes it easier for various portions of the app to find out what the current course is without being aware of which page is loaded.

In this MFE, we can basically always assume _some_ course is loaded.
This commit is contained in:
David Joy
2020-08-05 10:19:01 -04:00
parent 2c9fcce01e
commit 8967edee8d
27 changed files with 162 additions and 97 deletions

View File

@@ -4,6 +4,6 @@ Because we have a variety of models in this app (course, section, sequence, unit
https://redux.js.org/faq/organizing-state#how-do-i-organize-nested-or-duplicate-data-in-my-state
Different modules of the application maintain individual/lists of IDs that reference data stored in the model store. These are akin to indices in a database, in that they allow you to quickly extract data from the model store without iteration or filtering.
Different modules of the application maintain individual/lists of IDs that reference data stored in the model store. These are akin to indices in a database, in that they allow you to quickly extract data from the model store without iteration or filtering.
A common pattern when loading data from an API endpoint is to use the model-store's redux actions (addModel, updateModel, etc.) to load the "models" themselves into the model store by ID, and then dispatch another action to save references elsewhere in the redux store to the data that was just added. When adding courses, sequences, etc., to model-store, we also save the courseId and sequenceId in the 'courseware' part of redux. This means the courseware React Components can extract the data from the model-store quickly by using the courseId as a key: `state.models.courses[state.courseware.courseId]`. For an array, it iterates once over the ID list in order to extract the models from model-store. This iteration is done when React components' re-render, and can be done less often through memoization as necessary.
A common pattern when loading data from an API endpoint is to use the model-store's redux actions (addModel, updateModel, etc.) to load the "models" themselves into the model store by ID, and then dispatch another action to save references elsewhere in the redux store to the data that was just added. When adding courses, sequences, etc., to model-store, we also save the courseId and sequenceId in the 'courseware' part of redux. This means the courseware React Components can extract the data from the model-store quickly by using the courseId as a key: `state.models.courses[state.activeCourse.courseId]`. For an array, it iterates once over the ID list in order to extract the models from model-store. This iteration is done when React components' re-render, and can be done less often through memoization as necessary.

View File

@@ -10,6 +10,7 @@ import executeThunk from '../../utils';
import initializeMockApp from '../../setupTest';
import initializeStore from '../../store';
import { LOADING, FAILED } from '../../course/data/slice';
const { loggingService } = initializeMockApp();
@@ -41,7 +42,8 @@ describe('Data layer integration tests', () => {
});
it('Should initialize store', () => {
expect(store.getState()).toMatchSnapshot();
expect(store.getState().activeCourse.courseId).toBeNull();
expect(store.getState().activeCourse.courseStatus).toEqual(LOADING);
});
describe('Test fetchDatesTab', () => {
@@ -55,7 +57,7 @@ describe('Data layer integration tests', () => {
await executeThunk(thunks.fetchDatesTab(courseId), store.dispatch);
expect(loggingService.logError).toHaveBeenCalled();
expect(store.getState().courseHome.courseStatus).toEqual('failed');
expect(store.getState().activeCourse.courseStatus).toEqual(FAILED);
});
it('Should fetch, normalize, and save metadata', async () => {
@@ -70,8 +72,30 @@ describe('Data layer integration tests', () => {
await executeThunk(thunks.fetchDatesTab(courseId), store.dispatch);
const state = store.getState();
expect(state.courseHome.courseStatus).toEqual('loaded');
expect(state).toMatchSnapshot();
expect(state.activeCourse.courseStatus).toEqual('loaded');
expect(state.activeCourse.courseId).toEqual(courseId);
// Validate course
const course = state.models.courses[courseId];
const expectedFieldCount = Object.keys(course).length;
// If this breaks, you should consider adding assertions below for the new data. If it's not
// an "interesting" addition, bump the number.
expect(expectedFieldCount).toBe(8);
expect(course.title).toEqual(courseHomeMetadata.title);
// Representative sample of data that proves data normalization and ingestion happened.
expect(course.id).toEqual(courseId);
expect(course.isStaff).toBe(courseHomeMetadata.is_staff);
expect(course.number).toEqual(courseHomeMetadata.number);
expect(Array.isArray(course.tabs)).toBe(true);
expect(course.tabs.length).toBe(5); // Weak assertion, but proves the array made it through.
// This proves the tab type came through as a modelType. We don't need to assert much else
// here because the shape of this data is not passed through any sort of normalization scheme,
// it just gets camelCased.
const dates = state.models.dates[courseId];
expect(dates.id).toEqual(courseId);
expect(dates.verifiedUpgradeLink).toBe(datesTabData.verified_upgrade_link);
});
});
@@ -86,7 +110,7 @@ describe('Data layer integration tests', () => {
await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch);
expect(loggingService.logError).toHaveBeenCalled();
expect(store.getState().courseHome.courseStatus).toEqual('failed');
expect(store.getState().activeCourse.courseStatus).toEqual('failed');
});
it('Should fetch, normalize, and save metadata', async () => {
@@ -101,8 +125,29 @@ describe('Data layer integration tests', () => {
await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch);
const state = store.getState();
expect(state.courseHome.courseStatus).toEqual('loaded');
expect(state).toMatchSnapshot();
expect(state.activeCourse.courseStatus).toEqual('loaded');
// Validate course
const course = state.models.courses[courseId];
const expectedFieldCount = Object.keys(course).length;
// If this breaks, you should consider adding assertions below for the new data. If it's not
// an "interesting" addition, bump the number.
expect(expectedFieldCount).toBe(8);
expect(course.title).toEqual(courseHomeMetadata.title);
// Representative sample of data that proves data normalization and ingestion happened.
expect(course.id).toEqual(courseId);
expect(course.isStaff).toBe(courseHomeMetadata.is_staff);
expect(course.number).toEqual(courseHomeMetadata.number);
expect(Array.isArray(course.tabs)).toBe(true);
expect(course.tabs.length).toBe(5); // Weak assertion, but proves the array made it through.
// This proves the tab type came through as a modelType. We don't need to assert much else
// here because the shape of this data is not passed through any sort of normalization scheme,
// it just gets camelCased.
const outline = state.models.outline[courseId];
expect(outline.id).toEqual(courseId);
expect(outline.handoutsHtml).toBe(outlineTabData.handouts_html);
});
});

View File

@@ -8,23 +8,9 @@ export const FAILED = 'failed';
const slice = createSlice({
name: 'course-home',
initialState: {
courseStatus: 'loading',
courseId: null,
displayResetDatesToast: false,
},
reducers: {
fetchTabRequest: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = LOADING;
},
fetchTabSuccess: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = LOADED;
},
fetchTabFailure: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = FAILED;
},
toggleResetDatesToast: (state, { payload }) => {
state.displayResetDatesToast = payload.displayResetDatesToast;
},
@@ -32,9 +18,6 @@ const slice = createSlice({
});
export const {
fetchTabRequest,
fetchTabSuccess,
fetchTabFailure,
toggleResetDatesToast,
} = slice.actions;

View File

@@ -14,15 +14,14 @@ import {
} from '../../generic/model-store';
import {
fetchTabFailure,
fetchTabRequest,
fetchTabSuccess,
toggleResetDatesToast,
} from './slice';
fetchCourseRequest,
fetchCourseSuccess,
} from '../../course/data/slice';
export function fetchTab(courseId, tab, getTabData) {
return async (dispatch) => {
dispatch(fetchTabRequest({ courseId }));
dispatch(fetchCourseRequest({ courseId }));
Promise.allSettled([
getCourseHomeCourseMetadata(courseId),
getTabData(courseId),
@@ -55,9 +54,9 @@ export function fetchTab(courseId, tab, getTabData) {
}
if (fetchedCourseHomeCourseMetadata && fetchedTabData) {
dispatch(fetchTabSuccess({ courseId }));
dispatch(fetchCourseSuccess({ courseId }));
} else {
dispatch(fetchTabFailure({ courseId }));
dispatch(fetchCourseFailure({ courseId }));
}
});
};

View File

@@ -14,7 +14,7 @@ function DatesBannerContainer(props) {
const {
courseId,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
const {
courseDateBlocks,

View File

@@ -14,7 +14,7 @@ function Day({
}) {
const {
courseId,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
const {
userTimezone,

View File

@@ -9,7 +9,7 @@ import { daycmp, isLearnerAssignment } from './utils';
export default function Timeline() {
const {
courseId,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
const {
courseDateBlocks,

2
src/course-home/index.js Normal file
View File

@@ -0,0 +1,2 @@
/* eslint-disable import/prefer-default-export */
export { reducer } from './data';

View File

@@ -23,7 +23,7 @@ import WelcomeMessage from './widgets/WelcomeMessage';
function OutlineTab({ intl }) {
const {
courseId,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
const {
title,

View File

@@ -9,7 +9,7 @@ export default function DueDateTime({
}) {
const {
courseId,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
const {
userTimezone,
} = useModel('progress', courseId);

View File

@@ -10,7 +10,7 @@ import messages from './messages';
function ProgressTab({ intl }) {
const {
courseId,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
const { administrator } = getAuthenticatedUser();

View File

@@ -0,0 +1,8 @@
/* eslint-disable import/prefer-default-export */
import { createSelector } from 'reselect';
export const activeCourseSelector = createSelector(
(state) => state.models.courses || {},
(state) => state.activeCourse.courseId,
(coursesById, courseId) => (coursesById[courseId] ? coursesById[courseId] : null),
);

44
src/course/data/slice.js Normal file
View File

@@ -0,0 +1,44 @@
/* eslint-disable no-param-reassign */
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: 'activeCourse',
initialState: {
courseStatus: LOADING,
courseId: null,
},
reducers: {
fetchCourseRequest: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = LOADING;
},
fetchCourseSuccess: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = LOADED;
},
fetchCourseFailure: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = FAILED;
},
fetchCourseDenied: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = DENIED;
},
},
});
export const {
fetchCourseRequest,
fetchCourseSuccess,
fetchCourseFailure,
fetchCourseDenied,
} = slice.actions;
export const {
reducer,
} = slice;

2
src/course/index.js Normal file
View File

@@ -0,0 +1,2 @@
export { activeCourseSelector } from './data/selectors';
export { reducer } from './data/slice';

View File

@@ -18,6 +18,7 @@ import { TabPage } from '../tab-page';
import Course from './course';
import { handleNextSectionCelebration } from './course/celebration';
import { activeCourseSelector } from '../course/data/selectors';
const checkExamRedirect = memoize((sequenceStatus, sequence) => {
if (sequenceStatus === 'loaded') {
@@ -280,12 +281,6 @@ CoursewareContainer.defaultProps = {
sequence: null,
};
const currentCourseSelector = createSelector(
(state) => state.models.courses || {},
(state) => state.courseware.courseId,
(coursesById, courseId) => (coursesById[courseId] ? coursesById[courseId] : null),
);
const currentSequenceSelector = createSelector(
(state) => state.models.sequences || {},
(state) => state.courseware.sequenceId,
@@ -293,8 +288,8 @@ const currentSequenceSelector = createSelector(
);
const sequenceIdsSelector = createSelector(
(state) => state.courseware.courseStatus,
currentCourseSelector,
(state) => state.activeCourse.courseStatus,
activeCourseSelector,
(state) => state.models.sections,
(courseStatus, course, sectionsById) => {
if (courseStatus !== 'loaded') {
@@ -334,8 +329,8 @@ const nextSequenceSelector = createSelector(
);
const firstSequenceIdSelector = createSelector(
(state) => state.courseware.courseStatus,
currentCourseSelector,
(state) => state.activeCourse.courseStatus,
activeCourseSelector,
(state) => state.models.sections || {},
(courseStatus, course, sectionsById) => {
if (courseStatus !== 'loaded') {
@@ -353,8 +348,11 @@ const firstSequenceIdSelector = createSelector(
const mapStateToProps = (state) => {
const {
courseId, sequenceId, unitId, courseStatus, sequenceStatus,
sequenceId, sequenceStatus, unitId,
} = state.courseware;
const {
courseId, courseStatus,
} = state.activeCourse;
return {
courseId,
@@ -362,7 +360,7 @@ const mapStateToProps = (state) => {
unitId,
courseStatus,
sequenceStatus,
course: currentCourseSelector(state),
course: activeCourseSelector(state),
sequence: currentSequenceSelector(state),
previousSequence: previousSequenceSelector(state),
nextSequence: nextSequenceSelector(state),

View File

@@ -40,7 +40,7 @@ export default function CourseBreadcrumbs({
const course = useModel('courses', courseId);
const sequence = useModel('sequences', sequenceId);
const section = useModel('sections', sectionId);
const courseStatus = useSelector(state => state.courseware.courseStatus);
const courseStatus = useSelector(state => state.activeCourse.courseStatus);
const sequenceStatus = useSelector(state => state.courseware.sequenceStatus);
const links = useMemo(() => {

View File

@@ -20,11 +20,11 @@ describe('Sequence', () => {
beforeAll(async () => {
const store = await initializeTestStore({ courseMetadata, unitBlocks });
const { courseware } = store.getState();
const { courseware, activeCourse } = store.getState();
mockData = {
unitId: unitBlocks[0].id,
sequenceId: courseware.sequenceId,
courseId: courseware.courseId,
courseId: activeCourse.courseId,
unitNavigationHandler: () => {},
nextSequenceHandler: () => {},
previousSequenceHandler: () => {},

View File

@@ -8,10 +8,10 @@ describe('Sequence Content', () => {
beforeAll(async () => {
store = await initializeTestStore();
const { models, courseware } = store.getState();
const { models, courseware, activeCourse } = store.getState();
mockData = {
gated: false,
courseId: courseware.courseId,
courseId: activeCourse.courseId,
sequenceId: courseware.sequenceId,
unitId: models.sequences[courseware.sequenceId].unitIds[0],
unitLoadedHandler: () => {},

View File

@@ -7,7 +7,7 @@ import { sequenceIdsSelector } from '../../../data/selectors';
export function useSequenceNavigationMetadata(currentSequenceId, currentUnitId) {
const sequenceIds = useSelector(sequenceIdsSelector);
const sequence = useModel('sequences', currentSequenceId);
const courseStatus = useSelector(state => state.courseware.courseStatus);
const courseStatus = useSelector(state => state.activeCourse.courseStatus);
// If we don't know the sequence and unit yet, then assume no.
if (courseStatus !== 'loaded' || !currentSequenceId || !currentUnitId) {

View File

@@ -53,7 +53,7 @@ describe('Data layer integration tests', () => {
await executeThunk(thunks.fetchCourse(courseId), store.dispatch);
expect(loggingService.logError).toHaveBeenCalled();
expect(store.getState().courseware).toEqual(expect.objectContaining({
expect(store.getState().activeCourse).toEqual(expect.objectContaining({
courseId,
courseStatus: 'failed',
}));
@@ -78,7 +78,7 @@ describe('Data layer integration tests', () => {
const state = store.getState();
expect(state.courseware.courseStatus).toEqual('denied');
expect(state.activeCourse.courseStatus).toEqual('denied');
// check that at least one key camel cased, thus course data normalized
expect(state.models.courses[forbiddenCourseMetadata.id].canLoadCourseware).not.toBeUndefined();
@@ -92,8 +92,8 @@ describe('Data layer integration tests', () => {
const state = store.getState();
expect(state.courseware.courseStatus).toEqual('loaded');
expect(state.courseware.courseId).toEqual(courseId);
expect(state.activeCourse.courseStatus).toEqual('loaded');
expect(state.activeCourse.courseId).toEqual(courseId);
expect(state.courseware.sequenceStatus).toEqual('loading');
expect(state.courseware.sequenceId).toEqual(null);
@@ -139,8 +139,8 @@ describe('Data layer integration tests', () => {
// Update our state variable again.
state = store.getState();
expect(state.courseware.courseStatus).toEqual('loaded');
expect(state.courseware.courseId).toEqual(courseId);
expect(state.activeCourse.courseStatus).toEqual('loaded');
expect(state.activeCourse.courseId).toEqual(courseId);
expect(state.courseware.sequenceStatus).toEqual('loading');
expect(state.courseware.sequenceId).toEqual(null);
@@ -163,8 +163,8 @@ describe('Data layer integration tests', () => {
}),
});
expect(state.courseware.courseStatus).toEqual('loaded');
expect(state.courseware.courseId).toEqual(courseId);
expect(state.activeCourse.courseStatus).toEqual('loaded');
expect(state.activeCourse.courseId).toEqual(courseId);
expect(state.courseware.sequenceStatus).toEqual('loaded');
expect(state.courseware.sequenceId).toEqual(sequenceId);
});

View File

@@ -1,9 +1,9 @@
/* eslint-disable import/prefer-default-export */
export function sequenceIdsSelector(state) {
if (state.courseware.courseStatus !== 'loaded') {
if (state.activeCourse.courseStatus !== 'loaded') {
return [];
}
const { sectionIds = [] } = state.models.courses[state.courseware.courseId];
const { sectionIds = [] } = state.models.courses[state.activeCourse.courseId];
const sequenceIds = sectionIds
.flatMap(sectionId => state.models.sections[sectionId].sequenceIds);

View File

@@ -9,28 +9,10 @@ export const DENIED = 'denied';
const slice = createSlice({
name: 'courseware',
initialState: {
courseStatus: 'loading',
courseId: null,
sequenceStatus: 'loading',
sequenceId: null,
},
reducers: {
fetchCourseRequest: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = LOADING;
},
fetchCourseSuccess: (state, { payload }) => {
state.courseId = payload.courseId;
state.courseStatus = LOADED;
},
fetchCourseFailure: (state, { payload }) => {
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;
@@ -47,10 +29,6 @@ const slice = createSlice({
});
export const {
fetchCourseRequest,
fetchCourseSuccess,
fetchCourseFailure,
fetchCourseDenied,
fetchSequenceRequest,
fetchSequenceSuccess,
fetchSequenceFailure,

View File

@@ -14,6 +14,8 @@ import {
fetchCourseSuccess,
fetchCourseFailure,
fetchCourseDenied,
} from '../../course/data/slice';
import {
fetchSequenceRequest,
fetchSequenceSuccess,
fetchSequenceFailure,

View File

@@ -1,2 +1,3 @@
export { default } from './CoursewareContainer';
export { default as CoursewareRedirect } from './CoursewareRedirect';
export { reducer } from './data/slice';

View File

@@ -14,7 +14,7 @@ import { configureStore } from '@reduxjs/toolkit';
import { IntlProvider } from 'react-intl';
import MockAdapter from 'axios-mock-adapter';
import AppProvider from '@edx/frontend-platform/react/AppProvider';
import { reducer as courseHomeReducer } from './course-home/data';
import { reducer as activeCourseReducer } from './course';
import { reducer as coursewareReducer } from './courseware/data/slice';
import { reducer as modelsReducer } from './generic/model-store';
import { UserMessagesProvider } from './generic/user-messages';
@@ -81,8 +81,8 @@ export async function initializeTestStore(options = {}, overrideStore = true) {
const store = configureStore({
reducer: {
models: modelsReducer,
activeCourse: activeCourseReducer,
courseware: coursewareReducer,
courseHome: courseHomeReducer,
},
});
if (overrideStore) {

View File

@@ -1,14 +1,17 @@
import { configureStore } from '@reduxjs/toolkit';
import { reducer as courseHomeReducer } from './course-home/data';
import { reducer as coursewareReducer } from './courseware/data/slice';
import { reducer as activeCourseReducer } from './course';
import { reducer as courseHomeReducer } from './course-home';
import { reducer as coursewareReducer } from './courseware';
import { reducer as modelsReducer } from './generic/model-store';
export default function initializeStore() {
return configureStore({
reducer: {
models: modelsReducer,
courseware: coursewareReducer,
activeCourse: activeCourseReducer,
courseHome: courseHomeReducer,
courseware: coursewareReducer,
models: modelsReducer,
},
});
}

View File

@@ -24,7 +24,7 @@ export default function TabContainer(props) {
const {
courseId,
courseStatus,
} = useSelector(state => state.courseHome);
} = useSelector(state => state.activeCourse);
return (
<TabPage