From 18537e3f6242cd38c146781c59f470a55a154c6d Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Fri, 2 Feb 2024 11:21:44 -0500 Subject: [PATCH] fix: model update for usage locations (#819) --- src/files-and-videos/videos-page/data/api.js | 21 +++++++----- .../videos-page/data/api.test.js | 33 ++++++++++++++++++- .../videos-page/data/thunks.js | 26 +++------------ 3 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/files-and-videos/videos-page/data/api.js b/src/files-and-videos/videos-page/data/api.js index a7137679f6..43a858526b 100644 --- a/src/files-and-videos/videos-page/data/api.js +++ b/src/files-and-videos/videos-page/data/api.js @@ -31,19 +31,22 @@ export async function getVideos(courseId) { }; } -export async function getAllUsagePaths({ courseId, videos, updateModel }) { - const apiPromises = videos.map(video => getAuthenticatedHttpClient() - .get(`${getVideosUrl(courseId)}/${video.id}/usage`, { videoId: video.id })); +export async function getAllUsagePaths({ courseId, videoIds }) { + const apiPromises = videoIds.map(id => getAuthenticatedHttpClient() + .get(`${getVideosUrl(courseId)}/${id}/usage`, { videoId: id })); + const updatedUsageLocations = []; const results = await Promise.allSettled(apiPromises); + results.forEach(result => { - const data = camelCaseObject(result.value.data); - const { videoId } = result.value.config; - if (data) { - updateModel(data, videoId); - } else { - updateModel({ usageLocation: [] }, videoId); + const value = camelCaseObject(result.value); + if (value) { + const { usageLocations } = value.data; + const activeStatus = usageLocations?.length > 0 ? 'active' : 'inactive'; + const { videoId } = value.config; + updatedUsageLocations.push({ id: videoId, usageLocations, activeStatus }); } }); + return updatedUsageLocations; } /** diff --git a/src/files-and-videos/videos-page/data/api.test.js b/src/files-and-videos/videos-page/data/api.test.js index 134ec5b252..605edbb4ab 100644 --- a/src/files-and-videos/videos-page/data/api.test.js +++ b/src/files-and-videos/videos-page/data/api.test.js @@ -3,7 +3,7 @@ import MockAdapter from 'axios-mock-adapter'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { getDownload, getVideosUrl } from './api'; +import { getDownload, getVideosUrl, getAllUsagePaths } from './api'; jest.mock('file-saver'); @@ -72,4 +72,35 @@ describe('api.js', () => { }); }); }); + describe('getAllUsagePaths', () => { + const courseId = 'random-course-id'; + const videoIds = ['test1']; + it('returns empty array when no videos are given', async () => { + const expected = []; + const actual = await getAllUsagePaths({ courseId, videoIds: [] }); + expect(actual).toEqual(expected); + }); + it('pushes an empty usageLocations field when video api call fails', async () => { + axiosMock.onGet(`${getVideosUrl(courseId)}/${videoIds[0]}/usage`, { videoId: videoIds[0] }).reply(404); + const expected = []; + const actual = await getAllUsagePaths({ courseId, videoIds }); + expect(actual).toEqual(expected); + }); + it('sets activeStatus to active', async () => { + const usageLocations = [{ link: '/test', name: 'test' }]; + axiosMock.onGet(`${getVideosUrl(courseId)}/${videoIds[0]}/usage`, { videoId: videoIds[0] }) + .reply(200, { usageLocations }); + const expected = [{ id: videoIds[0], usageLocations, activeStatus: 'active' }]; + const actual = await getAllUsagePaths({ courseId, videoIds }); + expect(actual).toEqual(expected); + }); + it('sets activeStatus to inactive', async () => { + const usageLocations = []; + axiosMock.onGet(`${getVideosUrl(courseId)}/${videoIds[0]}/usage`, { videoId: videoIds[0] }) + .reply(200, { usageLocations }); + const expected = [{ id: videoIds[0], usageLocations, activeStatus: 'inactive' }]; + const actual = await getAllUsagePaths({ courseId, videoIds }); + expect(actual).toEqual(expected); + }); + }); }); diff --git a/src/files-and-videos/videos-page/data/thunks.js b/src/files-and-videos/videos-page/data/thunks.js index 570f182708..12c8937282 100644 --- a/src/files-and-videos/videos-page/data/thunks.js +++ b/src/files-and-videos/videos-page/data/thunks.js @@ -5,6 +5,7 @@ import { addModels, removeModel, updateModel, + updateModels, } from '../../../generic/model-store'; import { addThumbnail, @@ -38,19 +39,6 @@ import { import { updateFileValues } from './utils'; -function updateUsageLocation(videoId, dispatch, usageLocations) { - const activeStatus = usageLocations?.length > 0 ? 'active' : 'inactive'; - - dispatch(updateModel({ - modelType: 'videos', - model: { - id: videoId, - usageLocations, - activeStatus, - }, - })); -} - export function fetchVideos(courseId) { return async (dispatch) => { dispatch(updateLoadingStatus({ courseId, status: RequestStatus.IN_PROGRESS })); @@ -65,16 +53,12 @@ export function fetchVideos(courseId) { dispatch(updateLoadingStatus({ courseId, status: RequestStatus.SUCCESSFUL })); } else { const parsedVideos = updateFileValues(previousUploads); + const videoIds = parsedVideos.map(video => video.id); dispatch(addModels({ modelType: 'videos', models: parsedVideos })); - dispatch(setVideoIds({ - videoIds: parsedVideos.map(video => video.id), - })); + dispatch(setVideoIds({ videoIds })); dispatch(updateLoadingStatus({ courseId, status: RequestStatus.PARTIAL })); - await getAllUsagePaths({ - courseId, - videos: parsedVideos, - updateModel: (apiData, videoId) => updateUsageLocation(videoId, dispatch, apiData.usageLocations), - }); + const allUsageLocations = await getAllUsagePaths({ courseId, videoIds }); + dispatch(updateModels({ modelType: 'videos', models: allUsageLocations })); dispatch(updateLoadingStatus({ courseId, status: RequestStatus.SUCCESSFUL })); } } catch (error) {