Fix checkBlockCompletion parameters

We were assuming a prop named unitId existed in CoursewareContainer - it doesn’t.  unitId is not in redux.  What we do have, is the unitId in the route params - what we refer to as routeUnitId.  If we use this instead of the non-existent unitId, then life is good.

I wrote a test (that breaks!) prior to implementing the fix.  The fix satisfies the test. 🎉
This commit is contained in:
David Joy
2020-08-12 12:13:46 -04:00
committed by David Joy
parent a975b8ae70
commit cc7142e5c1
3 changed files with 37 additions and 8 deletions

View File

@@ -125,9 +125,15 @@ class CoursewareContainer extends Component {
handleUnitNavigationClick = (nextUnitId) => { handleUnitNavigationClick = (nextUnitId) => {
const { const {
courseId, sequenceId, unitId, courseId, sequenceId,
match: {
params: {
unitId: routeUnitId,
},
},
} = this.props; } = this.props;
this.props.checkBlockCompletion(courseId, sequenceId, unitId);
this.props.checkBlockCompletion(courseId, sequenceId, routeUnitId);
history.push(`/course/${courseId}/${sequenceId}/${nextUnitId}`); history.push(`/course/${courseId}/${sequenceId}/${nextUnitId}`);
} }
@@ -256,7 +262,6 @@ CoursewareContainer.propTypes = {
courseId: PropTypes.string, courseId: PropTypes.string,
sequenceId: PropTypes.string, sequenceId: PropTypes.string,
firstSequenceId: PropTypes.string, firstSequenceId: PropTypes.string,
unitId: PropTypes.string,
courseStatus: PropTypes.oneOf(['loaded', 'loading', 'failed', 'denied']).isRequired, courseStatus: PropTypes.oneOf(['loaded', 'loading', 'failed', 'denied']).isRequired,
sequenceStatus: PropTypes.oneOf(['loaded', 'loading', 'failed']).isRequired, sequenceStatus: PropTypes.oneOf(['loaded', 'loading', 'failed']).isRequired,
nextSequence: sequenceShape, nextSequence: sequenceShape,
@@ -273,7 +278,6 @@ CoursewareContainer.defaultProps = {
courseId: null, courseId: null,
sequenceId: null, sequenceId: null,
firstSequenceId: null, firstSequenceId: null,
unitId: null,
nextSequence: null, nextSequence: null,
previousSequence: null, previousSequence: null,
course: null, course: null,
@@ -353,13 +357,12 @@ const firstSequenceIdSelector = createSelector(
const mapStateToProps = (state) => { const mapStateToProps = (state) => {
const { const {
courseId, sequenceId, unitId, courseStatus, sequenceStatus, courseId, sequenceId, courseStatus, sequenceStatus,
} = state.courseware; } = state.courseware;
return { return {
courseId, courseId,
sequenceId, sequenceId,
unitId,
courseStatus, courseStatus,
sequenceStatus, sequenceStatus,
course: currentCourseSelector(state), course: currentCourseSelector(state),

View File

@@ -1,7 +1,7 @@
import { getConfig, history } from '@edx/frontend-platform'; import { getConfig, history } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { AppProvider } from '@edx/frontend-platform/react'; import { AppProvider } from '@edx/frontend-platform/react';
import { waitForElementToBeRemoved } from '@testing-library/dom'; import { waitForElementToBeRemoved, fireEvent } from '@testing-library/dom';
import '@testing-library/jest-dom/extend-expect'; import '@testing-library/jest-dom/extend-expect';
import { render, screen } from '@testing-library/react'; import { render, screen } from '@testing-library/react';
import React from 'react'; import React from 'react';
@@ -33,6 +33,8 @@ jest.mock(
() => MockUnit, () => MockUnit,
); );
jest.mock('@edx/frontend-platform/analytics');
initializeMockApp(); initializeMockApp();
describe('CoursewareContainer', () => { describe('CoursewareContainer', () => {
@@ -261,6 +263,26 @@ describe('CoursewareContainer', () => {
expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId);
expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].id); expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].id);
}); });
it('should navigate between units and check block completion', async () => {
history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`);
const { container } = render(component);
axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/xmodule_handler/get_completion`).reply(200, {
complete: true,
});
// This is an important line that ensures the spinner has been removed - and thus our main
// content has been loaded - prior to proceeding with our expectations.
await waitForElementToBeRemoved(screen.getByRole('status'));
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button');
const sequenceNextButton = sequenceNavButtons[4];
expect(sequenceNextButton).toHaveTextContent('Next');
fireEvent.click(sequenceNavButtons[4]);
expect(global.location.href).toEqual(`http://localhost/course/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`);
});
}); });
describe('when the current sequence is an exam', () => { describe('when the current sequence is an exam', () => {

View File

@@ -60,7 +60,11 @@ export default function UnitNavigation(props) {
UnitNavigation.propTypes = { UnitNavigation.propTypes = {
sequenceId: PropTypes.string.isRequired, sequenceId: PropTypes.string.isRequired,
unitId: PropTypes.string.isRequired, unitId: PropTypes.string,
onClickPrevious: PropTypes.func.isRequired, onClickPrevious: PropTypes.func.isRequired,
onClickNext: PropTypes.func.isRequired, onClickNext: PropTypes.func.isRequired,
}; };
UnitNavigation.defaultProps = {
unitId: null,
};