Skip to content

Commit

Permalink
fix: ignore failed outline API call when learner has no access
Browse files Browse the repository at this point in the history
If the learner doesn't even have access to the course (e.g. because the
course starts in the future), don't worry about a 404 fetching the
course outline since we're not planning to use it anyway.

ENT-8078
  • Loading branch information
pwnage101 committed Jan 22, 2024
1 parent 4928f50 commit ab87167
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 26 deletions.
39 changes: 27 additions & 12 deletions src/course-home/data/redux.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,29 @@ describe('Data layer integration tests', () => {
expect(store.getState().courseHome.courseStatus).toEqual('failed');
});

it('should result in fetch failed if course metadata call errored', async () => {
const datesTabData = Factory.build('datesTabData');
const datesUrl = `${datesBaseUrl}/${courseId}`;

axiosMock.onGet(courseMetadataUrl).networkError();
axiosMock.onGet(datesUrl).reply(200, datesTabData);

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

expect(loggingService.logError).toHaveBeenCalled();
expect(store.getState().courseHome.courseStatus).toEqual('failed');
});

it('should result in fetch failed if course metadata call errored', async () => {
axiosMock.onGet(courseMetadataUrl).reply(200, courseHomeMetadata);
axiosMock.onGet(`${datesBaseUrl}/${courseId}`).networkError();

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

expect(loggingService.logError).toHaveBeenCalled();
expect(store.getState().courseHome.courseStatus).toEqual('failed');
});

it('Should fetch, normalize, and save metadata', async () => {
const datesTabData = Factory.build('datesTabData');

Expand All @@ -78,18 +101,14 @@ describe('Data layer integration tests', () => {
});

it.each([401, 403, 404])(
'should result in fetch denied for expected errors and failed for all others',
'should result in fetch denied if course access is denied, regardless of dates API status',
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);
expect(store.getState().courseHome.courseStatus).toEqual('denied');
},
);
});
Expand Down Expand Up @@ -129,18 +148,14 @@ describe('Data layer integration tests', () => {
});

it.each([401, 403, 404])(
'should result in fetch denied for expected errors and failed for all others',
'should result in fetch denied if course access is denied, regardless of outline API status',
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);
expect(store.getState().courseHome.courseStatus).toEqual('denied');
},
);
});
Expand Down
41 changes: 27 additions & 14 deletions src/course-home/data/thunks.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,41 @@ export function fetchTab(courseId, tab, getTabData, targetUserId) {
return async (dispatch) => {
dispatch(fetchTabRequest({ courseId }));
try {
const courseHomeCourseMetadata = await getCourseHomeCourseMetadata(courseId, 'outline');
dispatch(addModel({
modelType: 'courseHomeMeta',
model: {
id: courseId,
...courseHomeCourseMetadata,
},
}));
const tabDataResult = getTabData && await getTabData(courseId, targetUserId);
if (tabDataResult) {
const promisesToFulfill = [getCourseHomeCourseMetadata(courseId, 'outline')];
if (getTabData) {
promisesToFulfill.push(getTabData(courseId, targetUserId));
}
const [
courseHomeCourseMetadataResult,
tabDataResult,
] = await Promise.allSettled(promisesToFulfill);
if (courseHomeCourseMetadataResult.status === 'fulfilled') {
dispatch(addModel({
modelType: 'courseHomeMeta',
model: {
id: courseId,
...courseHomeCourseMetadataResult.value,
},
}));
}
if (tabDataResult?.status === 'fulfilled') {
dispatch(addModel({
modelType: tab,
model: {
id: courseId,
...tabDataResult,
...tabDataResult.value,
},
}));
}
// Disable the access-denied path for now - it caused a regression
if (!courseHomeCourseMetadata.courseAccess.hasAccess) {
if (courseHomeCourseMetadataResult.status === 'rejected') {
throw courseHomeCourseMetadataResult.reason;
} else if (!courseHomeCourseMetadataResult.value.courseAccess.hasAccess) {
// If the learner does not have access to the course, short cut to dispatch to a denied response regardless of
// the tabDataResult.
dispatch(fetchTabDenied({ courseId }));
} else if (tabDataResult || !getTabData) {
} else if (tabDataResult?.status === 'rejected') {
throw tabDataResult.reason;
} else {
dispatch(fetchTabSuccess({
courseId,
targetUserId,
Expand Down

0 comments on commit ab87167

Please sign in to comment.