From efd73f9c0b04dec61fb287430acddd818ba17bc1 Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:52:08 -0400 Subject: [PATCH] fix: progress bar display for uploads (#1135) --- src/files-and-videos/generic/FileTable.jsx | 18 +++--- .../videos-page/VideosPage.jsx | 18 ++++-- .../videos-page/VideosPage.test.jsx | 1 - .../videos-page/data/thunks.js | 58 ++++++++++++++----- .../videos-page/data/thunks.test.js | 8 +-- .../videos-page/upload-modal/UploadModal.jsx | 6 +- 6 files changed, 76 insertions(+), 33 deletions(-) diff --git a/src/files-and-videos/generic/FileTable.jsx b/src/files-and-videos/generic/FileTable.jsx index 9e82ab0502..33a770ffb8 100644 --- a/src/files-and-videos/generic/FileTable.jsx +++ b/src/files-and-videos/generic/FileTable.jsx @@ -243,14 +243,16 @@ const FileTable = ({ fileType={fileType} /> - + {fileType === 'files' && ( + + )} closeUploadTracker(), 500); + break; + case RequestStatus.FAILED: + setTimeout(() => closeUploadTracker(), 500); + break; + default: + closeUploadTracker(); + break; } }, [addVideoStatus]); @@ -298,6 +307,7 @@ const VideosPage = ({ isUploadTrackerOpen, currentUploadingIdsRef: uploadingIdsRef.current, handleUploadCancel, + addVideoStatus, }} /> diff --git a/src/files-and-videos/videos-page/VideosPage.test.jsx b/src/files-and-videos/videos-page/VideosPage.test.jsx index d11788a6b4..fa73cd2e30 100644 --- a/src/files-and-videos/videos-page/VideosPage.test.jsx +++ b/src/files-and-videos/videos-page/VideosPage.test.jsx @@ -685,7 +685,6 @@ describe('Videos page', () => { expect(screen.queryByText('Delete mOckID1.mp4')).toBeNull(); }); - await executeThunk(deleteVideoFile(courseId, 'mOckID1', 5), store.dispatch); await waitFor(() => { const deleteStatus = store.getState().videos.deletingStatus; expect(deleteStatus).toEqual(RequestStatus.FAILED); diff --git a/src/files-and-videos/videos-page/data/thunks.js b/src/files-and-videos/videos-page/data/thunks.js index 9d63b588c6..2fa064ce6a 100644 --- a/src/files-and-videos/videos-page/data/thunks.js +++ b/src/files-and-videos/videos-page/data/thunks.js @@ -224,6 +224,7 @@ const uploadToBucket = async ({ const currentController = new AbortController(); controllers.push(currentController); const currentVideoData = uploadingIdsRef.current.uploadData[edxVideoId]; + try { const putToServerResponse = await uploadVideo( uploadUrl, @@ -279,6 +280,24 @@ const uploadToBucket = async ({ } }; +const newUploadData = ({ + status, + edxVideoId, + currentData, + key, + originalValue, +}) => { + const newData = currentData; + if (edxVideoId && edxVideoId !== key) { + newData[edxVideoId] = { ...originalValue, status }; + delete newData[key]; + return newData; + } + + newData[key] = { ...originalValue, status }; + return newData; +}; + export function addVideoFile( courseId, files, @@ -292,31 +311,38 @@ export function addVideoFile( let hasFailure = false; await Promise.all(files.map(async (file, idx) => { const name = file?.name || `Video ${idx + 1}`; + const progress = 0; + + uploadingIdsRef.current.uploadData = newUploadData({ + status: RequestStatus.PENDING, + currentData: uploadingIdsRef.current.uploadData, + originalValue: { name, progress }, + key: `video_${idx}`, + }); + const { edxVideoId, uploadUrl } = await addVideoToEdxVal(courseId, file, dispatch); + if (uploadUrl && edxVideoId) { - uploadingIdsRef.current.uploadData = { - ...uploadingIdsRef.current.uploadData, - [edxVideoId]: { - name, - status: RequestStatus.PENDING, - progress: 0, - }, - }; + uploadingIdsRef.current.uploadData = newUploadData({ + status: RequestStatus.PENDING, + currentData: uploadingIdsRef.current.uploadData, + originalValue: { name, progress }, + key: `video_${idx}`, + edxVideoId, + }); hasFailure = await uploadToBucket({ courseId, uploadUrl, file, uploadingIdsRef, edxVideoId, dispatch, }); } else { hasFailure = true; - uploadingIdsRef.current.uploadData = { - ...uploadingIdsRef.current.uploadData, - [idx]: { - name, - status: RequestStatus.FAILED, - progress: 0, - }, + uploadingIdsRef.current.uploadData[idx] = { + status: RequestStatus.FAILED, + name, + progress, }; } })); + try { const { videos } = await fetchVideoList(courseId); const newVideos = videos.filter( @@ -337,11 +363,13 @@ export function addVideoFile( updateErrors({ error: 'add', message: 'Failed to load videos' }), ); } + if (hasFailure) { dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED })); } else { dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.SUCCESSFUL })); } + uploadingIdsRef.current = { uploadData: {}, uploadCount: 0, diff --git a/src/files-and-videos/videos-page/data/thunks.test.js b/src/files-and-videos/videos-page/data/thunks.test.js index b06634ed7a..dae6508ecc 100644 --- a/src/files-and-videos/videos-page/data/thunks.test.js +++ b/src/files-and-videos/videos-page/data/thunks.test.js @@ -7,8 +7,8 @@ describe('addVideoFile', () => { const courseId = 'course-123'; const mockFile = { name: 'mockName', - }; + const uploadingIdsRef = { current: { uploadData: {} } }; beforeEach(() => { jest.clearAllMocks(); @@ -18,7 +18,7 @@ describe('addVideoFile', () => { status: 404, }); - await addVideoFile(courseId, [mockFile], undefined, { current: [] })(dispatch, getState); + await addVideoFile(courseId, [mockFile], undefined, uploadingIdsRef)(dispatch, getState); expect(dispatch).toHaveBeenCalledWith({ payload: { @@ -43,7 +43,7 @@ describe('addVideoFile', () => { jest.spyOn(api, 'uploadVideo').mockResolvedValue({ status: 404, }); - await addVideoFile(courseId, [mockFile], undefined, { current: [] })(dispatch, getState); + await addVideoFile(courseId, [mockFile], undefined, uploadingIdsRef)(dispatch, getState); expect(videoStatusMock).toHaveBeenCalledWith(courseId, mockEdxVideoId, 'Upload failed', 'upload_failed'); expect(dispatch).toHaveBeenCalledWith({ payload: { @@ -70,7 +70,7 @@ describe('addVideoFile', () => { jest.spyOn(api, 'uploadVideo').mockResolvedValue({ status: 200, }); - await addVideoFile(courseId, [mockFile], undefined, { current: [] })(dispatch, getState); + await addVideoFile(courseId, [mockFile], undefined, uploadingIdsRef)(dispatch, getState); expect(videoStatusMock).toHaveBeenCalledWith(courseId, mockEdxVideoId, 'Upload completed', 'upload_completed'); }); }); diff --git a/src/files-and-videos/videos-page/upload-modal/UploadModal.jsx b/src/files-and-videos/videos-page/upload-modal/UploadModal.jsx index 4169f31663..d2fa526edd 100644 --- a/src/files-and-videos/videos-page/upload-modal/UploadModal.jsx +++ b/src/files-and-videos/videos-page/upload-modal/UploadModal.jsx @@ -12,15 +12,18 @@ import { import { WarningFilled } from '@openedx/paragon/icons'; import messages from '../messages'; import UploadProgressList from './UploadProgressList'; +import { RequestStatus } from '../../../data/constants'; const UploadModal = ({ isUploadTrackerOpen, handleUploadCancel, currentUploadingIdsRef, + addVideoStatus, }) => { const intl = useIntl(); const videosPagePath = ''; const { uploadData, uploadCount } = currentUploadingIdsRef; + const cancelIsDisabled = addVideoStatus === RequestStatus.FAILED || addVideoStatus === RequestStatus.SUCCESSFUL; return ( - @@ -89,6 +92,7 @@ UploadModal.propTypes = { }).isRequired, uploadCount: PropTypes.number.isRequired, }).isRequired, + addVideoStatus: PropTypes.string.isRequired, }; export default UploadModal;