From d76aaa73a44cbb42d77181bcf9fbb15b7cd3447d Mon Sep 17 00:00:00 2001 From: connorhaugh <49422820+connorhaugh@users.noreply.github.com> Date: Wed, 13 Mar 2024 09:37:55 -0400 Subject: [PATCH] feat: add progress bar for video uploads and refactor (#860) * feat: add progress bar for video uploads and refactor --------- Co-authored-by: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> --- .../files-page/data/thunks.js | 1 - src/files-and-videos/generic/FileTable.jsx | 20 ++-- .../videos-page/AddVideoProgressBarToast.jsx | 35 ++++++ .../videos-page/VideosPage.jsx | 5 + .../videos-page/VideosPage.test.jsx | 3 +- src/files-and-videos/videos-page/data/api.js | 48 +++----- .../videos-page/data/api.test.js | 34 +++++- .../videos-page/data/slice.js | 7 +- .../videos-page/data/thunks.js | 78 +++++++----- .../videos-page/data/thunks.test.js | 112 ++++++++++++++++++ .../factories/mockApiResponses.jsx | 1 + src/files-and-videos/videos-page/messages.js | 4 + 12 files changed, 277 insertions(+), 71 deletions(-) create mode 100644 src/files-and-videos/videos-page/AddVideoProgressBarToast.jsx create mode 100644 src/files-and-videos/videos-page/data/thunks.test.js diff --git a/src/files-and-videos/files-page/data/thunks.js b/src/files-and-videos/files-page/data/thunks.js index ded413170..1aafc604f 100644 --- a/src/files-and-videos/files-page/data/thunks.js +++ b/src/files-and-videos/files-page/data/thunks.js @@ -161,7 +161,6 @@ export function resetErrors({ errorType }) { export function getUsagePaths({ asset, courseId }) { return async (dispatch) => { dispatch(updateEditStatus({ editType: 'usageMetrics', status: RequestStatus.IN_PROGRESS })); - try { const { usageLocations } = await getAssetUsagePaths({ assetId: asset.id, courseId }); const assetLocations = usageLocations[asset.id]; diff --git a/src/files-and-videos/generic/FileTable.jsx b/src/files-and-videos/generic/FileTable.jsx index 9907164ef..ea98b0b1a 100644 --- a/src/files-and-videos/generic/FileTable.jsx +++ b/src/files-and-videos/generic/FileTable.jsx @@ -244,14 +244,18 @@ const FileTable = ({ setSelectedRows={setSelectedRows} fileType={fileType} /> - + { + fileType !== 'video' && ( + + ) + } { + let isOpen = false; + useEffect(() => { + isOpen = !!uploadVideoProgress; + }, [uploadVideoProgress]); + + return ( + + {intl.formatMessage(messages.videoUploadProgressBarLabel)} + + + ); +}; + +AddVideoProgressBarToast.defaultProps = { + uploadVideoProgress: 0, +}; +AddVideoProgressBarToast.propTypes = { + uploadVideoProgress: PropTypes.number, + // injected + intl: intlShape.isRequired, +}; + +export default injectIntl(AddVideoProgressBarToast); diff --git a/src/files-and-videos/videos-page/VideosPage.jsx b/src/files-and-videos/videos-page/VideosPage.jsx index 662577c65..d34a45514 100644 --- a/src/files-and-videos/videos-page/VideosPage.jsx +++ b/src/files-and-videos/videos-page/VideosPage.jsx @@ -38,6 +38,7 @@ import { ThumbnailColumn, TranscriptColumn, } from '../generic'; +import AddVideoProgressBarToast from './AddVideoProgressBarToast'; import TranscriptSettings from './transcript-settings'; import VideoThumbnail from './VideoThumbnail'; import { getFormattedDuration, resampleFile } from './data/utils'; @@ -62,6 +63,7 @@ const VideosPage = ({ videoIds, loadingStatus, transcriptStatus, + uploadNewVideoProgress, addingStatus: addVideoStatus, deletingStatus: deleteVideoStatus, updatingStatus: updateVideoStatus, @@ -190,6 +192,9 @@ const VideosPage = ({ return ( + { axiosMock.onPost(getCourseVideosApiUrl(courseId)).reply(204, generateNewVideoApiResponse()); axiosMock.onGet(getCourseVideosApiUrl(courseId)).reply(200, generateAddVideoApiResponse()); - Object.defineProperty(dropzone, 'files', { value: [file], }); fireEvent.drop(dropzone); - await executeThunk(addVideoFile(courseId, file), store.dispatch); + await executeThunk(addVideoFile(courseId, file, []), store.dispatch); }); const addStatus = store.getState().videos.addingStatus; expect(addStatus).toEqual(RequestStatus.SUCCESSFUL); diff --git a/src/files-and-videos/videos-page/data/api.js b/src/files-and-videos/videos-page/data/api.js index 43a858526..fed81c0cc 100644 --- a/src/files-and-videos/videos-page/data/api.js +++ b/src/files-and-videos/videos-page/data/api.js @@ -180,21 +180,30 @@ export async function addVideo(courseId, file) { const postJson = { files: [{ file_name: file.name, content_type: file.type }], }; - - const { data } = await getAuthenticatedHttpClient() + const response = await getAuthenticatedHttpClient() .post(getCourseVideosApiUrl(courseId), postJson); - return camelCaseObject(data); + return { data: camelCaseObject(response.data), ...response }; +} + +export async function sendVideoUploadStatus( + courseId, + edxVideoId, + message, + status, +) { + return getAuthenticatedHttpClient() + .post(getCourseVideosApiUrl(courseId), [{ + edxVideoId, + message, + status, + }]); } export async function uploadVideo( - courseId, uploadUrl, uploadFile, - edxVideoId, ) { - const uploadErrors = []; - - await fetch(uploadUrl, { + return fetch(uploadUrl, { method: 'PUT', headers: { 'Content-Disposition': `attachment; filename="${uploadFile.name}"`, @@ -202,28 +211,7 @@ export async function uploadVideo( }, multipart: false, body: uploadFile, - }) - .then(async (response) => { - if (!response.ok) { - throw new Error(); - } - await getAuthenticatedHttpClient() - .post(getCourseVideosApiUrl(courseId), [{ - edxVideoId, - message: 'Upload completed', - status: 'upload_completed', - }]); - }) - .catch(async () => { - uploadErrors.push(`Failed to upload ${uploadFile.name} to server.`); - await getAuthenticatedHttpClient() - .post(getCourseVideosApiUrl(courseId), [{ - edxVideoId, - message: 'Upload failed', - status: 'upload_failed', - }]); - }); - return uploadErrors; + }); } export async function deleteTranscriptPreferences(courseId) { 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 605edbb4a..20a504aaf 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,9 @@ import MockAdapter from 'axios-mock-adapter'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { getDownload, getVideosUrl, getAllUsagePaths } from './api'; +import { + getDownload, getVideosUrl, getAllUsagePaths, getCourseVideosApiUrl, uploadVideo, sendVideoUploadStatus, +} from './api'; jest.mock('file-saver'); @@ -103,4 +105,34 @@ describe('api.js', () => { expect(actual).toEqual(expected); }); }); + + describe('uploadVideo', () => { + it('PUTs to the provided URL', async () => { + const mockUrl = 'mock.com'; + const mockFile = { mock: 'file' }; + const expectedResult = 'Something'; + global.fetch = jest.fn().mockResolvedValue(expectedResult); + const actual = await uploadVideo(mockUrl, mockFile); + expect(actual).toEqual(expectedResult); + }); + }); + describe('sendVideoUploadStatus', () => { + it('Posts to the correct url', async () => { + const mockCourseId = 'wiZard101'; + const mockEdxVideoId = 'wIzOz.mp3'; + const mockStatus = 'Im mElTinG'; + const mockMessage = 'DinG DOng The WiCked WiTCH isDead'; + const expectedResult = 'Something'; + axiosMock.onPost(`${getCourseVideosApiUrl(mockCourseId)}`) + .reply(200, expectedResult); + const actual = await sendVideoUploadStatus( + mockCourseId, + mockEdxVideoId, + mockMessage, + mockStatus, + ); + expect(actual.data).toEqual(expectedResult); + jest.clearAllMocks(); + }); + }); }); diff --git a/src/files-and-videos/videos-page/data/slice.js b/src/files-and-videos/videos-page/data/slice.js index 74574660f..94c0d078c 100644 --- a/src/files-and-videos/videos-page/data/slice.js +++ b/src/files-and-videos/videos-page/data/slice.js @@ -11,6 +11,7 @@ const slice = createSlice({ loadingStatus: RequestStatus.IN_PROGRESS, updatingStatus: '', addingStatus: '', + uploadNewVideoProgress: 0, deletingStatus: '', usageStatus: '', transcriptStatus: '', @@ -62,9 +63,12 @@ const slice = createSlice({ deleteVideoSuccess: (state, { payload }) => { state.videoIds = state.videoIds.filter(id => id !== payload.videoId); }, - addVideoSuccess: (state, { payload }) => { + addVideoById: (state, { payload }) => { state.videoIds = [payload.videoId, ...state.videoIds]; }, + updateVideoUploadProgress: (state, { payload }) => { + state.uploadNewVideoProgress = payload.uploadNewVideoProgress; + }, updateTranscriptCredentialsSuccess: (state, { payload }) => { const { provider } = payload; state.pageSettings.transcriptCredentials = { @@ -102,6 +106,7 @@ export const { updateEditStatus, updateTranscriptCredentialsSuccess, updateTranscriptPreferenceSuccess, + updateVideoUploadProgress, } = slice.actions; export const { diff --git a/src/files-and-videos/videos-page/data/thunks.js b/src/files-and-videos/videos-page/data/thunks.js index 12c893728..695983bc7 100644 --- a/src/files-and-videos/videos-page/data/thunks.js +++ b/src/files-and-videos/videos-page/data/thunks.js @@ -20,6 +20,7 @@ import { uploadTranscript, getVideoUsagePaths, deleteTranscriptPreferences, + sendVideoUploadStatus, setTranscriptCredentials, setTranscriptPreferences, getAllUsagePaths, @@ -29,12 +30,12 @@ import { setPageSettings, updateLoadingStatus, deleteVideoSuccess, - addVideoSuccess, updateErrors, clearErrors, updateEditStatus, updateTranscriptCredentialsSuccess, updateTranscriptPreferenceSuccess, + updateVideoUploadProgress, } from './slice'; import { updateFileValues } from './utils'; @@ -42,7 +43,6 @@ import { updateFileValues } from './utils'; export function fetchVideos(courseId) { return async (dispatch) => { dispatch(updateLoadingStatus({ courseId, status: RequestStatus.IN_PROGRESS })); - try { const { previousUploads, ...data } = await getVideos(courseId); dispatch(setPageSettings({ ...data })); @@ -87,7 +87,6 @@ export function updateVideoOrder(courseId, videoIds) { export function deleteVideoFile(courseId, id) { return async (dispatch) => { dispatch(updateEditStatus({ editType: 'delete', status: RequestStatus.IN_PROGRESS })); - try { await deleteVideo(courseId, id); dispatch(deleteVideoSuccess({ videoId: id })); @@ -103,42 +102,65 @@ export function deleteVideoFile(courseId, id) { export function addVideoFile(courseId, file, videoIds) { return async (dispatch) => { dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.IN_PROGRESS })); - try { - const { files } = await addVideo(courseId, file); - const { edxVideoId, uploadUrl } = files[0]; - const errors = await uploadVideo( - courseId, - uploadUrl, - file, - edxVideoId, - ); - const { videos } = await fetchVideoList(courseId); - const newVideos = videos.filter(video => !videoIds.includes(video.edxVideoId)); - const parsedVideos = updateFileValues(newVideos, true); - dispatch(addModels({ - modelType: 'videos', - models: parsedVideos, - })); - dispatch(addVideoSuccess({ - videoId: edxVideoId, - })); - dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.SUCCESSFUL })); - if (!isEmpty(errors)) { - errors.forEach(error => { - dispatch(updateErrors({ error: 'add', message: error })); - }); + const createUrlResponse = await addVideo(courseId, file); + if (createUrlResponse.status < 200 && createUrlResponse.status >= 300) { + dispatch(updateErrors({ error: 'add', message: `Failed to add ${file.name}.` })); dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED })); + return; + } + const { edxVideoId, uploadUrl } = createUrlResponse.data.files[0]; + const putToServerResponse = await uploadVideo(uploadUrl, file); + if (putToServerResponse.status < 200 && putToServerResponse.status >= 300) { + dispatch(updateErrors({ error: 'add', message: `Failed to upload ${file.name}.` })); + sendVideoUploadStatus(courseId, edxVideoId, 'Upload failed', 'upload_failed'); + dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED })); + return; + } + if (putToServerResponse.body) { + const reader = putToServerResponse.body.getReader(); + const contentLength = +putToServerResponse.headers.get('Content-Length'); + let loaded = 0; + // eslint-disable-next-line no-constant-condition + while (true) { + // eslint-disable-next-line no-await-in-loop + const { done, value } = await reader.read(); + if (done) { + dispatch(updateVideoUploadProgress({ uploadNewVideoProgress: 100 })); + break; + } + loaded += value.byteLength; + const progress = Math.round((loaded / contentLength) * 100); + dispatch(updateVideoUploadProgress({ uploadNewVideoProgress: progress })); + } + + dispatch(updateVideoUploadProgress({ uploadNewVideoProgress: 0 })); + sendVideoUploadStatus(courseId, edxVideoId, 'Upload completed', 'upload_completed'); } } catch (error) { if (error.response && error.response.status === 413) { const message = error.response.data.error; dispatch(updateErrors({ error: 'add', message })); } else { - dispatch(updateErrors({ error: 'add', message: `Failed to add ${file.name}.` })); + dispatch(updateErrors({ error: 'add', message: `Failed to upload ${file.name}.` })); } + dispatch(updateVideoUploadProgress({ uploadNewVideoProgress: 0 })); dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED })); + return; } + try { + const { videos } = await fetchVideoList(courseId); + const newVideos = videos.filter(video => !videoIds.includes(video.edxVideoId)); + const newVideoIds = newVideos.map(video => video.edxVideoId); + const parsedVideos = updateFileValues(newVideos, true); + dispatch(addModels({ modelType: 'videos', models: parsedVideos })); + dispatch(setVideoIds({ videoIds: videoIds.concat(newVideoIds) })); + } catch (error) { + dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED })); + dispatch(updateErrors({ error: 'add', message: error.message })); + return; + } + dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.SUCCESSFUL })); }; } diff --git a/src/files-and-videos/videos-page/data/thunks.test.js b/src/files-and-videos/videos-page/data/thunks.test.js new file mode 100644 index 000000000..c295dd085 --- /dev/null +++ b/src/files-and-videos/videos-page/data/thunks.test.js @@ -0,0 +1,112 @@ +import { addVideoFile } from './thunks'; +import * as api from './api'; + +describe('addVideoFile', () => { + const dispatch = jest.fn(); + const getState = jest.fn(); + const courseId = 'course-123'; + const mockFile = { + name: 'mockName', + + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + it('Should dispatch failed status if url cannot be created.', async () => { + jest.spyOn(api, 'addVideo').mockResolvedValue({ + status: 404, + }); + + await addVideoFile(courseId, mockFile)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { + editType: 'add', + status: 'failed', + }, + type: 'videos/updateEditStatus', + }); + }); + it('Failed video upload dispatches updateEditStatus with failed', async () => { + jest.spyOn(api, 'addVideo').mockResolvedValue({ + status: 200, + data: { + files: [ + { edxVideoId: 'iD', uploadUrl: 'a Url' }, + ], + }, + }); + jest.spyOn(api, 'uploadVideo').mockResolvedValue({ + status: 404, + }); + await addVideoFile(courseId, mockFile)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { + editType: 'add', + status: 'failed', + }, + type: 'videos/updateEditStatus', + }); + }); + it('should handle successful upload with progress bar', async () => { + const mockPutToServerResponse = { + body: { + getReader: jest.fn(() => ({ + read: jest.fn().mockResolvedValueOnce({ done: true }), + })), + }, + headers: new Map([['Content-Length', '100']]), + }; + jest.spyOn(api, 'addVideo').mockResolvedValue({ + status: 200, + data: { + files: [ + { edxVideoId: 'iD', uploadUrl: 'a Url' }, + ], + }, + }); + jest.spyOn(api, 'sendVideoUploadStatus').mockResolvedValue({ status: 200 }); + jest.spyOn(api, 'uploadVideo').mockResolvedValue(mockPutToServerResponse); + + await addVideoFile(courseId, mockFile)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { uploadNewVideoProgress: 100 }, + type: 'videos/updateVideoUploadProgress', + }); + }); + it('should handle successful upload with progress bar', async () => { + const mockPutToServerResponse = { + body: { + getReader: jest.fn(() => ({ + read: jest.fn().mockResolvedValueOnce({ + value: { + byteLength: 50, + + }, + }), + })), + }, + headers: new Map([['Content-Length', '100']]), + }; + jest.spyOn(api, 'addVideo').mockResolvedValue({ + status: 200, + data: { + files: [ + { edxVideoId: 'iD', uploadUrl: 'a Url' }, + ], + }, + }); + jest.spyOn(api, 'sendVideoUploadStatus').mockResolvedValue({ status: 200 }); + jest.spyOn(api, 'uploadVideo').mockResolvedValue(mockPutToServerResponse); + + await addVideoFile(courseId, mockFile)(dispatch, getState); + + expect(dispatch).toHaveBeenCalledWith({ + payload: { uploadNewVideoProgress: 50 }, + type: 'videos/updateVideoUploadProgress', + }); + }); +}); diff --git a/src/files-and-videos/videos-page/factories/mockApiResponses.jsx b/src/files-and-videos/videos-page/factories/mockApiResponses.jsx index be3d93941..39e38cbfe 100644 --- a/src/files-and-videos/videos-page/factories/mockApiResponses.jsx +++ b/src/files-and-videos/videos-page/factories/mockApiResponses.jsx @@ -213,6 +213,7 @@ export const generateFetchVideosApiResponse = () => ({ }); export const generateAddVideoApiResponse = () => ({ + ok: true, videos: [ { edx_video_id: 'mOckID4', diff --git a/src/files-and-videos/videos-page/messages.js b/src/files-and-videos/videos-page/messages.js index db10abaf7..774dfe2f0 100644 --- a/src/files-and-videos/videos-page/messages.js +++ b/src/files-and-videos/videos-page/messages.js @@ -37,6 +37,10 @@ const messages = defineMessages({ id: 'course-authoring.files-and-videos.sort-and-filter.modal.filter.failedCheckbox.label', defaultMessage: 'Failed', }, + videoUploadProgressBarLabel: { + id: 'course-authoring.files-and-videos.add-video-progress-bar.progress-bar.label', + defaultMessage: 'Video upload progress:', + }, }); export default messages;