Andal.LND/frontend-app-learning
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: handle courseware paths more liberally (#395)

Browse Source 
Valid courseware URLs currently include:
* /course/:courseId
* /course/:courseId/:sequenceId
* /course/:courseId/:sequenceId/:unitId

In this commit we add support for:
* /course/:courseId/:sectionId
* /course/:courseId/:sectionId/:unitId
* /course/:courseId/:unitId

All URL forms still redirect to:
  /course/:courseId/:sequenceId/:unitId

See ADR #8 for more context.

All changes:
* refactor: allow courseBlocks factory to build multiple sections
* refactor: make CoursewareContainer tests less brittle & stateful
* feat: handle courseware paths more liberally
* refactor: reorder, rename, & comment redirection functions

TNL-7796
This commit is contained in:
Kyle McCormick
2021-04-01 09:10:00 -04:00
committed by GitHub
parent 6a376b20c7
commit 353964e75c
12 changed files with 621 additions and 237 deletions
Download Patch File Download Diff File Expand all files Collapse all files

357
src/courseware/CoursewareContainer.test.jsx
View File

@@ -14,7 +14,7 @@ import tabMessages from '../tab-page/messages';
import { initializeMockApp } from '../setupTest';
import CoursewareContainer from './CoursewareContainer';
import { buildSimpleCourseBlocks } from '../shared/data/__factories__/courseBlocks.factory';
import { buildSimpleCourseBlocks, buildBinaryCourseBlocks } from '../shared/data/__factories__/courseBlocks.factory';
import initializeStore from '../store';
import { appendBrowserTimezoneToUrl } from '../utils';
@@ -43,6 +43,37 @@ describe('CoursewareContainer', () => {
let component;
let axiosMock;
// This is a standard set of data that can be used in CoursewareContainer tests.
// By default, `setUpMockRequests()` will configure the mock LMS API to return use this data.
// Certain test cases override these in order to test with special blocks/metadata.
const defaultCourseMetadata = Factory.build('courseMetadata');
const defaultCourseId = defaultCourseMetadata.id;
const defaultUnitBlocks = [
Factory.build(
'block',
{ type: 'vertical' },
{ courseId: defaultCourseId },
),
Factory.build(
'block',
{ type: 'vertical' },
{ courseId: defaultCourseId },
),
Factory.build(
'block',
{ type: 'vertical' },
{ courseId: defaultCourseId },
),
];
const {
courseBlocks: defaultCourseBlocks,
sequenceBlocks: [defaultSequenceBlock],
} = buildSimpleCourseBlocks(
defaultCourseId,
defaultCourseMetadata.name,
{ unitBlocks: defaultUnitBlocks },
);
beforeEach(() => {
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
@@ -66,6 +97,47 @@ describe('CoursewareContainer', () => {
);
});
function setUpMockRequests(options = {}) {
// If we weren't given course blocks or metadata, use the defaults.
const courseBlocks = options.courseBlocks || defaultCourseBlocks;
const courseMetadata = options.courseMetadata || defaultCourseMetadata;
const courseId = courseMetadata.id;
// If we weren't given a list of sequence metadatas for URL mocking,
// then construct it ourselves by looking at courseBlocks.
const sequenceMetadatas = options.sequenceMetadatas || (
Object.values(courseBlocks.blocks)
.filter(block => block.type === 'sequential')
.map(sequenceBlock => Factory.build(
'sequenceMetadata',
{},
{
courseId,
sequenceBlock,
unitBlocks: sequenceBlock.children.map(unitId => courseBlocks.blocks[unitId]),
},
))
);
const courseBlocksUrlRegExp = new RegExp(`${getConfig().LMS_BASE_URL}/api/courses/v2/blocks/*`);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks);
const courseMetadataUrl = appendBrowserTimezoneToUrl(`${getConfig().LMS_BASE_URL}/api/courseware/course/${courseId}`);
axiosMock.onGet(courseMetadataUrl).reply(200, courseMetadata);
sequenceMetadatas.forEach(sequenceMetadata => {
const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceMetadata.item_id}`;
axiosMock.onGet(sequenceMetadataUrl).reply(200, sequenceMetadata);
});
}
async function loadContainer() {
const { container } = render(component);
// Wait for the page spinner to be removed, such that we can wait for our main
// content to load before making any assertions.
await waitForElementToBeRemoved(screen.getByRole('status'));
return container;
}
it('should initialize to show a spinner', () => {
history.push('/course/abc123');
render(component);
@@ -78,13 +150,8 @@ describe('CoursewareContainer', () => {
});
describe('when receiving successful course data', () => {
let courseId;
let courseMetadata;
let courseBlocks;
let sequenceMetadata;
let sequenceBlock;
let unitBlocks;
const courseMetadata = defaultCourseMetadata;
const courseId = defaultCourseId;
function assertLoadedHeader(container) {
const courseHeader = container.querySelector('.course-header');
@@ -95,64 +162,27 @@ describe('CoursewareContainer', () => {
expect(courseHeader.querySelector('.course-title')).toHaveTextContent(courseMetadata.name);
}
function assertSequenceNavigation(container) {
function assertSequenceNavigation(container, expectedUnitCount = 3) {
// Ensure we had appropriate sequence navigation buttons. We should only have one unit.
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button');
expect(sequenceNavButtons).toHaveLength(5);
expect(sequenceNavButtons).toHaveLength(expectedUnitCount + 2);
expect(sequenceNavButtons[0]).toHaveTextContent('Previous');
// Prove this button is rendering an SVG tasks icon, meaning it's a unit/vertical.
expect(sequenceNavButtons[1].querySelector('svg')).toHaveClass('fa-tasks');
expect(sequenceNavButtons[4]).toHaveTextContent('Next');
}
function setupMockRequests() {
axiosMock.onGet(appendBrowserTimezoneToUrl(`${getConfig().LMS_BASE_URL}/api/courseware/course/${courseId}`)).reply(200, courseMetadata);
axiosMock.onGet(new RegExp(`${getConfig().LMS_BASE_URL}/api/courses/v2/blocks/*`)).reply(200, courseBlocks);
axiosMock.onGet(`${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceBlock.id}`).reply(200, sequenceMetadata);
expect(sequenceNavButtons[sequenceNavButtons.length - 1]).toHaveTextContent('Next');
}
beforeEach(async () => {
// On page load, SequenceContext attempts to scroll to the top of the page.
global.scrollTo = jest.fn();
courseMetadata = Factory.build('courseMetadata');
courseId = courseMetadata.id;
const customUnitBlocks = [
Factory.build(
'block',
{ type: 'vertical' },
{ courseId },
),
Factory.build(
'block',
{ type: 'vertical' },
{ courseId },
),
Factory.build(
'block',
{ type: 'vertical' },
{ courseId },
),
];
const result = buildSimpleCourseBlocks(courseId, courseMetadata.name, { unitBlocks: customUnitBlocks });
courseBlocks = result.courseBlocks;
unitBlocks = result.unitBlocks;
// eslint-disable-next-line prefer-destructuring
sequenceBlock = result.sequenceBlock[0];
sequenceMetadata = Factory.build(
'sequenceMetadata',
{},
{ courseId, unitBlocks, sequenceBlock },
);
setupMockRequests();
setUpMockRequests();
});
describe('when the URL only contains a course ID', () => {
const sequenceBlock = defaultSequenceBlock;
const unitBlocks = defaultUnitBlocks;
it('should use the resume block repsonse to pick a unit if it contains one', async () => {
axiosMock.onGet(`${getConfig().LMS_BASE_URL}/api/courseware/resume/${courseId}`).reply(200, {
sectionId: sequenceBlock.id,
@@ -160,11 +190,7 @@ describe('CoursewareContainer', () => {
});
history.push(`/course/${courseId}`);
const { container } = render(component);
// 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 container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container);
@@ -175,25 +201,19 @@ describe('CoursewareContainer', () => {
});
it('should use the first sequence ID and activeUnitIndex if the resume block response is empty', async () => {
// OVERRIDE SEQUENCE METADATA:
// set the position to the third unit so we can prove activeUnitIndex is working
sequenceMetadata = Factory.build(
const sequenceMetadata = Factory.build(
'sequenceMetadata',
{ position: 3 }, // position index is 1-based and is converted to 0-based for activeUnitIndex
{ courseId, unitBlocks, sequenceBlock },
);
setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] });
// Re-call the mock setup now that sequenceMetadata is different.
setupMockRequests();
// Note how there is no sectionId/unitId returned in this mock response!
axiosMock.onGet(`${getConfig().LMS_BASE_URL}/api/courseware/resume/${courseId}`).reply(200, {});
history.push(`/course/${courseId}`);
const { container } = render(component);
// 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 container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container);
@@ -204,14 +224,110 @@ describe('CoursewareContainer', () => {
});
});
describe('when the URL contains a section ID instead of a sequence ID', () => {
const {
courseBlocks, unitTree, sequenceTree, sectionTree,
} = buildBinaryCourseBlocks(
courseId, courseMetadata.name,
);
function setUrl(urlSequenceId, urlUnitId = null) {
history.push(`/course/${courseId}/${urlSequenceId}/${urlUnitId || ''}`);
}
function assertLocation(container, sequenceId, unitId) {
const expectedUrl = `http://localhost/course/${courseId}/${sequenceId}/${unitId}`;
expect(global.location.href).toEqual(expectedUrl);
expect(container.querySelector('.fake-unit')).toHaveTextContent(unitId);
}
beforeEach(async () => {
setUpMockRequests({ courseBlocks });
});
describe('when the URL contains a unit ID', () => {
it('should ignore the section ID and redirect based on the unit ID', async () => {
const urlUnit = unitTree[1][1][1];
setUrl(sectionTree[1].id, urlUnit.id);
const container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container, 2);
assertLocation(container, sequenceTree[1][1].id, urlUnit.id);
});
});
describe('when the URL does not contain a unit ID', () => {
it('should choose a unit within the section\'s first sequence', async () => {
setUrl(sectionTree[1].id);
const container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container, 2);
assertLocation(container, sequenceTree[1][0].id, unitTree[1][0][0].id);
});
});
describe('when the section is empty', () => {
// Make a (shallow-)copy of the course blocks.
// Remove all descendents of the second section.
const blocksWithEmptySection = { ...courseBlocks.blocks };
blocksWithEmptySection[sectionTree[1].id] = {
...sectionTree[1],
children: [],
};
sequenceTree[1].forEach(sequence => { delete blocksWithEmptySection[sequence.id]; });
unitTree[1].flat().forEach(unit => { delete blocksWithEmptySection[unit.id]; });
const courseBlocksWithEmptySection = {
...courseBlocks,
blocks: blocksWithEmptySection,
};
beforeEach(async () => {
setUpMockRequests({ courseBlocks: courseBlocksWithEmptySection });
});
it('should ignore the section ID and instead redirect to the course root', async () => {
setUrl(sectionTree[1].id);
await loadContainer();
expect(global.location.href).toEqual(`http://localhost/course/${courseId}`);
});
it('should ignore the section and unit IDs and instead to the course root', async () => {
// Specific unit ID used here shouldn't matter; is ignored due to empty section.
setUrl(sectionTree[1].id, unitTree[0][0][0]);
await loadContainer();
expect(global.location.href).toEqual(`http://localhost/course/${courseId}`);
});
});
});
describe('when the URL only contains a unit ID', () => {
const { courseBlocks, unitTree, sequenceTree } = buildBinaryCourseBlocks(courseId, courseMetadata.name);
beforeEach(async () => {
setUpMockRequests({ courseBlocks });
});
it('should insert the sequence ID into the URL', async () => {
const unit = unitTree[1][0][1];
history.push(`/course/${courseId}/${unit.id}`);
const container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container, 2);
const expectedSequenceId = sequenceTree[1][0].id;
const expectedUrl = `http://localhost/course/${courseId}/${expectedSequenceId}/${unit.id}`;
expect(global.location.href).toEqual(expectedUrl);
expect(container.querySelector('.fake-unit')).toHaveTextContent(unit.id);
});
});
describe('when the URL contains a course ID and sequence ID', () => {
const sequenceBlock = defaultSequenceBlock;
const unitBlocks = defaultUnitBlocks;
it('should pick the first unit if position was not defined (activeUnitIndex becomes 0)', async () => {
history.push(`/course/${courseId}/${sequenceBlock.id}`);
const { container } = render(component);
// 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 container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container);
@@ -222,22 +338,15 @@ describe('CoursewareContainer', () => {
});
it('should use activeUnitIndex to pick a unit from the sequence', async () => {
// OVERRIDE SEQUENCE METADATA:
sequenceMetadata = Factory.build(
const sequenceMetadata = Factory.build(
'sequenceMetadata',
{ position: 3 }, // position index is 1-based and is converted to 0-based for activeUnitIndex
{ courseId, unitBlocks, sequenceBlock },
);
// Re-call the mock setup now that sequenceMetadata is different.
setupMockRequests();
setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] });
history.push(`/course/${courseId}/${sequenceBlock.id}`);
const { container } = render(component);
// 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 container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container);
@@ -249,13 +358,12 @@ describe('CoursewareContainer', () => {
});
describe('when the URL contains a course, sequence, and unit ID', () => {
const sequenceBlock = defaultSequenceBlock;
const unitBlocks = defaultUnitBlocks;
it('should load the specified unit', async () => {
history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`);
const { container } = render(component);
// 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 container = await loadContainer();
assertLoadedHeader(container);
assertSequenceNavigation(container);
@@ -266,16 +374,12 @@ describe('CoursewareContainer', () => {
});
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'));
history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`);
const container = await loadContainer();
const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button');
const sequenceNextButton = sequenceNavButtons[4];
@@ -288,6 +392,8 @@ describe('CoursewareContainer', () => {
describe('when the current sequence is an exam', () => {
const { location } = window;
const sequenceBlock = defaultSequenceBlock;
const unitBlocks = defaultUnitBlocks;
beforeEach(() => {
delete window.location;
@@ -301,21 +407,15 @@ describe('CoursewareContainer', () => {
});
it('should redirect to the sequence lmsWebUrl', async () => {
// OVERRIDE SEQUENCE METADATA:
sequenceMetadata = Factory.build(
const sequenceMetadata = Factory.build(
'sequenceMetadata',
{ is_time_limited: true }, // position index is 1-based and is converted to 0-based for activeUnitIndex
{ courseId, unitBlocks, sequenceBlock },
);
setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] });
// Re-call the mock setup now that sequenceMetadata is different.
setupMockRequests();
history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`);
render(component);
// 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'));
await loadContainer();
expect(global.location.assign).toHaveBeenCalledWith(sequenceBlock.lms_web_url);
});
@@ -323,10 +423,8 @@ describe('CoursewareContainer', () => {
});
describe('when receiving a can_load_courseware error_code', () => {
let courseMetadata;
function setupWithDeniedStatus(errorCode) {
courseMetadata = Factory.build('courseMetadata', {
function setUpWithDeniedStatus(errorCode) {
const courseMetadata = Factory.build('courseMetadata', {
can_load_courseware: {
has_access: false,
error_code: errorCode,
@@ -334,66 +432,43 @@ describe('CoursewareContainer', () => {
},
});
const courseId = courseMetadata.id;
const { courseBlocks, unitBlocks, sequenceBlock } = buildSimpleCourseBlocks(courseId, courseMetadata.name);
const sequenceMetadata = Factory.build(
'sequenceMetadata',
{},
{ courseId, unitBlocks, sequenceBlock },
);
let forbiddenCourseUrl = `${getConfig().LMS_BASE_URL}/api/courseware/course/${courseId}`;
forbiddenCourseUrl = appendBrowserTimezoneToUrl(forbiddenCourseUrl);
const courseBlocksUrlRegExp = new RegExp(`${getConfig().LMS_BASE_URL}/api/courses/v2/blocks/*`);
const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceBlock.id}`;
axiosMock.onGet(forbiddenCourseUrl).reply(200, courseMetadata);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks);
axiosMock.onGet(sequenceMetadataUrl).reply(200, sequenceMetadata);
const { courseBlocks } = buildSimpleCourseBlocks(courseId, courseMetadata.name);
setUpMockRequests({ courseBlocks, courseMetadata });
history.push(`/course/${courseId}`);
return courseMetadata;
}
it('should go to course home for an enrollment_required error code', async () => {
setupWithDeniedStatus('enrollment_required');
render(component);
await waitForElementToBeRemoved(screen.getByRole('status'));
const courseMetadata = setUpWithDeniedStatus('enrollment_required');
await loadContainer();
expect(global.location.href).toEqual(`http://localhost/redirect/course-home/${courseMetadata.id}`);
});
it('should go to course home for an authentication_required error code', async () => {
setupWithDeniedStatus('authentication_required');
render(component);
await waitForElementToBeRemoved(screen.getByRole('status'));
const courseMetadata = setUpWithDeniedStatus('authentication_required');
await loadContainer();
expect(global.location.href).toEqual(`http://localhost/redirect/course-home/${courseMetadata.id}`);
});
it('should go to dashboard for an unfulfilled_milestones error code', async () => {
setupWithDeniedStatus('unfulfilled_milestones');
render(component);
await waitForElementToBeRemoved(screen.getByRole('status'));
setUpWithDeniedStatus('unfulfilled_milestones');
await loadContainer();
expect(global.location.href).toEqual('http://localhost/redirect/dashboard');
});
it('should go to the dashboard with an attached access_response_error for an audit_expired error code', async () => {
setupWithDeniedStatus('audit_expired');
render(component);
await waitForElementToBeRemoved(screen.getByRole('status'));
setUpWithDeniedStatus('audit_expired');
await loadContainer();
expect(global.location.href).toEqual('http://localhost/redirect/dashboard?access_response_error=uhoh%20oh%20no');
});
it('should go to the dashboard with a notlive start date for a course_not_started error code', async () => {
setupWithDeniedStatus('course_not_started');
render(component);
await waitForElementToBeRemoved(screen.getByRole('status'));
setUpWithDeniedStatus('course_not_started');
await loadContainer();
const startDate = '2/5/2013'; // This date is based on our courseMetadata factory's sample data.
expect(global.location.href).toEqual(`http://localhost/redirect/dashboard?notlive=${startDate}`);