From 6ae9cdac0088117836138e713606c423bec317d5 Mon Sep 17 00:00:00 2001 From: connorhaugh <49422820+connorhaugh@users.noreply.github.com> Date: Wed, 13 Mar 2024 14:08:18 -0400 Subject: [PATCH] Revert "feat: refactor upload thunk, no progress bar (#894)" (#895) This reverts commit a88a88e9aff28a7d07e9f34f875d5e51b219d4a4. --- .../files-page/data/thunks.js | 1 + src/files-and-videos/generic/FileTable.jsx | 2 - .../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 | 3 +- .../videos-page/data/thunks.js | 57 +++++++------ .../videos-page/data/thunks.test.js | 84 ------------------- .../factories/mockApiResponses.jsx | 1 - src/files-and-videos/videos-page/messages.js | 4 - 10 files changed, 63 insertions(+), 174 deletions(-) delete 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 d07c3c6a4b..9b0a623978 100644 --- a/src/files-and-videos/files-page/data/thunks.js +++ b/src/files-and-videos/files-page/data/thunks.js @@ -189,6 +189,7 @@ 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 a774029224..60d364c5e9 100644 --- a/src/files-and-videos/generic/FileTable.jsx +++ b/src/files-and-videos/generic/FileTable.jsx @@ -244,7 +244,6 @@ const FileTable = ({ setSelectedRows={setSelectedRows} fileType={fileType} /> - - { 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 fed81c0ccb..43a858526b 100644 --- a/src/files-and-videos/videos-page/data/api.js +++ b/src/files-and-videos/videos-page/data/api.js @@ -180,30 +180,21 @@ export async function addVideo(courseId, file) { const postJson = { files: [{ file_name: file.name, content_type: file.type }], }; - const response = await getAuthenticatedHttpClient() - .post(getCourseVideosApiUrl(courseId), postJson); - return { data: camelCaseObject(response.data), ...response }; -} -export async function sendVideoUploadStatus( - courseId, - edxVideoId, - message, - status, -) { - return getAuthenticatedHttpClient() - .post(getCourseVideosApiUrl(courseId), [{ - edxVideoId, - message, - status, - }]); + const { data } = await getAuthenticatedHttpClient() + .post(getCourseVideosApiUrl(courseId), postJson); + return camelCaseObject(data); } export async function uploadVideo( + courseId, uploadUrl, uploadFile, + edxVideoId, ) { - return fetch(uploadUrl, { + const uploadErrors = []; + + await fetch(uploadUrl, { method: 'PUT', headers: { 'Content-Disposition': `attachment; filename="${uploadFile.name}"`, @@ -211,7 +202,28 @@ 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 20a504aafb..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,9 +3,7 @@ import MockAdapter from 'axios-mock-adapter'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { - getDownload, getVideosUrl, getAllUsagePaths, getCourseVideosApiUrl, uploadVideo, sendVideoUploadStatus, -} from './api'; +import { getDownload, getVideosUrl, getAllUsagePaths } from './api'; jest.mock('file-saver'); @@ -105,34 +103,4 @@ 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 c6db45f5c0..74574660f7 100644 --- a/src/files-and-videos/videos-page/data/slice.js +++ b/src/files-and-videos/videos-page/data/slice.js @@ -62,7 +62,7 @@ const slice = createSlice({ deleteVideoSuccess: (state, { payload }) => { state.videoIds = state.videoIds.filter(id => id !== payload.videoId); }, - addVideoById: (state, { payload }) => { + addVideoSuccess: (state, { payload }) => { state.videoIds = [payload.videoId, ...state.videoIds]; }, updateTranscriptCredentialsSuccess: (state, { payload }) => { @@ -102,7 +102,6 @@ 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 f55b7c1366..12c8937282 100644 --- a/src/files-and-videos/videos-page/data/thunks.js +++ b/src/files-and-videos/videos-page/data/thunks.js @@ -20,7 +20,6 @@ import { uploadTranscript, getVideoUsagePaths, deleteTranscriptPreferences, - sendVideoUploadStatus, setTranscriptCredentials, setTranscriptPreferences, getAllUsagePaths, @@ -30,6 +29,7 @@ import { setPageSettings, updateLoadingStatus, deleteVideoSuccess, + addVideoSuccess, updateErrors, clearErrors, updateEditStatus, @@ -42,6 +42,7 @@ 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 })); @@ -86,6 +87,7 @@ 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 })); @@ -101,45 +103,42 @@ export function deleteVideoFile(courseId, id) { export function addVideoFile(courseId, file, videoIds) { return async (dispatch) => { dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.IN_PROGRESS })); + try { - 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'); + 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 })); + }); dispatch(updateEditStatus({ editType: 'add', status: RequestStatus.FAILED })); - return; } - 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 upload ${file.name}.` })); + dispatch(updateErrors({ error: 'add', message: `Failed to add ${file.name}.` })); } 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 deleted file mode 100644 index 96341a268c..0000000000 --- a/src/files-and-videos/videos-page/data/thunks.test.js +++ /dev/null @@ -1,84 +0,0 @@ -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 and set error if url cannot be created.', async () => { - jest.spyOn(api, 'addVideo').mockResolvedValue({ - status: 404, - }); - - await addVideoFile(courseId, mockFile)(dispatch, getState); - - expect(dispatch).toHaveBeenCalledWith({ - payload: { - error: 'add', - message: `Failed to upload ${mockFile.name}.`, - }, - - type: 'videos/updateErrors', - }); - expect(dispatch).toHaveBeenCalledWith({ - payload: { - editType: 'add', - status: 'failed', - }, - type: 'videos/updateEditStatus', - }); - }); - it('Failed video upload dispatches updateEditStatus with failed, and sends the failure to the api', async () => { - const videoStatusMock = jest.spyOn(api, 'sendVideoUploadStatus').mockResolvedValue({ - status: 200, - }); - const mockEdxVideoId = 'iD'; - jest.spyOn(api, 'addVideo').mockResolvedValue({ - status: 200, - data: { - files: [ - { edxVideoId: mockEdxVideoId, uploadUrl: 'a Url' }, - ], - }, - }); - jest.spyOn(api, 'uploadVideo').mockResolvedValue({ - status: 404, - }); - await addVideoFile(courseId, mockFile)(dispatch, getState); - expect(videoStatusMock).toHaveBeenCalledWith(courseId, mockEdxVideoId, 'Upload failed', 'upload_failed'); - expect(dispatch).toHaveBeenCalledWith({ - payload: { - editType: 'add', - status: 'failed', - }, - type: 'videos/updateEditStatus', - }); - }); - it('Successful video upload sends the success to the api', async () => { - const videoStatusMock = jest.spyOn(api, 'sendVideoUploadStatus').mockResolvedValue({ - status: 200, - }); - const mockEdxVideoId = 'iD'; - jest.spyOn(api, 'addVideo').mockResolvedValue({ - status: 200, - data: { - files: [ - { edxVideoId: mockEdxVideoId, uploadUrl: 'a Url' }, - ], - }, - }); - jest.spyOn(api, 'uploadVideo').mockResolvedValue({ - status: 200, - }); - await addVideoFile(courseId, mockFile)(dispatch, getState); - expect(videoStatusMock).toHaveBeenCalledWith(courseId, mockEdxVideoId, 'Upload completed', 'upload_completed'); - }); -}); diff --git a/src/files-and-videos/videos-page/factories/mockApiResponses.jsx b/src/files-and-videos/videos-page/factories/mockApiResponses.jsx index 39e38cbfea..be3d93941b 100644 --- a/src/files-and-videos/videos-page/factories/mockApiResponses.jsx +++ b/src/files-and-videos/videos-page/factories/mockApiResponses.jsx @@ -213,7 +213,6 @@ 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 774dfe2f0b..db10abaf77 100644 --- a/src/files-and-videos/videos-page/messages.js +++ b/src/files-and-videos/videos-page/messages.js @@ -37,10 +37,6 @@ 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;