From ab871670527a6773d7e9e4d769ceca72b808894e Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Thu, 21 Dec 2023 12:46:14 -0800 Subject: [PATCH] fix: ignore failed outline API call when learner has no access 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 --- src/course-home/data/redux.test.js | 39 +++++++++++++++++++--------- src/course-home/data/thunks.js | 41 ++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/src/course-home/data/redux.test.js b/src/course-home/data/redux.test.js index cf03fffd36..b46433927a 100644 --- a/src/course-home/data/redux.test.js +++ b/src/course-home/data/redux.test.js @@ -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'); @@ -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'); }, ); }); @@ -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'); }, ); }); diff --git a/src/course-home/data/thunks.js b/src/course-home/data/thunks.js index 6e0d972530..ec0567f9cf 100644 --- a/src/course-home/data/thunks.js +++ b/src/course-home/data/thunks.js @@ -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,