Skip to content

Commit

Permalink
fix: gracefully handle 403 responses in tab loading (#1111)
Browse files Browse the repository at this point in the history
  • Loading branch information
jansenk authored May 24, 2023
1 parent fc8f5d4 commit 79b65da
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Factory.define('outlineTabData')
upgrade_url: `${host}/dashboard`,
}))
.attrs({
course_access_redirect: false,
has_scheduled_content: null,
access_expiration: null,
can_show_upgrade_sock: false,
Expand Down
31 changes: 28 additions & 3 deletions src/course-home/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,18 @@ export async function getDatesTabData(courseId) {
const { data } = await getAuthenticatedHttpClient().get(url);
return camelCaseObject(data);
} catch (error) {
const { httpErrorStatus } = error && error.customAttributes;
const httpErrorStatus = error?.response?.status;
if (httpErrorStatus === 401) {
// 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 {};
}
if (httpErrorStatus === 403) {
// The backend sends this if there is a course access error and the user should be redirected. The redirect
// info is included in the course metadata request and will be handled there as long as this call returns
// without an error
return {};
}
throw error;
}
}
Expand Down Expand Up @@ -259,7 +265,7 @@ export async function getProgressTabData(courseId, targetUserId) {

return camelCasedData;
} catch (error) {
const { httpErrorStatus } = error && error.customAttributes;
const httpErrorStatus = error?.response?.status;
if (httpErrorStatus === 404) {
global.location.replace(`${getConfig().LMS_BASE_URL}/courses/${courseId}/progress`);
return {};
Expand All @@ -269,6 +275,12 @@ export async function getProgressTabData(courseId, targetUserId) {
// courseAccess in the metadata call, so just ignore this status for now.
return {};
}
if (httpErrorStatus === 403) {
// The backend sends this if there is a course access error and the user should be redirected. The redirect
// info is included in the course metadata request and will be handled there as long as this call returns
// without an error
return {};
}
throw error;
}
}
Expand Down Expand Up @@ -322,7 +334,20 @@ export function getTimeOffsetMillis(headerDate, requestTime, responseTime) {
export async function getOutlineTabData(courseId) {
const url = `${getConfig().LMS_BASE_URL}/api/course_home/outline/${courseId}`;
const requestTime = Date.now();
const tabData = await getAuthenticatedHttpClient().get(url);
let tabData;
try {
tabData = await getAuthenticatedHttpClient().get(url);
} catch (error) {
const httpErrorStatus = error?.response?.status;
if (httpErrorStatus === 403) {
// The backend sends this if there is a course access error and the user should be redirected. The redirect
// info is included in the course metadata request and will be handled there as long as this call returns
// without an error
return {};
}
throw error;
}

const responseTime = Date.now();

const {
Expand Down
62 changes: 59 additions & 3 deletions src/course-home/data/redux.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ describe('Data layer integration tests', () => {
let courseMetadataUrl = `${getConfig().LMS_BASE_URL}/api/course_home/course_metadata/${courseId}`;
courseMetadataUrl = appendBrowserTimezoneToUrl(courseMetadataUrl);

const courseHomeAccessDeniedMetadata = Factory.build(
'courseHomeMetadata',
{
id: courseId,
course_access: {
has_access: false,
error_code: 'bad codes',
additional_context_user_message: 'your Codes Are BAD',
},
},
);

let store;

beforeEach(() => {
Expand Down Expand Up @@ -57,14 +69,31 @@ describe('Data layer integration tests', () => {
expect(state.courseHome.courseStatus).toEqual('loaded');
expect(state).toMatchSnapshot();
});

it.each([401, 403, 404])(
'should result in fetch denied for expected errors and failed for all others',
async (errorStatus) => {
axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeAccessDeniedMetadata);
axiosMock.onGet(`${datesBaseUrl}/${courseId}`).reply(errorStatus, {});

await executeThunk(thunks.fetchDatesTab(courseId), store.dispatch);

let expectedState = 'failed';
if (errorStatus === 401 || errorStatus === 403) {
expectedState = 'denied';
}
expect(store.getState().courseHome.courseStatus).toEqual(expectedState);
},
);
});

describe('Test fetchOutlineTab', () => {
const outlineBaseUrl = `${getConfig().LMS_BASE_URL}/api/course_home/outline`;
const outlineUrl = `${outlineBaseUrl}/${courseId}`;

it('Should result in fetch failure if error occurs', async () => {
axiosMock.onGet(courseMetadataUrl).networkError();
axiosMock.onGet(`${outlineBaseUrl}/${courseId}`).networkError();
axiosMock.onGet(outlineUrl).networkError();

await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch);

Expand All @@ -75,8 +104,6 @@ describe('Data layer integration tests', () => {
it('Should fetch, normalize, and save metadata', async () => {
const outlineTabData = Factory.build('outlineTabData', { courseId });

const outlineUrl = `${outlineBaseUrl}/${courseId}`;

axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeMetadata);
axiosMock.onGet(outlineUrl).reply(200, outlineTabData);

Expand All @@ -86,6 +113,22 @@ describe('Data layer integration tests', () => {
expect(state.courseHome.courseStatus).toEqual('loaded');
expect(state).toMatchSnapshot();
});

it.each([401, 403, 404])(
'should result in fetch denied for expected errors and failed for all others',
async (errorStatus) => {
axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeAccessDeniedMetadata);
axiosMock.onGet(outlineUrl).reply(errorStatus, {});

await executeThunk(thunks.fetchOutlineTab(courseId), store.dispatch);

let expectedState = 'failed';
if (errorStatus === 403) {
expectedState = 'denied';
}
expect(store.getState().courseHome.courseStatus).toEqual(expectedState);
},
);
});

describe('Test fetchProgressTab', () => {
Expand Down Expand Up @@ -129,6 +172,19 @@ describe('Data layer integration tests', () => {
const state = store.getState();
expect(state.courseHome.targetUserId).toEqual(2);
});

it.each([401, 403, 404])(
'should result in fetch denied for expected errors and failed for all others',
async (errorStatus) => {
const progressUrl = `${progressBaseUrl}/${courseId}`;
axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeAccessDeniedMetadata);
axiosMock.onGet(progressUrl).reply(errorStatus, {});

await executeThunk(thunks.fetchProgressTab(courseId), store.dispatch);

expect(store.getState().courseHome.courseStatus).toEqual('denied');
},
);
});

describe('Test saveCourseGoal', () => {
Expand Down

0 comments on commit 79b65da

Please sign in to comment.